Make listener finished method work with reverse insertion order #2627
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
krmahadevan
requested changes
Aug 12, 2021
testng-core/src/main/java/org/testng/internal/invokers/BaseInvoker.java
Outdated
Show resolved
Hide resolved
testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java
Outdated
Show resolved
Hide resolved
testng-core/src/main/java/org/testng/junit/JUnitTestRunner.java
Outdated
Show resolved
Hide resolved
dianny123
force-pushed
the
listenerOrder
branch
2 times, most recently
from
0facded
to
4d1b3f1
Compare
|
comments fixed, pls have a look, thanks! |
krmahadevan
reviewed
Aug 19, 2021
krmahadevan
reviewed
Aug 19, 2021
krmahadevan
reviewed
Aug 19, 2021
krmahadevan
reviewed
Aug 19, 2021
dianny123
force-pushed
the
listenerOrder
branch
from
4d1b3f1
to
193115a
Compare
krmahadevan
approved these changes
Aug 20, 2021
|
@dianny123 - Thanks for taking the time to help raise this PR. Once @juherr takes a quick look and approves, we can get this merged. I believe that @juherr is on a personal time off. So lets wait for him to be back |
juherr
reviewed
Aug 22, 2021
| @@ -54,7 +55,12 @@ protected void runInvokedMethodListeners( | |||
|
|
|||
| InvokedMethodListenerInvoker invoker = | |||
| new InvokedMethodListenerInvoker(listenerMethod, testResult, testResult.getTestContext()); | |||
| for (IInvokedMethodListener currentListener : m_invokedMethodListeners) { | |||
| boolean isBeforeInvocation = InvokedMethodListenerMethod.BEFORE_INVOCATION == listenerMethod; | |||
There was a problem hiding this comment.
Could you add a comment which explains why we have a specific behavior for BEFORE_INVOCATION?
There was a problem hiding this comment.
For BEFORE_INVOCATION method, still run as insert order, but regarding AFTER_INVOCATION, it should be reverse order.
There was a problem hiding this comment.
Why not, but why is it important? How/where can we track the rule?
Could you implement what you describe by replacing isBeforeInvocation with isAfterInvocation?
juherr
reviewed
Aug 23, 2021
| @@ -232,7 +233,12 @@ public FailureContext retryFailed( | |||
| } | |||
|
|
|||
| public void runTestResultListener(ITestResult tr) { | |||
| TestListenerHelper.runTestListeners(tr, m_notifier.getTestListeners()); | |||
| boolean isStarted = tr.getStatus() == ITestResult.STARTED; | |||
There was a problem hiding this comment.
juherr
reviewed
Aug 23, 2021
| @@ -87,7 +84,12 @@ public void endTest(Test test) { | |||
|
|
|||
| org.testng.internal.TestResult tr = recordResults(test, tri); | |||
|
|
|||
| TestListenerHelper.runTestListeners(tr, m_parentRunner.getTestListeners()); | |||
| boolean isStarted = tr.getStatus() == ITestResult.STARTED; | |||
There was a problem hiding this comment.
Now the listener start method and finished method worked with insertion order, this commit changes the finished method worked with reverse order.
dianny123
force-pushed
the
listenerOrder
branch
from
193115a
to
7a096c7
Compare
juherr
approved these changes
Aug 24, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now the listener start method and finished method worked with insertion
order, this commit changes the finished method worked with reverse
order.
Fixes #2558 .