Skip to content

Conversation

@mstahv
Copy link
Contributor

@mstahv mstahv commented Dec 19, 2024

I'm a big fan of setting up the developent time datasource with Testcontainers. The problem I see people checking out and trying my examples/demos is that they are not (yet) familiar with the idea of starting the development time server from the srs/test/java.

Similarly to which DataSourceBeanCreationFailureAnalyzer currently hints about embedded databases, I think in 2024 it should hint somehow about Testcontainers too. Probably just guiding them to seek an application class from the test sources is enough (like currently in this PR), but maybe it should mention e.g. mvn spring-boot:test-run 🤷‍♂️

For one of my recent demos I added an app specific FailureAnalyzer, but I don't think that is a good approach.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 19, 2024
@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Dec 20, 2024
@philwebb
Copy link
Member

philwebb commented Jan 6, 2025

Thanks for the suggestion, but we're not keen to add this link because there are many other services as well as databases that could be started via Testcontainers. We also don't have a good way of detecting the test main class when running just the main application.

@philwebb philwebb closed this Jan 6, 2025
@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply 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 Jan 6, 2025
@mstahv
Copy link
Contributor Author

mstahv commented Jan 6, 2025

How are the "many other services" related to missing DataSource🤔 Would be indeed nice if this message could be hidden if already starting with test classpath, as then it is obviously not valid. For a moment I considered checking if some classes from JUnit/TestNG are available, but then though that also the existing hint about embedded databases is (most?) often invalid.

@philwebb
Copy link
Member

philwebb commented Jan 6, 2025

How are the "many other services" related to missing DataSource

By other services, I mean any other Testcontainer that we have ConnectionDetails support for. E.g. Redis. i.e. the problem is larger than just datasources.

For a moment I considered checking if some classes from JUnit/TestNG are available

That's probably not going to work for this situation because the IDE usually doesn't include test dependencies when running the main code. I don't think we have a good way of telling that there is another class with an alternative main method available under src/test/java.

That only leaves us with the option of always suggesting applications might want to be starting from the test sources, but we're really not keen on that.

@mstahv
Copy link
Contributor Author

mstahv commented Jan 6, 2025

Ah, so you would wish that the hint would be in some more generic place than in DataSource related analyzer👍 Don't have good ideas for that.

Yes, I know how IDEs regarding test/main dependencies, but checking for non-exisiting "org.junit.jupiter.api.Test" (or something similar like "SpringBootTest") works pretty well as the hint could be hidden if already runnign from the test sources (and the issue is then probably somewhere else).

 public class TestDemoApplication {

	public static void main(String[] args) {
		if (checkTestClassAvailable()) {
			System.out.println("You have JUnit 5 on the classpath, running from test sources hint should be hidden.");
		}
		SpringApplication.from(DemoApplication::main).with(TestcontainersConfiguration.class).run(args);
	}

	private static boolean checkTestClassAvailable() {
		try {
			Class.forName("org.junit.jupiter.api.Test");
			return true;
		} catch (ClassNotFoundException e) {
			return false;
		}
	}

}

But sure, I agree the hint can't be made perfect, but I think it might help many newbies in the same way as the current embedded database hint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: declined A suggestion or change that we don't feel we should currently apply

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants