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

Add testcase for dynamic tests and Jackson classloading #43766

Conversation

holly-cummins
Copy link
Contributor

@holly-cummins holly-cummins commented Oct 8, 2024

#10435 shared a reproducer, but we didn't include the reproducer in our codebase when we did the fix. This strikes me as the kind of thing that could regress, so I've added it. I couldn't definitively confirm that the reproducer, as I've added it to the codebase, fails on 1.15.2.Final, because the shape of our codebase has changed too much. The risk is that classloading is sometimes different in a @QuarkusTest executed as part of the main build and one done in a standalone resources project, and this is a classloading problem. But the standalone sub-projects are expensive to execute, and since we didn't even have a test before, I didn't want to slow down our build too much, especially if the top-level test actually would catch the problem.

I could confirm that in a standalone project, the reproducer test case failed for 1.15.2.Final and passes now, which I think is a good enough level of confidence in the test case.

Fixes #10435 (technically speaking, it doesn't, since #10435 is already fixed, but this adds a test case, which means there's no barrier at all to closing #10435 once this merges).

Copy link

quarkus-bot bot commented Oct 8, 2024

Thanks for your pull request!

Your pull request does not follow our editorial rules. Could you have a look?

  • title should not contain an issue number (use Fix #1234 in the description instead)

This message is automatically generated by a bot.

@holly-cummins holly-cummins requested a review from geoand October 8, 2024 09:51
@holly-cummins holly-cummins changed the title Add testcase for #10435 Add testcase for dynamic tests and Jackson classloading Oct 8, 2024
@quarkus-bot quarkus-bot bot added the area/jackson Issues related to Jackson (JSON library) label Oct 8, 2024
@holly-cummins holly-cummins force-pushed the reproducer-for-jackson-tccl-dynamic-tests branch from 58a2e3c to c9941d0 Compare October 8, 2024 09:52
Copy link

quarkus-bot bot commented Oct 8, 2024

Status for workflow Quarkus CI

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

✅ 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.

@geoand geoand merged commit aab06ab into quarkusio:main Oct 8, 2024
20 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.16 - main milestone Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Misconfigured TCCL for @TestFactory dynamic tests breaks RestAssured since Quarkus 1.5.0
2 participants