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

JUnit 5 Test Suite not working - "No tests found" #2462

Closed
Phillipus opened this issue Jun 1, 2023 · 20 comments · Fixed by #3358
Closed

JUnit 5 Test Suite not working - "No tests found" #2462

Phillipus opened this issue Jun 1, 2023 · 20 comments · Fixed by #3358

Comments

@Phillipus
Copy link

Phillipus commented Jun 1, 2023

I'm trying to execute a JUnit 5 Test Suite using Tycho Surefire. The suite is this:

@Suite
@SelectClasses({
    FooTest1.class,
    FooTest2.class
})

@SuiteDisplayName("All Tests")
public class AllTests {
}

This runs in Eclipse itself, but not using Maven/Tycho. Error is "Failed to execute goal org.eclipse.tycho:tycho-surefire-plugin:3.0.4:test (default-test) on project test.package.tests: No tests found."

Attached is a simple test project. Run with mvn clean verify

Tested on Tycho 3.0.4 and 4.0.0-SNAPSHOT

test.zip

@Phillipus
Copy link
Author

If I place a dummy test inside the AllTests class body it runs that test so it seems to be ignoring the @Suite annotation.

@Suite
@SelectClasses({
    FooTest1.class,
    FooTest2.class
})

@SuiteDisplayName("All Tests")
public class AllTests {

    @Test
    public void foo() {
    }
    
}

@ptziegler
Copy link
Contributor

ptziegler commented Sep 15, 2023

I had a brief look at it the other day, because we ran into a similar situation and I think there are two reasons why the JUnit 5 test suites don't work:

  1. Test suites are executed via the SuiteTestEngine, which is provided by platform-suite-engine. This bundle is not included in the embedded JUnit5 runner (see org.eclipse.tycho.surefire.junit59).

  2. Even if this bundle is added, the services are not merged correctly. Thus, the test engine is not discovered via SPI. Services are set manually, but right now only the "normal" JupiterTestEngine is defined.

@laeubi
Copy link
Member

laeubi commented Sep 18, 2023

As always it would be great to provide an integration-test to demonstrate the issue.

Nerveless one main problem might be that Tycho has a pre-check that, if triggered, skips the execution all together, so the first step should be to check if the suite is detected at that stage already.

@Phillipus
Copy link
Author

As always it would be great to provide an integration-test to demonstrate the issue.

I tried, I really did, but gave up in sheer frustration in getting the workflow to run.

#2469

@ptziegler
Copy link
Contributor

ptziegler commented Sep 18, 2023

I can have a look at it this afternoon as well. Given that I played around with it for a little bit, there should be a small test project I can add to the integration tests.

Nerveless one main problem might be that Tycho has a pre-check that, if triggered, skips the execution all together, so the first step should be to check if the suite is detected at that stage already.

In short: The problem is that the SuiteTestEngine (or any of the JUnit suite jarts for that matter), which is required to handle those Suite annotations, is not included in the org.eclipse.tycho.surefire.junit59 bundle.
From what I understand, there was a similar issue with executing JUnit3 and JUnit4 tests with the JUnit5 runtime, which is why Tycho also has the org.eclipse.tycho.surefire.junit59withvintage bundle.
Depending on whether JUnit4 is in the classpath, one of those bundles is added to the test environment.
A dirty fix would probably be to also add a -junit59withsuite bundle, which is chosen whenever junit-platform-suite-api is part of the classpath. But I like to think there is a better solution, because then you'd also have to consider the case where you'd want to use both the vintage and the suite test engine...

@laeubi
Copy link
Member

laeubi commented Sep 18, 2023

@Phillipus if you are a first-time contributor workflows need approval from a committer, in such case simply ask on the PR and I'll take a look at it. In general if existing documentation is not sufficient please let us know so we can improve those:

@laeubi
Copy link
Member

laeubi commented Sep 18, 2023

@ptziegler yes the existing tycho-surefire way of adding test dependencies is rather verbose and cumbersome.
You might want to try out the new bnd-test way as described here:

it should hopefully find/pull everything automatically.

If there is any feature missing with that please let me know at best
provide an integration-test to demonstrate the missing feature with a demo as we also test the demo project.

@laeubi
Copy link
Member

laeubi commented Jan 10, 2024

This is most probably related to

HeikoKlare added a commit to HeikoKlare/tycho that referenced this issue Jan 10, 2024
@HeikoKlare
Copy link
Contributor

This is most probably related to

I am not into the details, but #3247 refers to test selection patterns, which I would expect to be independent of whether a single test or a test suite is executed. For that reason, I am not sure whether this issue is actually related.
As mentioned in #2462 (comment), I would rather expect the problem to be the missing suite engine.

I have now provided an integration test in #3342, which fails as expected because no tests are found when executing a test suite:

Failed to execute goal org.eclipse.tycho:tycho-surefire-plugin:5.0.0-SNAPSHOT:test (default-test) on project bundle.test.junit5suite: No tests found -> [Help 1]

@laeubi
Copy link
Member

laeubi commented Jan 10, 2024

I am not into the details, but #3247 refers to test selection patterns, which I would expect to be independent of whether a single test or a test suite is executed. For that reason, I am not sure whether this issue is actually related.
As mentioned in #2462 (comment), I would rather expect the problem to be the missing suite engine.

It all relates a bit, maybe its enough to add some more dependencies but in the end we should not need all this as surefires seems to be able to do this successful already, so we should adopt to it as much as possible.

Nevertheless do you plan to debug/work on this?

@HeikoKlare
Copy link
Contributor

It all relates a bit, maybe its enough to add some more dependencies but in the end we should not need all this as surefires seems to be able to do this successful already, so we should adopt to it as much as possible.

I have now digged a big into the issue and think you are right. With a pure Maven setup, test suites are executed as long as junit-platform-suite-engine is on the classpath (see the example I have provided for comparison at the integration test PR: #3342 (comment)). Even the generated surefire.properties look quite fine, in particular they contain an entry for the suite to be executed, such as tc.0=bundle.test.SuiteWithAllTests. The difference seems to be in how the OSGiSurefireBooter and the ForkedBooter set up the provider, which is why #3247 is related, as you mentioned.

Nevertheless do you plan to debug/work on this?

I would really like to do so, but after I now had a first look into the code I think that due to lack of experience with the technologies it will probably take too long for me to achieve an "acceptable cost/benefit ratio". But of course I am aware that no one else will work on the issue as long as it's not pressing for them.
@laeubi Independent from this issue, do you plan to work on #3247 (soon)? It looks that it makes sense to address that issue first, because it will affect this suite issue (or maybe even solve it implicitly).

@laeubi
Copy link
Member

laeubi commented Jan 11, 2024

It all depends a bit how important that one becomes, I can try to spend some time on it as part of the platform-releng (if it is important for platform...)

@akurtakov
Copy link
Member

Being able to use latest JUnit is important in general but when there are people (like @HeikoKlare ) actively improving the tests it becomes even more IMHO.

@HeikoKlare
Copy link
Contributor

I see several reasons why platform tests should use latest JUnit (capabilities):

  • Getting rid of outdated patterns imposed by JUnit 3. This already improved test stability in the platform.
  • Validating that things work with current testing technologies (which we can then find before adopters face them). In particular this one also applies to Tycho (surefire).
  • Making the project (look) more modern. This is minor, but we also have some test frameworks (such as the session tests), in which contributors (including me) will be more willing to debug issues if the frameworks use latest technology instead of JUnit 3, as you then might learn something valuable (which is more unlikely when debugging things based on JUnit 3).

With respect to this suites issue, I think that I can also work around that for some time by (temporarily) replacing test suites with include/exclude definitions for Tycho surefire (particularly required to not execute performance tests). Existing projects in Platform UI using JUnit 5 do the same: in Tycho builds, they simply execute all available tests and ignore the provided JUnit 5 test suite. Anyway, that should not be the final solution but only ensure that missing test suite support in JUnit 5 is not a blocker for migration.

@laeubi
Copy link
Member

laeubi commented Jan 11, 2024

Actually these suite where more a workaround for me because of the crippled ant based test execution that don't allow to select test by a pattern, but you are right it should work of course.

Skipping tests is also better handled by assumptions than by test suites or filter patterns.

@HeikoKlare
Copy link
Contributor

Skipping tests is also better handled by assumptions than by test suites or filter patterns.

Agreed. In my opinion, test suites should actually only be an (optional) means of structuring, not for filtering.

Further Insights

Here are some more insights from further debugging the issue, which might help for further processing.

To ensure that the SuiteTestEngine is on the classpath and registered as a TestEngine service, I have added the junit-platform-suite-engine to the o.e.tycho.surefire.junit59 and o.e.tycho.surefire.junit59withvintage bundles and added the SuiteTestEngine to the TestEngine service specifications.

Then I found the following things worth to document:

So from my understanding the code should automatically load and execute the SuiteTestEngine if it's on the classpath. Since there are several different class loaders involved and swapped around, I would expect the one finally used to locate the engines does, somehow, not have junit-platform-suite-engine on its classpath. But unfortunately I was not yet able to debug this further.

It can also be seen that the ServiceLoader loads the engines from the classpath by setting the classLoaderOrder in the test properties to testProbeFirst. Then the combined class loader will use the test plugin's class loader first:

case "testProbeFirst":
loaders = new ClassLoader[] { testClassLoader, surefireClassLoader, contextClassLoader };

Then the ServiceLoader finds the SuiteTestEngine but throws an error because the ServiceLoader cannot assign it to TestEngine (probably because the class loaders use different / incompatible versions of JUnit platform).

HeikoKlare added a commit to HeikoKlare/tycho that referenced this issue Jan 12, 2024
Currently, Tycho is not capable of running JUnit 5 test suites, since
the requires SuiteTestEngine is not provided while running tests.

This change adds the SuiteTestEngine to tycho-surefire executions using
JUnit Platform 1.8 (the first with suite support) or newer. To this end,
the SuiteTestEngine is always enabled when running JUnit 5 tests. It
also adds a regression test for executing a test suite based on JUnit
Jupiter 5.9.

Fixes eclipse-tycho#2462
HeikoKlare added a commit to HeikoKlare/tycho that referenced this issue Jan 12, 2024
Currently, Tycho is not capable of running JUnit 5 test suites, since
the required SuiteTestEngine is not provided while running tests.

This change adds the SuiteTestEngine to tycho-surefire executions using
JUnit Platform 1.8 (the first with suite support) or newer. To this end,
the SuiteTestEngine is always enabled when running JUnit 5 tests. It
also adds a regression test for executing a test suite based on JUnit
Jupiter 5.9.

Fixes eclipse-tycho#2462
HeikoKlare added a commit to HeikoKlare/tycho that referenced this issue Jan 12, 2024
Currently, Tycho is not capable of running JUnit 5 test suites, since
the required SuiteTestEngine is not provided while running tests.

This change adds the SuiteTestEngine to tycho-surefire executions using
JUnit Platform 1.8 (the first with suite support) or newer. To this end,
the SuiteTestEngine is always enabled when running JUnit 5 tests. It
also adds a regression test for executing a test suite based on JUnit
Jupiter 5.9.

Fixes eclipse-tycho#2462
@HeikoKlare
Copy link
Contributor

I finally found the reason and a solution for missing suite support. It is actually only a configuration problem. Properly registering the SuiteTestEngine in the junit-specific fragments is sufficient, see #3358

HeikoKlare added a commit to HeikoKlare/tycho that referenced this issue Jan 12, 2024
Currently, Tycho is not capable of running JUnit 5 test suites, since
the required SuiteTestEngine is not provided while running tests.

This change adds the SuiteTestEngine to tycho-surefire executions using
JUnit Platform 1.8 (the first with suite support) or newer. To this end,
the SuiteTestEngine is always enabled when running JUnit 5 tests. It
also adds a regression test for executing a test suite based on JUnit
Jupiter 5.9.

Fixes eclipse-tycho#2462
github-actions bot pushed a commit that referenced this issue Jan 15, 2024
Currently, Tycho is not capable of running JUnit 5 test suites, since
the required SuiteTestEngine is not provided while running tests.

This change adds the SuiteTestEngine to tycho-surefire executions using
JUnit Platform 1.8 (the first with suite support) or newer. To this end,
the SuiteTestEngine is always enabled when running JUnit 5 tests. It
also adds a regression test for executing a test suite based on JUnit
Jupiter 5.9.

Fixes #2462

(cherry picked from commit 82dd36b)
@Phillipus
Copy link
Author

Thanks to everyone who worked on this, especially @HeikoKlare for running with it. :-)

What little I can offer in return is some immediate user feedback:

  1. I checked out the latest source from the Tycho repo
  2. Compiled and installed latest Tycho with mvn clean install -DskipTests
  3. Edited the maven.config file in my project to include the line -Dtycho-version=5.0.0-SNAPSHOT
  4. Ran mvn clean verify on my project containing a Junit 5 Test Suite
  5. All tests passed!

👍

@HeikoKlare
Copy link
Contributor

Thanks for the feedback, @Phillipus! Great to hear that you could verify that it works as expected.

laeubi pushed a commit that referenced this issue Jan 15, 2024
Currently, Tycho is not capable of running JUnit 5 test suites, since
the required SuiteTestEngine is not provided while running tests.

This change adds the SuiteTestEngine to tycho-surefire executions using
JUnit Platform 1.8 (the first with suite support) or newer. To this end,
the SuiteTestEngine is always enabled when running JUnit 5 tests. It
also adds a regression test for executing a test suite based on JUnit
Jupiter 5.9.

Fixes #2462

(cherry picked from commit 82dd36b)
@Phillipus
Copy link
Author

@SelectClasses is working with this fix, however @SelectPackages is not working. See #3709

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants