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

Improve coverage for JUnit @TestTemplate #40773

Merged
merged 2 commits into from
May 22, 2024

Conversation

holly-cummins
Copy link
Contributor

@holly-cummins holly-cummins commented May 22, 2024

#40601 exposed a JUnit bug and broke JUnit @TestTemplate tests in dev mode. How did that happen? Well, we didn't have any tests, so pretty easily. :)

This PR adds some coverage for these kinds of tests, so we should get earlier warning of issues like quarkiverse/quarkiverse#94 in future. At the moment, the test fails, so it's disabled, but we should re-enable it as soon as possible, for example, when #40751 (or its micro-release equivalent) merges.

This is the failure we see when the test is not disabled

2024-05-22 11:53:10,847 ERROR [io.qua.test] (Test runner thread) ==================== TEST REPORT #1 ====================
2024-05-22 11:53:10,847 ERROR [io.qua.test] (Test runner thread) Test TemplatedQuarkusTest#[@io.quarkus.test.junit.QuarkusTest()] failed 
: java.lang.NullPointerException: Cannot invoke "org.junit.platform.engine.UniqueId.toString()" because the return value of "org.junit.platform.launcher.TestIdentifier.access$800(org.junit.platform.launcher.TestIdentifier)" is null

The PR's a bit hard to follow from looking at the changes. This is what it's got in it:

  • A "simple" test where a test used a @TestTemplate and the supporting classes are in the same project. This test passes.
  • A slightly more complex test where we edit classes whose tests use a @TestTemplate. Sadly, tests don't get re-triggered in dev mode. Because of that, this test doesn't pass, but it's nothing to do with @dmlloyd's serialization work. I've disabled the test and raised JUnit TestTemplate tests are not re-run on code change in dev mode #40770.
  • A more complex test where the supporting classes come from an extension. That triggers a hop between classloaders, and so it doesn't work at the moment. I'm confident it would pass if Bump JUnit to 5.11.0-M2 #40751 was merged, and I think it would pass if Test framework: Use JBoss Marshalling cloner #40601 was reverted.
  • An existing test, relating to test templates, was checking whether bytecode changes made by an extension were visible in the test. (They aren't. That needs Draft: Load test classes with runtime classloader  #34681.) The name of that overlapped with the name of my new tests, so I've renamed it to make it more clear what scenario is being covered.

Note that we still don't have much coverage for some other JUnit advanced use cases, partly because some of the tests are there but failing, and partly because tests are missing.

This comment has been minimized.

@holly-cummins holly-cummins marked this pull request as draft May 22, 2024 14:00
@holly-cummins holly-cummins force-pushed the test-template-tests branch from b8b1069 to da65f40 Compare May 22, 2024 14:31
@holly-cummins holly-cummins marked this pull request as ready for review May 22, 2024 14:32
Copy link

quarkus-bot bot commented May 22, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit da65f40.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@gastaldi gastaldi merged commit b51310c into quarkusio:main May 22, 2024
33 of 35 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.12 - main milestone May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants