Fix PlainOsgiTest #3097
Fix PlainOsgiTest #3097
Conversation
This test has recently started failing after the PR testng-team#3089 got merged. With the below exception org.ops4j.pax.exam.TestContainerException: java.lang.IllegalArgumentException: Can not set java.util.List field org.testng.internal.NoOpTestClass.m_beforeTestMethods to [Lorg.testng.ITestNGMethod; This PR changes the type of a field Which was previously an array to a list. The PaxExam listener is making Use of reflection to access the data member “m_beforeTestMethods” instead of it just using The “setter” that is part of NoOpTestClass. Since this library is being used ONLY for test and Since the library in question seems to be something That is not updated for quite sometime, resorting to class path overriding wherein we duplicate the Same class into TestNG codebase and then fix the Problem locally so that the tests will pass.
WalkthroughThis update introduces a TestNG driver for Pax Exam, enhancing integration by implementing TestNG listener interfaces. It facilitates running TestNG tests with Pax Exam support, managing test methods, dependency injections, transactions, and reactor staging efficiently. The driver also includes improvements in configuration methods, probe invokers, and test method manipulation. Changes
Related issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- testng-test-osgi/src/test/java/org/ops4j/pax/exam/testng/listener/PaxExam.java (1 hunks)
Additional comments: 1
testng-test-osgi/src/test/java/org/ops4j/pax/exam/testng/listener/PaxExam.java (1)
- 494-497: The change to use a list instead of an array for
m_beforeTestMethodsandsetAfterTestMethodinNoOpTestClassis a direct response to the issue described in the PR objectives. This modification ensures compatibility with the PaxExam library's reflection-based access patterns. However, it's crucial to ensure that this change does not inadvertently affect other parts of the TestNG framework that might rely on the original array type. It would be beneficial to verify that all internal usages within TestNG and external dependencies that might interact with these methods are compatible with this change.
|
Could you make a pull request of the fix here https://github.com/ops4j/org.ops4j.pax.exam2 ? |
I didn't quite get you @juherr Are you saying that we need to do the following :
|
|
Yes, except the pr should be done before because we need the link 😉 |
|
Here's the PR: ops4j/org.ops4j.pax.exam#1112 |
|
Perfect 👍 Just temporary disable the failing test. |
|
The change is part of #3098 |
This test has recently started failing after the PR #3089
got merged.
With the below exception
PR #3089 changed the type of a field which was previously an array to a list.
The PaxExam listener is making use of reflection to access the data member
m_beforeTestMethodsinstead of it just usingThe “setter” that is part of NoOpTestClass.
Since this library is being used ONLY for test and Since the library in question seems to be something That is not updated for quite sometime, resorting to class path overriding wherein we duplicate the Same class into TestNG codebase and then fix the
Problem locally so that the tests will pass.
Summary by CodeRabbit