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

Introduce @DisabledInAotMode in the TestContext framework #30834

Closed
dsyer opened this issue Jul 7, 2023 · 13 comments
Closed

Introduce @DisabledInAotMode in the TestContext framework #30834

dsyer opened this issue Jul 7, 2023 · 13 comments
Assignees
Labels
in: test Issues in the test module theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Milestone

Comments

@dsyer
Copy link
Member

dsyer commented Jul 7, 2023

I guess technically @DisabledInNativeImage doesn't mean "disable AOT processing", but if the annotation is there, it's probably a sign that we don't expect AOT to work? So the stack traces (logged at WARN level) seem harsh, and hard to debug (there's no hint as to why it's there). Arguably it shouldn't even be a warning. To reproduce just add a @MockBean and @DisabledInNativeImage to a test and and try to run native tests. The test doesn't run but the exception is logged in the build.

Example from PetClinic:

2023-07-07T10:35:23.700+01:00  WARN 3778203 --- [           main] o.s.t.c.aot.TestContextAotGenerator      : Failed to generate AOT artifacts for test classes [org.springframework.samples.petclinic.vet.VetControllerTests]

org.springframework.test.context.aot.TestContextAotException: Failed to process test class [org.springframework.samples.petclinic.vet.VetControllerTests] for AOT
        at org.springframework.test.context.aot.TestContextAotGenerator.processAheadOfTime(TestContextAotGenerator.java:239) ~[spring-test-6.0.10.jar:6.0.10]
        at org.springframework.test.context.aot.TestContextAotGenerator.lambda$processAheadOfTime$4(TestContextAotGenerator.java:205) ~[spring-test-6.0.10.jar:6.0.10]
        at java.base/java.util.LinkedHashMap.forEach(LinkedHashMap.java:721) ~[na:na]
        at org.springframework.util.MultiValueMapAdapter.forEach(MultiValueMapAdapter.java:179) ~[spring-core-6.0.10.jar:6.0.10]
        at org.springframework.test.context.aot.TestContextAotGenerator.processAheadOfTime(TestContextAotGenerator.java:197) ~[spring-test-6.0.10.jar:6.0.10]
...
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 7, 2023
@bclozel bclozel transferred this issue from spring-projects/spring-boot Jul 7, 2023
@bclozel bclozel added in: test Issues in the test module theme: aot An issue related to Ahead-of-time processing labels Jul 7, 2023
@sbrannen
Copy link
Member

sbrannen commented Jul 7, 2023

At one point I proposed introducing a Spring-specific @DisabledInAot annotation, but the team decided we likely did not need that feature.

If we were to introduce such a @DisabledInAot annotation (that is potentially meta-annotated with JUnit Jupiter's @DisabledInNativeImage) and then skip AOT processing for any test class annotated with @DisabledInAot, would that suit your needs?

p.s. @DisabledInAot could also disable any test class/method when the spring.aot.enabled property is set to true -- which would disable the tests when running in AOT mode on the JVM.

@sbrannen sbrannen self-assigned this Jul 7, 2023
@sbrannen sbrannen added the status: waiting-for-feedback We need additional information before we can continue label Jul 7, 2023
@dsyer
Copy link
Member Author

dsyer commented Jul 7, 2023

@DisabledInAot would work I guess. I don't understand the comment about spring.aot.enabled - what would @DisabledInAot do if it wasn't going to disable the test?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 7, 2023
@sbrannen
Copy link
Member

sbrannen commented Jul 7, 2023

@DisabledInAot would work I guess.

Glad to hear it.

I don't understand the comment about spring.aot.enabled - what would @DisabledInAot do if it wasn't going to disable the test?

Sorry for being vague. I meant that @DisabledInAot would serve two roles.

  1. As a marker annotation to indicate that the annotated test class should not be processed for AOT optimizations -- thereby avoiding the build-time TestContextAotException you reported.
  2. As a JUnit Jupiter ExecutionCondition that disables the annotated test class or method when AotDetector.useGeneratedArtifacts() (which checks the spring.aot.enabled property) evaluates to true.

Does that make sense now?

If so, I think we should go with that.

@sbrannen
Copy link
Member

sbrannen commented Jul 7, 2023

Naming is hard.

Perhaps @DisabledInAot is not the best name for the annotation, since you cannot really be "in AOT".

Maybe @DisabledInAotMode is better.

The long-winded alternative is @DisabledWhenUsingAotGeneratedArtifacts which is technically correct, but I doubt anyone would favor that annotation name.

Thoughts?

@sbrannen sbrannen added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Jul 7, 2023
@sbrannen sbrannen changed the title Test AOT processing logs stack traces even when the test is disabled for native Introduce @DisabledInAotMode in the TestContext framework Jul 7, 2023
@dsyer
Copy link
Member Author

dsyer commented Jul 7, 2023

Makes sense (but not sure why you had to ask). I don't like the names much either. It's up to you, but I would go with the shortest one.

@sbrannen sbrannen added this to the 6.1.0-M3 milestone Jul 7, 2023
@snicoll
Copy link
Member

snicoll commented Jul 8, 2023

I am confused, I think. Can we take a step back and check why AOT processing for the test is failing in the first place?

@dsyer
Copy link
Member Author

dsyer commented Jul 8, 2023

It has a @MockBean which isn’t supported with AOT.

@sbrannen
Copy link
Member

sbrannen commented Jul 8, 2023

In addition to the @MockBean issue, this is a general concern.

Any time something simply does not work in AOT processing (for whatever reason), users currently have no way to disable/ignore a test that relies on the unsupported feature.

The end goal is of course to support everything in AOT. Similarly, GraalVM has the same goal for native image support. But there are things that currently do not work in a native image.

We created @DisabledInNativeImage exactly for such use cases, and @DisabledInAotMode would effectively address the same need for Spring AOT.

@sbrannen sbrannen removed this from the 6.1.0-M3 milestone Jul 11, 2023
@sbrannen
Copy link
Member

I guess technically @DisabledInNativeImage doesn't mean "disable AOT processing", but if the annotation is there, it's probably a sign that we don't expect AOT to work?

Not necessarily. A test may disabled in a native image due to compatibility issues specific to GraalVM native image; however, the same test may work fine with Spring AOT on the JVM. For that reason, we cannot infer that @DisabledInNativeImage means that AOT processing should be skipped.

As pointed out in this issue's description, if a test is annotated with @DisabledInNativeImage, it will not be executed with Spring AOT within a native image. So that is a viable solution to avoiding a test failure within a native image if the test's ApplicationContext could not be processed for AOT.

Similarly, if someone wishes to disable a test in AOT mode (on the JVM or within a native image), that is possible via a custom JUnit Jupiter ExecutionCondition such as the following.

@Target({ElementType.TYPE, ElementType.METHOD})
@Retention(RetentionPolicy.RUNTIME)
@Documented
@DisabledIf(value = "org.springframework.aot.AotDetector#useGeneratedArtifacts",
		disabledReason = "Disabled in Spring AOT mode")
public @interface DisabledInAotMode {
}

Note, however, that use of an ExecutionCondition like the above @DisabledInAotMode would not cause the test class to be skipped during AOT processing. Consequently, a warning would still be logged to notify the user of an AOT processing failure.

With regard to @MockBean and @SpyBean causing issues with AOT processing, that will need to be addressed in Spring Boot.

In light of the above, the team has decided not to introduce @DisabledInAotMode in spring-test.

@sbrannen sbrannen closed this as not planned Won't fix, can't repro, duplicate, stale Jul 11, 2023
@sbrannen sbrannen added the status: declined A suggestion or change that we don't feel we should currently apply label Jul 11, 2023
@sbrannen
Copy link
Member

If you're interested in seeing @MockBean and @SpyBean supported with Spring AOT and GraalVM native image, consider contributing a fix for spring-projects/spring-boot#32195.

@dsyer
Copy link
Member Author

dsyer commented Jul 11, 2023

Could we maybe not log the stack trace unless the user asks for it (requests DEBUG logs for example)?

@sbrannen
Copy link
Member

Could we maybe not log the stack trace unless the user asks for it (requests DEBUG logs for example)?

Yes, I'll address that in #30867.

@jhoeller jhoeller reopened this Oct 10, 2023
@sbrannen
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants