Copy test result attributes when unexpected failures #3065
Conversation
WalkthroughThis update addresses critical issues within TestNG, focusing on enhancing the reliability of test result handling and listener functionality. It resolves problems related to the loss of Changes
Assessment against linked 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: 1
Configuration used: CodeRabbit UI
Files selected for processing (6)
- CHANGES.txt (1 hunks)
- testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java (1 hunks)
- testng-core/src/test/java/test/listeners/ListenersTest.java (2 hunks)
- testng-core/src/test/java/test/listeners/issue3064/EvidenceListener.java (1 hunks)
- testng-core/src/test/java/test/listeners/issue3064/EvidenceRetryAnalyzer.java (1 hunks)
- testng-core/src/test/java/test/listeners/issue3064/SampleTestCase.java (1 hunks)
Additional comments: 6
testng-core/src/test/java/test/listeners/issue3064/EvidenceRetryAnalyzer.java (1)
- 8-10: LGTM. The intentional failure in the constructor serves the testing purpose.
testng-core/src/test/java/test/listeners/issue3064/EvidenceListener.java (1)
- 6-18: LGTM. The implementation correctly sets an attribute on test start and captures the test result on failure for testing purposes.
testng-core/src/test/java/test/listeners/issue3064/SampleTestCase.java (1)
- 16-27: LGTM. The suite setup correctly assigns the retry analyzer, and the test method is designed to fail for testing purposes.
testng-core/src/test/java/test/listeners/ListenersTest.java (1)
- 42-47: LGTM. The test method correctly validates that attributes set in
EvidenceListenerare carried forward as expected.testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java (1)
- 975-976: Verify the implementation of attribute copying from the current test result to a newly created test result before marking the test as a failure, as described in the PR objectives. This specific change is not annotated in the provided code.
CHANGES.txt (1)
- 3-3: Ensure the version number
7.10.0is correctly incremented based on the previous version in the project history.
| Current | ||
| 7.10.0 | ||
| Fixed: GITHUB-3064: TestResult lost if failure creating RetryAnalyzer (Krishnan Mahadevan) | ||
| Fixed: GITHUB-3048: ConcurrentModificationException when injecting values (Krishnan Mahadevan) | ||
| Fixed: GITHUB-3050: Race condition when creating Guice Modules (Krishnan Mahadevan) | ||
| Fixed: GITHUB-3059: Support the ability to inject custom listener factory (Krishnan Mahadevan) |
There was a problem hiding this comment.
Tip
Codebase Verification
The verification process for the issues mentioned in the CHANGES.txt file for version 7.10.0 reveals the following:
- GITHUB-3064 is still OPEN and assigned to milestone 7.10.0. This indicates that it might not be accurately resolved in this version as mentioned.
- GITHUB-3048, GITHUB-3050, GITHUB-3059, and GITHUB-3045 are CLOSED and correctly assigned to milestone 7.10.0, aligning with the summary of changes.
Given this discrepancy with GITHUB-3064, it appears there is an issue with the completeness of the resolution as documented in the CHANGES.txt file for version 7.10.0.
Analysis chain
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [3-7]
The summary of changes for version 7.10.0 should be consistent with the project's versioning history and the detailed list of changes provided in the file. Confirm that all mentioned issues (e.g., GITHUB-3064, GITHUB-3048) are accurately resolved in this version.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the resolution of mentioned GitHub issues
gh issue view 3064 --repo testng-team/testng
gh issue view 3048 --repo testng-team/testng
gh issue view 3050 --repo testng-team/testng
gh issue view 3059 --repo testng-team/testng
gh issue view 3045 --repo testng-team/testng
gh issue view 3038 --repo testng-team/testng
gh issue view 3022 --repo testng-team/testng
Length of output: 19211
There shouldn't be any memory implications because we are only copying the attributes that were set by a user's listener else where to be available in the test result being passed on to |
Closes #3064
Fixes #3064 .
Did you remember to?
CHANGES.txt./gradlew autostyleApplyWe encourage pull requests that:
If your pull request involves fixing SonarQube issues then we would suggest that you please discuss this with the
TestNG-dev before you spend time working on it.
Note: For more information on contribution guidelines please make sure you refer our Contributing section for detailed set of steps.
Summary by CodeRabbit
EvidenceListenerandEvidenceRetryAnalyzerto improve test result attributes handling and retry logic.