Skip to content

Make spring-test available to compile classpath of consumers of spring-boot-test #39901

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

Closed
xenoterracide opened this issue Mar 11, 2024 · 9 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@xenoterracide
Copy link
Contributor

/home/xeno/.gradle/caches/modules-2/files-2.1/org.springframework.boot/spring-boot-test-autoconfigure/3.2.3/ea2e1778142fc8a05bd9325b433b13fe0a6845c1/spring-boot-test-autoconfigure-3.2.3.jar(/org/springframework/boot/test/autoconfigure/orm/jpa/DataJpaTest.class): warning: Cannot find annotation method 'value()' in type 'BootstrapWith': class file for org.springframework.test.context.BootstrapWith not found

not doing anything special here

@DataJpaTest
public class JpaAggregateTest {

since you're not using standard library definitions, maybe you need something like optionalApi either way the gradle metadata generated could be fixed by having this exposed as some kind of api. At least I think, the whole optional thing makes me uncertain of what's being generated for Gradle's metadata, and I'm not looking at that atm.

https://github.com/spring-projects/spring-boot/blob/main/spring-boot-project/spring-boot-test-autoconfigure/build.gradle#L34

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 11, 2024
@mhalbritter
Copy link
Contributor

Hello! Do you happen to have a small reproducer for that?

@mhalbritter mhalbritter added the status: waiting-for-feedback We need additional information before we can continue label Mar 12, 2024
@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 Mar 12, 2024
@mhalbritter
Copy link
Contributor

There's a lot going on in your project. The smallest possible reproducer would help to diagnose this.

@mhalbritter mhalbritter added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Mar 13, 2024
@xenoterracide
Copy link
Contributor Author

I actually think it could probably be more minimal than this, I don't think the multi-module is necessary, but it took me a bit to get to here. The java compiler diags are what emits this.

the commented out implementations if uncommented make these warnings go away, proving that api would work.

I found an additional thing whilst creating this reproducer. Although requiring/providing spring-test for spring-boot-test-autoconfigure seems reasonable. spring data commons seems less so.

./gradlew build or minimally ./gradlew testClasses will emit the warnings

demo.tar.gz

@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 Mar 13, 2024
@mhalbritter
Copy link
Contributor

mhalbritter commented Mar 14, 2024

Thanks for the reproducer! Any reason why you don't use

testImplementation("org.springframework.boot:spring-boot-starter-test")

instead of spring-boot-test-autoconfigure?

But besides that, I think we should expose the class Gradle complains about as an API dependency.

@xenoterracide
Copy link
Contributor Author

transient dependency hell. Even if I used it, I set starters to "runtime only".

I've seen real world examples of people adding compile time dependencies on things that they shouldn't have, which made updates and refactors later harder. Starters make it easier to have this problem. Imagine the dependencies that have been removed from the spring stack since spring started, that have perhaps, accidentally ended up being a hard requirement because they were there and so used.

You should check this out https://github.com/autonomousapps/dependency-analysis-gradle-plugin

@wilkinsona
Copy link
Member

This problem isn't specific to Gradle.

You can see a similar warning with Maven when configured with the appropriate compiler arg:

[INFO] --- compiler:3.11.0:testCompile (default-testCompile) @ gh-39901 ---
[INFO] Changes detected - recompiling the module! :dependency
[INFO] Compiling 1 source file with javac [debug release 17] to target/test-classes
[WARNING] Cannot find annotation method 'value()' in type 'org.springframework.test.context.BootstrapWith': class file for org.springframework.test.context.BootstrapWith not found

This was generated from an app with this pom.xml:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
	xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
	<modelVersion>4.0.0</modelVersion>
	<parent>
		<groupId>org.springframework.boot</groupId>
		<artifactId>spring-boot-starter-parent</artifactId>
		<version>3.2.3</version>
		<relativePath/> <!-- lookup parent from repository -->
	</parent>
	<groupId>com.example</groupId>
	<artifactId>gh-39901</artifactId>
	<version>0.0.1-SNAPSHOT</version>
	<name>gh-39901</name>
	<description>Demo project for Spring Boot</description>
	<properties>
		<java.version>17</java.version>
	</properties>
	<dependencies>
		<dependency>
			<groupId>org.springframework.boot</groupId>
			<artifactId>spring-boot-starter</artifactId>
		</dependency>
		<dependency>
			<groupId>org.springframework.boot</groupId>
			<artifactId>spring-boot-starter-data-jpa</artifactId>
			<scope>runtime</scope>
		</dependency>
		<dependency>
			<groupId>org.springframework.boot</groupId>
			<artifactId>spring-boot-test-autoconfigure</artifactId>
			<scope>test</scope>
		</dependency>
		<dependency>
			<groupId>org.junit.jupiter</groupId>
			<artifactId>junit-jupiter</artifactId>
			<scope>test</scope>
		</dependency>
	</dependencies>

	<build>
		<plugins>
			<plugin>
				<groupId>org.springframework.boot</groupId>
				<artifactId>spring-boot-maven-plugin</artifactId>
			</plugin>
			<plugin>
				<groupId>org.apache.maven.plugins</groupId>
				<artifactId>maven-compiler-plugin</artifactId>
				<configuration>
					<compilerArgs>
						<arg>-Xlint:classfile</arg>
					</compilerArgs>
				</configuration>
			</plugin>
		</plugins>
	</build>

</project>

It also contains a single test class annotated with @DataJpaTest.

That said, it's considerably less likely to occur with Maven as Maven does not separate the test compile classpath from the test runtime classpath. In the pom above, there's no spring-test dependency at all so the tests fail to run. Add a test-scoped dependency on spring-test removes the compiler warning and allows the tests to run.

But besides that, I think we should expose the class Gradle complains about as an API dependency

This is always tricky with optional dependencies. The most elegant solution to this problem is Gradle's feature variants. We haven't embraced them yet as there's no good equivalent on the Maven side so we're left with the optional approach for now.

In this case, when using optional dependencies as a solution, the question we have to answer is whether or not we think anyone should be using spring-boot-test-autoconfigure without spring-test. At the moment, the optional nature of its spring-test dependency means that we think the answer is yes. The knock-on effect of this is that anyone who wants to use a spring-test related feature of spring-boot-test-autoconfigure has to ensure that spring-test is on the classpath. For most people, spring-boot-starter-test achieves that but it doesn't help those who choose not to use it.

Looking through the public features of spring-boot-test-autoconfigure, I'm struggling to find one that doesn't require spring-test. This suggests that it should be a transitive dependency. We'd then have to decide if it should be a compile dependency or runtime dependency. Runtime would meet most people's needs, but not those compiling with -Xlint:classfile and striving to have no warnings.

We may want to perform a similar analysis of spring-boot-test as its spring-test dependency is also optional.

@wilkinsona wilkinsona added for: team-meeting An issue we'd like to discuss as a team to make progress and removed status: feedback-provided Feedback has been provided labels Mar 15, 2024
@xenoterracide
Copy link
Contributor Author

xenoterracide commented Mar 15, 2024

It's not specific to Gradle know, however I don't believe Maven has the tooling that Gradle has to deal with it. Essentially Maven has no equivalent to api() as I recall. I personally am of the opinion that just because Maven can't do it doesn't mean that you should not let Gradle based consumers do it. Of course the inverse is also true as there are a few rare things that Gradle doesn't seem to be able to do.

@philwebb philwebb changed the title expose BootstrapWith using gradle's api( expose BootstrapWith using gradle's 'api' Mar 20, 2024
@philwebb philwebb added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged for: team-meeting An issue we'd like to discuss as a team to make progress labels Mar 20, 2024
@philwebb philwebb added this to the 3.3.x milestone Mar 20, 2024
@philwebb
Copy link
Member

We're going to add an api dependency in spring-boot-test to spring-test which should fix this for both Maven and Gradle.

@philwebb philwebb changed the title expose BootstrapWith using gradle's 'api' Make spring-test available to compile classpath of consumers of spring-boot-test Mar 20, 2024
@mhalbritter mhalbritter self-assigned this Mar 26, 2024
@mhalbritter mhalbritter modified the milestones: 3.3.x, 3.3.0-RC1 Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants