Junit5: Dynamic children of TestDescriptor with Type.CONTAINER_AND_TEST are not registered correctly

[jlink]

For some strange reason, which I couldn't figure out yet, trying to dynamically register children of a descriptor with type Type.CONTAINER_AND_TEST will register them (read: show them in the test plan) as children of the container above and not the intended parent. Changing type to Type.CONTAINER and also implementing

boolean hasTests() { return true; }

fixes the symptom.

I've observed the phenomenon in IDEA's test view but assume it's a general problem.

[sormuras]

I've observed the phenomenon in IDEA's test view but assume it's a general problem.

IDEA view is based on M3, afaik. Let's either wait until they adopt to M4 (soon TM) or we use the console launcher tree output as reference.

Will look into the topic next week.

[jlink]

OK.

The error occurrs if something is both a test and a container.

[sormuras]

@jlink IDEA 2017.1 already supports M4. See how https://youtrack.jetbrains.com/issue/IDEA-170817#comment=27-2081280

[jlink]

I also noticed that IDEA does not propagate test failure display to parent descriptors when the descriptor type is CONTAINER_AND_TEST. I guess this is a related issued.

[sormuras]

Work in progress... see https://github.com/junit-team/junit5/compare/issues/756_container_and_test

At the current implemetation state, it seems like an issue within IDEA. Not 100% confirmed, though.

[marcphilipp]

Ok, so @sormuras and I wrote some additional tests and tried to reproduce this bug. However, everything worked as expected.

As an experiment, I changed getType() of TestFactoryTestDescriptor to return CONTAINER_AND_TEST and executed the following test in IntelliJ and using the Gradle plugin:

class BugDemo {
    @TestFactory
    Collection<DynamicTest> dynamicTestsFromCollection() {
        return Arrays.asList(
            dynamicTest("1st dynamic test", () -> assertTrue(true)),
            dynamicTest("2nd dynamic test", () -> assertEquals(4, 2 * 2))
        );
    }   
}

The Gradle plugin produced the expected result:

│  ├─ BugDemo ✔
│  │  └─ dynamicTestsFromCollection() ✔
│  │     ├─ 1st dynamic test ✔
│  │     └─ 2nd dynamic test ✔

However, in IntelliJ the tree looks like this:

It seems that the first dynamic test is completely omitted and the 2nd one is displayed using the wrong parent.

I registered a slightly modified version of our LoggingListener and verified that TestExectionListeners receive the correct events in the correct order:

TestPlan Execution Started: org.junit.platform.launcher.TestPlan@e6ea0c6
Execution Started: JUnit Jupiter - [engine:junit-jupiter]
Execution Started: BugDemo - [engine:junit-jupiter]/[class:example.BugDemo]
Execution Started: dynamicTestsFromCollection() - [engine:junit-jupiter]/[class:example.BugDemo]/[test-factory:dynamicTestsFromCollection()]
Dynamic Test Registered: 1st dynamic test - [engine:junit-jupiter]/[class:example.BugDemo]/[test-factory:dynamicTestsFromCollection()]/[dynamic-test:#1] (parent: Optional[[engine:junit-jupiter]/[class:example.BugDemo]/[test-factory:dynamicTestsFromCollection()]])
Execution Started: 1st dynamic test - [engine:junit-jupiter]/[class:example.BugDemo]/[test-factory:dynamicTestsFromCollection()]/[dynamic-test:#1]
Execution Finished: 1st dynamic test - [engine:junit-jupiter]/[class:example.BugDemo]/[test-factory:dynamicTestsFromCollection()]/[dynamic-test:#1] - TestExecutionResult [status = SUCCESSFUL, throwable = null]
Dynamic Test Registered: 2nd dynamic test - [engine:junit-jupiter]/[class:example.BugDemo]/[test-factory:dynamicTestsFromCollection()]/[dynamic-test:#2] (parent: Optional[[engine:junit-jupiter]/[class:example.BugDemo]/[test-factory:dynamicTestsFromCollection()]])
Execution Started: 2nd dynamic test - [engine:junit-jupiter]/[class:example.BugDemo]/[test-factory:dynamicTestsFromCollection()]/[dynamic-test:#2]
Execution Finished: 2nd dynamic test - [engine:junit-jupiter]/[class:example.BugDemo]/[test-factory:dynamicTestsFromCollection()]/[dynamic-test:#2] - TestExecutionResult [status = SUCCESSFUL, throwable = null]
Execution Finished: dynamicTestsFromCollection() - [engine:junit-jupiter]/[class:example.BugDemo]/[test-factory:dynamicTestsFromCollection()] - TestExecutionResult [status = SUCCESSFUL, throwable = null]
Execution Finished: BugDemo - [engine:junit-jupiter]/[class:example.BugDemo] - TestExecutionResult [status = SUCCESSFUL, throwable = null]
Execution Finished: JUnit Jupiter - [engine:junit-jupiter] - TestExecutionResult [status = SUCCESSFUL, throwable = null]
Execution Started: JUnit Vintage - [engine:junit-vintage]
Execution Finished: JUnit Vintage - [engine:junit-vintage] - TestExecutionResult [status = SUCCESSFUL, throwable = null]
TestPlan Execution Finished: org.junit.platform.launcher.TestPlan@e6ea0c6

As you can see, in particular in the lines that start with Dynamic Test Registered, the dynamic tests have the correct parent.

---

@akozlova Is there any code in IntelliJ that assumes that a single TestIdentifier can never return true for both isTest() and isContainer()?

[marcphilipp]

@akozlova Looks like this would be on of those places:
https://github.com/JetBrains/intellij-community/blob/73e5af46ecfb3c284713b8505da0c97eba878a5b/plugins/junit5_rt/src/com/intellij/junit5/JUnit5TestExecutionListener.java#L113

[akozlova]

Yes, I didn't change the code since the beginning as I am waiting here for the stable version, sorry. Is it supposed to be stable now?

[marcphilipp]

As far as I can predict the future, it will be stable as soon as we ship M5. 🙂

[marcphilipp]

@akozlova Shall we open a ticket over at YouTrack or are you already working on it?

[marcphilipp]

Closing this issue as it needs to be addressed by the IDEA integration.

[akozlova]

Looks fine to me:

dynamicTestsFromCollection#isTest == false though. What should I check else? Thanks

[sormuras]

Did you "_change getType() of TestFactoryTestDescriptor to return CONTAINER_AND_TEST_" as Marc mentioned here?

[marcphilipp]

@akozlova Please let us know if we should provide an example engine that does not require you to change our code to reproduce this issue.

[akozlova]

Oh, sorry! I forgot to do the modification. No need to provide an engine, I run tests inside junit 5 project itself, should be easy to me to change the code. Sorry for disturbing!