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

Testing: allow bytecode transformations in QuarkusComponentTest #35473

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Aug 22, 2023

This commit refactors QuarkusComponentTestExtension to do the same class loader dance QuarkusTestExtension does. The test instance is created in a new, per-test class loader, and all test method invocations are forwarded to it. The new class loader is dropped at the end of the test.

A stack of test instances is maintained to support @Nested tests.

Supporting programmatically or declaratively defined config properties, programmatically configured mocks or "simplified" interceptor methods required inventing rather creative protocols for exchanging information between the two class loaders:

  1. For configuration, the map of config properties and the ordinal are passed from the original CL to the extra CL verbatim, because those are basic Java types that never exist in the extra CL.
  2. For mocks, the mock creation functions are registered in both CLs under deterministic keys, so registration in one CL always matches registration in the other CL.
  3. For interceptor methods, we collect all the necessary information in the original CL as a data structure composed solely from basic Java types, transfer that to the extra CL, and finish registration there.

Fixes #34926

@quarkus-bot quarkus-bot bot added area/arc Issue related to ARC (dependency injection) area/testing labels Aug 22, 2023
@Ladicek
Copy link
Contributor Author

Ladicek commented Aug 22, 2023

This seems to work, but is a little crazy, so I'm accepting all kinds of objections and civil disobedience :-)

I also need to check this a bit more with continuous testing.

@Ladicek
Copy link
Contributor Author

Ladicek commented Aug 22, 2023

Also, I have a similar change for ArcTestContainer locally. @mkouba @manovotn I'll leave it to your discretion whether we want bytecode transformation support there as well. It is not as crazy as this PR, because there's no SmallRye Config, Mockito, or simplified interceptor declaration, but it still requires the class loader dance.

I hope @holly-cummins can figure out a better solution, or I'll have to experiment with class loader hiearchies myself, and noone wants that 😆

@quarkus-bot

This comment has been minimized.

@Ladicek
Copy link
Contributor Author

Ladicek commented Aug 23, 2023

Ah interesting, I didn't know we have a test for continuous testing :-) I'll look.

This commit refactors `QuarkusComponentTestExtension` to do the same
class loader dance `QuarkusTestExtension` does. The test instance
is created in a new, per-test class loader, and all test method
invocations are forwarded to it. The new class loader is dropped
at the end of the test.

A stack of test instances is maintained to support `@Nested` tests.

Supporting programmatically or declaratively defined config properties,
programmatically configured mocks or "simplified" interceptor methods
required inventing rather creative protocols for exchanging information
between the two class loaders:

1. For configuration, the map of config properties and the ordinal
   are passed from the original CL to the extra CL verbatim, because
   those are basic Java types that never exist in the extra CL.
2. For mocks, the mock creation functions are registered in both
   CLs under deterministic keys, so registration in one CL always
   matches registration in the other CL.
3. For interceptor methods, we collect all the necessary information
   in the original CL as a data structure composed solely from
   basic Java types, transfer that to the extra CL, and finish
   registration there.
@Ladicek Ladicek force-pushed the component-test-bytecode-transformations branch from 3ac0ec3 to 7aee040 Compare August 24, 2023 14:44
@Ladicek
Copy link
Contributor Author

Ladicek commented Aug 24, 2023

Fixed the continuous testing scenario. That actually allowed some simplification of the component test extension -- since the ArC-generated classes only need to be loaded by the extra CL, there's no need to distinguish between continuous testing and regular testing when generating the classes.

@quarkus-bot
Copy link

quarkus-bot bot commented Aug 24, 2023

Failing Jobs - Building 7aee040

| Status | Name | Step | Failures | Logs | Raw logs | Build scan |
| :-: | -- | -- | :-: | :-: | :-: |
| ✔️ | Maven Tests - JDK 11 | | | | |
| ✖ | Maven Tests - JDK 11 Windows | Build | Failures | Logs | Raw logs |

Full information is available in the Build summary check run.

Failures

⚙️ Maven Tests - JDK 11 Windows #

📦 integration-tests/maven

io.quarkus.maven.it.JarRunnerIT.testPlatformPropertiesOverridenInApplicationProperties line 135 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.maven.it.JarRunnerIT that uses io.quarkus.maven.it.verifier.MavenProcessInvocationResult was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

io.quarkus.maven.it.JarRunnerIT.testPlatformPropertiesOverridenInApplicationProperties line 135 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.maven.it.JarRunnerIT that uses io.quarkus.maven.it.verifier.MavenProcessInvocationResult was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) area/testing triage/invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@QuarkusComponentTest feedback
1 participant