-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test case failure #3093
Test case failure #3093
Conversation
Fixing failure for the test test.retryAnalyzer.RetryAnalyzerTest.ensureRetryDoesntRunEndlesslyForDataDrivenTests Github actions failure on `master` branch: https://github.com/testng-team/testng/actions/runs/8307524777/job/22736819681 Gradle scan results link: https://scans.gradle.com/s/wap5a5bvq3qgq/tests/task/:testng-core:test/details/test.retryAnalyzer.RetryAnalyzerTest/ensureRetryDoesntRunEndlesslyForDataDrivenTests?top-execution=1 Prerequisites for failure: The below JVM arguments should be used: * `-Duser.timezone="America/New_York”` * `-Duser.country=RU` * `-Duser.language=ru` * `-XX:+UnlockExperimentalVMOptions` * `-XX:hashCode=2` Root cause: We are using hashCode() to create Key that is used to determine if a RetryAnalyzer instance should be created or not. When we run the tests using `-XX:hashCode=2` it causes the JVM to generate the same hashcode (value 1) And this messes up the `RetryAnalyzer` object creation. Fix: Addressed this by wrapping the parameters into an object which will contain a unique id that can be used to represent the same set of parameters.
WalkthroughThe recent modifications focus on enhancing efficiency and reliability in handling test parameters within TestNG. By introducing a Changes
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java (3 hunks)
- testng-core/src/main/java/org/testng/internal/IObject.java (2 hunks)
Additional comments: 3
testng-core/src/main/java/org/testng/internal/IObject.java (1)
- 122-148: The introduction of the
IdentifiableArrayObject
class is a strategic move to address the issue with hash code manipulation under specific JVM settings. This class ensures that each set of parameters is uniquely identified, which is crucial for the correct operation of theRetryAnalyzer
in data-driven tests. The implementation ofequals
andhashCode
methods based on the parameters array is appropriate for this use case. However, consider documenting the potential impact of usingArrays.hashCode(parameters)
inhashCode
andArrays.equals(parameters, that.parameters)
inequals
for large arrays, as these operations could become performance bottlenecks in certain scenarios.testng-core/src/main/java/org/testng/internal/BaseTestMethod.java (2)
- 836-836: The modification to use a
ConcurrentHashMap
for cachingIRetryAnalyzer
instances based on a unique parameter ID is a robust solution to the problem of hash code manipulation under specific JVM settings. This change ensures that retries are handled correctly for parameterized tests, addressing the core issue of endless retries in data-driven tests. The use ofparameterId
as a key is a clever approach to uniquely identify test method invocations.- 849-852: The introduction of the
parameterId
method, which generates a unique ID for test parameters, is a key part of the solution to ensure the correct operation of theRetryAnalyzer
in the face of hash code manipulation. By cachingIdentifiableArrayObject
instances in aConcurrentHashMap
, this method efficiently handles parameter uniqueness. This approach significantly improves the reliability of retries in parameterized tests.
Fixing failure for the test
test.retryAnalyzer.RetryAnalyzerTest.ensureRetryDoesntRunEndlesslyForDataDrivenTests
Github actions failure on
master
branch:https://github.com/testng-team/testng/actions/runs/8307524777/job/22736819681
Gradle scan results link:
https://scans.gradle.com/s/wap5a5bvq3qgq/tests/task/:testng-core:test/details/test.retryAnalyzer.RetryAnalyzerTest/ensureRetryDoesntRunEndlesslyForDataDrivenTests?top-execution=1
Prerequisites for failure:
The below JVM arguments should be used:
-Duser.timezone="America/New_York”
-Duser.country=RU
-Duser.language=ru
-XX:+UnlockExperimentalVMOptions
-XX:hashCode=2
Root cause:
We are using
hashCode()
to create Key that is used to determine if aRetryAnalyzer
instance should be created or not.When we run the tests using
-XX:hashCode=2
it causes the JVM to generate the same hashcode (value 1) And this messes up the
RetryAnalyzer
object creation.Fix:
Addressed this by wrapping the parameters into an object which will contain a unique id that can be used to represent the same set of parameters.
Summary by CodeRabbit