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

Enhance DynamicTest API to support "ignored" tests so that I can see them in the test reports #1439

Closed
k1w1m8 opened this issue May 31, 2018 · 14 comments

Comments

@k1w1m8
Copy link

k1w1m8 commented May 31, 2018

I am planning to use DynamicTest API to as a kind of proxy to create and run tests inside an IDE or Maven which are written in XML and stored on a filesystem.

The tests stored on the filesystem include the concept of an ignored test, which is a feature supported by JUnit 4 via @Ignore and JUnit 5 via @Disabled, where the ignored/disabled test is shown visibly on the IDE test UI, or in the HTML JUnit reports generated by Maven.

However, I am not aware of a way to declare an ignored test via the DynamicTest API so that the ignored test is shown in the test reporting in IDE or HTML reports, as can be done for a static test.

I would like to see the DynamicTest API extended to allow me to dynamically ignore tests so that the ignored tests are shown in the UI rather than simply disappearing.

Hopefully this should be quite trivial to implement ;-)

@sormuras
Copy link
Member

sormuras commented Jun 1, 2018

Dynamic tests don't have lifecycles attached to them. Thus there is no way to statically disable dynamic tests. Or put it the other way round: you need to spawn a dynamic test and execute it for it to show up in the test plan.

However, you may achieve a similar behaviour using failing Assumptions in your executable. After modifying the demo source of dynamicTestsFromIntStream() from https://junit.org/junit5/docs/current/user-guide/#writing-tests-dynamic-tests-examples to this executable...

() -> Assumptions.assumeTrue(n % 3 == 0)

...all generated and executed tests are visible in the test plan tree.

image

Does this help?

@sbrannen
Copy link
Member

sbrannen commented Jun 1, 2018

I agree with @sormuras.

Dynamic tests cannot be ignored or disabled according to the semantics of the ExecutionCondition extension API; however, you can achieve your goal with the use of assumptions.

@sbrannen
Copy link
Member

sbrannen commented Jun 1, 2018

In light of the above comments pointing to a viable solution via assumptions, I am closing this issue.

@k1w1m8
Copy link
Author

k1w1m8 commented Jun 4, 2018

Hi, thanks for the feedback.

My intention with being able to "ignore" dynamic tests is not to actually "run" a test, simply to call a method to declare that the underlying test (stored on the fs) is disabled and to show the test as being ignored in the test plan tree.

If I can achieve that result by creating a fake dynamic test which simply generates an assertion failure, it will be good enough for my purposes.

Saying that, it does seem a bit hackky. I wonder if there is a more explicit way it could be done.

@sbrannen
Copy link
Member

sbrannen commented Jun 4, 2018

If I can achieve that result by creating a fake dynamic test which simply generates an assertion failure, it will be good enough for my purposes.

Don't throw an AssertionError (or "generate an assertion failure").

As @sormuras mentioned, Assumptions are the appropriate mechanism for aborting a test (or dynamic test) mid-flight.

@k1w1m8
Copy link
Author

k1w1m8 commented Jun 8, 2018

Sigh. Bad typo there on my part. I meant Assumption rather than Assertion.

However, looking further at Assumptions rather than "assuming", it seems they are the correct way of dealing with my issue.

Thanks for your patience.

Looking forward to using DynamicTests combined with Assumptions to allow my QA staff to re-run any test via right click, they will be so happy.

@sbrannen
Copy link
Member

sbrannen commented Jun 8, 2018

Looking forward to using DynamicTests combined with Assumptions to allow my QA staff to re-run any test via right click, they will be so happy.

👍

Thanks for the feedback.

@k1w1m8
Copy link
Author

k1w1m8 commented Jun 12, 2018

Well it seems to work

    @Override
    public void execute() throws Throwable {
        if (!test.isIgnored()) {
            runner.run(test);
        } else {
            System.out.println("Ignoring test " + test.getPath());

            // Junit team told me this is how to do "ignored" in JUnit 5 DynamicTest
            // https://github.com/junit-team/junit5/issues/1439
            Assumptions.assumeTrue(false, "This test was ignored");
        }
    }
Ignoring test /ExceptionFlows/#XXX_XXX_In.EF01p.020.CrdtTrnf

org.opentest4j.TestAbortedException: Assumption failed: This test was ignored


	at org.junit.jupiter.api.Assumptions.throwTestAbortedException(Assumptions.java:249)
	at org.junit.jupiter.api.Assumptions.assumeTrue(Assumptions.java:108)
	at com.clear2pay.jetqarun.junit5.JUnit5Executable.execute(JUnit5Executable.java:27)
	at org.junit.jupiter.engine.descriptor.JupiterTestDescriptor.executeAndMaskThrowable(JupiterTestDescriptor.java:141)

Slightly klunky, but gets the job done.

I've yet to test it with Maven FailSafe yet. Hopefully that works just as well.

@sbrannen
Copy link
Member

I've yet to test it with Maven FailSafe yet. Hopefully that works just as well.

That will work just fine.

@sbrannen
Copy link
Member

sbrannen commented Jun 13, 2018

By the way, the following would be the idiomatic way to use assumptions in this scenario.

    @Override
    public void execute() throws Throwable {
        // abort early?
        assumeFalse(test.isIgnored());
        // carry on with test...
        runner.run(test);
    }

If the assumption fails, the test will be reported as having been "aborted". Thus there is typically no need to manually log the fact as well.

@k1w1m8
Copy link
Author

k1w1m8 commented Jun 13, 2018

Thanks for that, my code is now "idiomatic". Love that term ;-).

Surefire indeed works, in that it reports test success, failures and now ignores!

Unfortunately, the surefire reporting of Dynamic Tests is not able to report the test names properly (we get the synthetic ids instead), which means our CI will need to remain using JUnit 3 Suite/Test "dynamic" tests for now.

This was already reported in #1320 so I have added a comment there.

I also noticed that when I ran using the JUnit5 provider and (accidentally) Java 7, tests were simply not run - there was no error saying "JUnit5 requires Java 8". Took me a while to figure that out. I raised #1464 for that.

@k1w1m8
Copy link
Author

k1w1m8 commented Oct 14, 2018

BTW, I retried this in IDEA 2018.3 EAP and the dynamic tests with false assumptions are no longer displayed in the results tree!

Sigh. I raised https://youtrack.jetbrains.com/issue/IDEA-200500 for that.

@sbrannen
Copy link
Member

Sigh. I raised https://youtrack.jetbrains.com/issue/IDEA-200500 for that.

Thanks for raising the issue with the IDEA team.

@k1w1m8
Copy link
Author

k1w1m8 commented Oct 15, 2018

NP.

Actually, I have raised several tickets with IDEA team about the latest EAP with JUnit, that was just one.

I also am about to raise a different set of tickets for latest Eclipse. Overall Eclipse seems to better but the treeview UI keeps expanding and contracting constantly which makes it pretty unusable.

I'm starting to get impostor syndrome about this - shouldn't someone from Core team be looking at how the major IDEs are going with JUnit5?

Do you think I should raise a JUnit ticket for IDE issues so the JUnit team can be aware of and track the issues together? Or maybe you have some other idea...

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

3 participants