Skip to content
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

Introduce TestWatcher extension API for processing test execution results #542

Closed
SqAutoTestTeam opened this issue Oct 11, 2016 · 26 comments

Comments

@SqAutoTestTeam
Copy link

SqAutoTestTeam commented Oct 11, 2016

Overview

Some functionality written in JUnit 4 is based on the usage of Rules.

There a custom rule is used to send test results to an external system - where they can be seen later.

The TestWatcher Rule API in JUnit 4 provides me with the following three callback methods:

  • protected void succeeded(Description desc);
  • protected void failed(Throwable e, Description desc);
  • protected void skipped(AssumptionViolatedException e, Description desc);

This enables me to be notified what the test result is -- failed, success, or skipped -- (AssumptionViolatedException exception is thrown during test execution).

@mmerdes has done a good job of adapting existing JUnit 4 rules to extensions in JUnit Jupiter,
but this "migration support" unfortunately does support method result callbacks.

I've researched this topic and came to realize that for now there is no straight reliable way to implement this feature using only JUnit Jupiter's extension model.

The closest extension is AfterTestExecutionCallback, but its callback method
afterTestExecution(ExtensionContext) does not provide the necessary information (i.e., the test execution result).

So you can't tell for sure if the test succeeded, failed, or was aborted/skipped. Indirect ways to solve this problem do not look reliable.

Proposal

One possible solution is to introduce a new Extension API (e.g., TestFinished) that receives the test execution result via its callback method.

public interface TestFinished extends Extension {
    void testExecutionFinished(TestIdentifier testIdentifier, TestExtensionContext context, 
                           TestExecutionResult testExecutionResult) throws Exception;
}

Each extension that implements the TestFinished interface will get notified of the test execution result.

@marcphilipp
Copy link
Member

Does this need to be a Jupiter extension or would a Platform TestExecutionListener work for you (if there was a more dynamic way to register it as proposed in #19 (comment))?

@SqAutoTestTeam
Copy link
Author

SqAutoTestTeam commented Oct 12, 2016

@marcphilipp

From my point of view :

Does this need to be a Jupiter extension ?

Yes this needs to be a Jupiter extension - because I need to know test result from extension.

For now there is no way to do so.

Or would a Platform TestExecutionListener work for you ?

No. Not at the moment

Typical use of TestExecutionListener is:

Launcher launcher = LauncherFactory.create();
launcher.registerTestExecutionListeners(new TestExecutionListener() {
    // ...
})

launcher.discover( <---- to discover tests, get TestPlan ...
launcher.execute( <----  actual tests execution

launcher object is not accessible from extensions, so for now there is no way yo use it from within an extension object.

But if you mean something like this

    /**
     * Callback that is invoked <em>before</em> each test is invoked.
     *
     * @param context the current extension context; never {@code null}
     */
    void beforeEach(TestExtensionContext context) throws Exception {
        // context.launcher()  <--- new method 
        context.launcher().registerTestExecutionListeners(....) // < ---- dynamic registering

it could work.

Nevertheless using this approach the caller must take care of unregistering added listener - all listeners are registered into the same object (added into internal notification list, I suppose).
unRegisterTestExecutionListeners() method does not also exist for now.

if custom code does not do that unregistration (by mistake) - notification list can be overfilled, (that in turn, results in memory leaks, triggering listeners that are not used... etc, and affect the rest of tests.)

context.launcher() <-- seems to be out of its place.

Approach with ....

public interface TestFinished extends Extension {
void testExecutionFinished(TestIdentifier testIdentifier, TestExtensionContext context, 
TestExecutionResult testExecutionResult) throws Exception;
}

seems more reliable.

@sbrannen sbrannen changed the title [Extension model] - Test result extension. Introduce extension for processing test results Oct 12, 2016
@marcphilipp
Copy link
Member

Let me try to sketch out what I meant in a bit more detail.

You would create a JAR with your TestExecutionListener and include a file META-INF/services/org.junit.platform.launcher.TestExecutionListener with the following content.

your.package.YourListenerImplementation

The JUnit Platform launcher would look for your listener implementation using the ServiceLoader API (i.e. the same way it finds TestEngines) and register it automatically.

You wouldn't have to reference your listener in any of your test cases. It just needs to be on the test runtime classpath. Would that work for you?

@SqAutoTestTeam
Copy link
Author

SqAutoTestTeam commented Nov 1, 2016

@marcphilipp I apologise in advance if I still misunderstand your point.
What I initially wanted is to know test execution result (status) from within a test class just like it is implemented in junit4 rules. JUnit5 provides well formed extension model but it does not have that feature.

It does not take much efforts to implement this feature

        Iterator<TestExecutionListener> providers = Service.providers(TestExecutionListener.class);
        while(providers.hasNext()) {
            TestExecutionListener listener = providers.next();

            listener;// <----

        }

but it will not give much of advantage. because it is not enough to make class to be aware of test results. (at least I did not find a way). But even though there is a way to make it

for instance
your.package.YourListenerImplementation somehow has a reference to listener list where it is added to to start "listening"
it is still a custom solution. I am asking for making it a part of extension model.

Could you please clarify what you mean if I still misunderstand you.

@marcphilipp
Copy link
Member

@junit-team/junit-lambda Can one of you chime in? 🤔

@sbrannen
Copy link
Member

sbrannen commented Nov 1, 2016

I think that we in general do need such functionality.

For example, in order to implement something like quickcheck as a JUnit Jupiter extension, the extension would need a real-time mechanism for determining whether previously submitted tests (e.g., dynamic tests or parameterized tests) succeeded in order to determine if additional tests should be submitted/generated.

The discussion in this issue simply involves a different use case for gaining access to the test execution result.

I thought we had discussed this topic elsewhere, but I don't have time to search right now. ;)

@SqAutoTestTeam
Copy link
Author

SqAutoTestTeam commented Nov 7, 2016

I think that we in general do need such functionality.

When approximately are you planing to implement it ?

@nipafx
Copy link
Contributor

nipafx commented Mar 12, 2017

FYI: People on StackOverflow are also looking for this feature.

@marcphilipp
Copy link
Member

Thanks, I've added a comment there: http://stackoverflow.com/questions/42721422/junit5-how-to-get-test-result-in-aftertestexecutioncallback#comment72608741_42721422

Looking at the test exception is not really reliable. The test might fail in an extension that hasn't been invoked, yet. I think we might need a new extension point that cannot influence the test's result. Any other ideas?

@nipafx
Copy link
Contributor

nipafx commented Mar 12, 2017

A comment? Pfft, I made it an answer. 😉

I think we might need a new extension point that cannot influence the test's result.

That makes sense, yes. The interesting question is, what else this extension point would be allowed to do. See Sam's comment above:

For example, in order to implement something like quickcheck as a JUnit Jupiter extension, the extension would need a real-time mechanism for determining whether previously submitted tests (e.g., dynamic tests or parameterized tests) succeeded in order to determine if additional tests should be submitted/generated.

@sormuras
Copy link
Member

For me, two extension points make sense:

  • one read-only test execution result consumer that cannot influence the test's result or change the original test plan. Maybe this simple extension point can "inherit/borrow/copy" methods from already mentioned TestExecutionListener.
  • one re-active that listens on the same events, but has measures to alter the test execution plan.

@bugs84
Copy link

bugs84 commented Mar 12, 2017

I asked this stackoverflow question.

I'll try to describe my usecase:

We have few tests with custom annotation something like:
@RelatedTo("ExternalId-123")
@test
void crucialTest() {}

when this test is executed I want write "specific String" to console. e.g.
"Test related to 'ExternalId-123' was executed. And PASSED/FAILED"

When we are releasing version. This specific string is parsed and recorded into external system.
(Because we want to be sure, that we didn't lost some crucial tests)

So i wrote Extension. Which will write this "specific String" to console. Everyting works like a charm and I really like your extension model. Good work boys!
Only issue was with getting test result. It is a little bit weird to get test result by "testException==null".
Plus it can return wrong result in case, that there are another Extensions.

Thats why I wasn't sure. If I do it correctly.
Maybe, that TestExecutionListener could be better way. I'll have to examine TestExecutionListener.

@brcolow
Copy link

brcolow commented Mar 12, 2017

If I understand this issue correctly, it is a super-set of #618 - is this correct?

I see in the stackoverflow issue you requested for us to write our use case - well, here's mine:

I am intending to use this functionality to take a screenshot of the GUI and save them so that test failures are accompanied by screenshots (see TestFX/TestFX#306 for more details).

Thanks. I hope I am understanding the state of things correctly.

@SqAutoTestTeam
Copy link
Author

SqAutoTestTeam commented Aug 2, 2017

Do I understand it right that for now to get test result state I have to use AfterTestExecutionCallback ?. Then from within implemented method I can take
the instance of test execution exception from passed context

context.getExecutionException().isPresent()
and then if there is a instance of Throwable I consider that the test is failed, otherwise it is not.
But it is still uncertain about if a test is skipped.

Are there any plans to implement this feature?
If so when approximately are you going to finish it?
If not please provide any reliable workaround.

As for question

If I understand this issue correctly, it is a super-set of #618 - is this correct?
Seems like they are related but this issue not exactly a super-set of #618

#618 is about TestExecutionListener which is not accessible from within extensions
when I initially asked for a extension - feature that gives an extension with callbacks for three possible test result
(Success, Failure, Skipped)

@marcphilipp marcphilipp added this to the 5.1 Backlog milestone Aug 19, 2017
@marcphilipp
Copy link
Member

I'm adding this issue to 5.1 Backlog. That means that we will reconsider it when we go over our 5.1 backlog. The current focus, however, is to finish the first GA version.

@littleclay
Copy link

Any additional word on this? It would be super useful. Currently we have JUnit rules for some tests that evaluate not only the test result but also some custom timing data and report that to an external service. Using an engine listener wouldn’t work for us since we would need to get access to the timing data which would be stored in an extension store.

@littleclay
Copy link

Also, a reactive version that allows for an alteration of the TestPlan sounds very interesting for things like automatically retrying certain tests only on failures, etc.

@TpSr52
Copy link

TpSr52 commented Feb 2, 2018

@marcphilipp There is any news about this feature?

@marcphilipp
Copy link
Member

Currently we have JUnit rules for some tests that evaluate not only the test result but also some custom timing data and report that to an external service. Using an engine listener wouldn’t work for us since we would need to get access to the timing data which would be stored in an extension store.

@littleclay You can publish the timing data from your extension using ExtensionContext.publishReportEntry() and consume it in TestExecutionListener.reportingEntryPublished(). However, it currently only supports strings.

Also, a reactive version that allows for an alteration of the TestPlan sounds very interesting for things like automatically retrying certain tests only on failures, etc.

It does, but I think it may be a different extension point as pointed out by @sormuras in #542 (comment).

There is any news about this feature?

@TpSr52 No, unfortunately not. While we're not opposed to adding such an extension point, we haven't had time to work on it so far. It would help if someone submitted a pull request for a TestWatcher-like extension point, so we'd have something concrete to discuss. Thus, I'm putting this issue "up-for-grabs" for now. 😉

@deepakab03
Copy link

It would be useful if this feature is added to the extension framework, we have a need to check when a Test is failed and do certain actions on the same

@geo-m
Copy link
Collaborator

geo-m commented Mar 24, 2018

This does sound quite fun to take a look at (up-for-grabs 😎 ) However, it is still not completely clear to me what you're trying to achieve that TestExecutionListener.executionFinished doesnt already do.

If it's about the store, i personally use an external map in my listener class, by uniqueID

@deepakab03 , @bugs84 care to explain please?

@igorstojanovski
Copy link

@geo-m @nicolaiparlog I tried to use a TestExecutionListener to get the result info into my CustomExtension, but for me it didn't work because of the order of the calls:

  • TestExecutionListener.executionStarted
  • CustomExtension.afterTestExecution
  • CustomExtension.afterEach
  • TestExecutionListener.executionFinished

I can't use any info collected in executionFinished because its collected after the extension methods are called where I actually need to use the info.

@marcphilipp marcphilipp modified the milestones: 5.3 RC1, 5.4 M1 Aug 13, 2018
@vyarosh
Copy link

vyarosh commented Aug 17, 2018

Voting for the initial request.
My use case is:

    @AfterEach
    public void finishTestCase(TestExecutionResult result) {
        try {
            ExtentManager.createScreenShot(driver.get());
            LOGGER.debug("Finishing test... " + result.getName());
            switch (result.getStatus()) {
                case FAILURE:
                    ExtentManager.getCurrentReporter().get().log(
                            Status.FAIL,
                            Utils.prettyReport(result.getThrowable().toString()),
                            ExtentManager.buildScreenshotMedia(result.getName()));
                    break;
                case SKIP:
                    ExtentManager.getCurrentReporter().get().skip(result.getThrowable().toString());
                    break;
                case SUCCESS:
                    ExtentManager.getCurrentReporter().get().info(
                            "Last Screen of the test",
                            ExtentManager.buildScreenshotMedia(result.getName()));
                    break;
            }
        } finally {
            LOGGER.debug("Closing Browser...");
            driver.get().quit();
        }

where ExtentManager is com.aventstack.extentreports.ExtentReports implementation.

@sbrannen sbrannen changed the title Introduce extension API for processing test execution results Introduce TestWatcher extension API for processing test execution results Nov 30, 2018
@marcphilipp marcphilipp modified the milestones: 5.4 M1, 5.4 M2 Dec 7, 2018
sbrannen pushed a commit to sbrannen/junit5 that referenced this issue Jan 12, 2019
sbrannen added a commit to sbrannen/junit5 that referenced this issue Jan 12, 2019
sbrannen added a commit to sbrannen/junit5 that referenced this issue Jan 12, 2019
sbrannen added a commit to sbrannen/junit5 that referenced this issue Jan 12, 2019
sbrannen added a commit that referenced this issue Jan 12, 2019
sbrannen added a commit that referenced this issue Jan 12, 2019
sbrannen added a commit that referenced this issue Jan 12, 2019
sbrannen added a commit that referenced this issue Jan 12, 2019
@sbrannen sbrannen self-assigned this Jan 12, 2019
@sbrannen
Copy link
Member

Closing this issue since the work by @geo-m (in #1393) has been merged into master in d1cee80 and further refined in 6ad7622 and d1cee80.

Remaining work will be performed in #1730.

@ghost ghost removed the status: in progress label Jan 12, 2019
sbrannen added a commit that referenced this issue Jan 13, 2019
sbrannen added a commit that referenced this issue Jan 13, 2019
sbrannen added a commit that referenced this issue Jan 13, 2019
Prior to this commit, the log message generated for exceptions thrown
by a TestWatcher callback method was specific to @test methods. However,
since @testfactory methods and @testtemplate methods (e.g.,
@RepeatedTest and @ParameterizedTest) are supported by the same
infrastructure, the log message was misleading in such cases.

This commit improves the log message by including the display name as
well. This helps to discern between individual invocations of repeated
tests, dynamic tests, parameterized tests, etc.

Issue: #542
sbrannen added a commit that referenced this issue Jan 13, 2019
This commit disables TestWatcher support for @testfactory methods, since
such methods are actually containers, and the TestWatcher API currently
does not support containers.

Issue: #542
sbrannen added a commit that referenced this issue Jan 13, 2019
sbrannen added a commit that referenced this issue Jan 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests