-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
"CAMEL-13454:camel-testcontainers - Should build if no docker" #3063
Conversation
components/camel-consul/src/test/java/org/apache/camel/component/consul/ConsulRegistryTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure this resolves the situation
components/camel-consul/src/test/java/org/apache/camel/component/consul/ConsulRegistryTest.java
Show resolved
Hide resolved
components/camel-consul/src/test/java/org/apache/camel/component/consul/ConsulRegistryTest.java
Show resolved
Hide resolved
|
I need to test this locally. |
@davsclaus can you double check on your Mac? |
This doesn't work without docker-machine
|
Tests will be skipped even with docker.service running. My proposal is to drop this PR, revert to the original situation and find a different way. |
-1 for me. |
Yeah I have suggested the validation using DockerMachineClient and this check really works only for Windows and Mac, where docker-machine is installed with toolbox. Sorry about that. I was looking into code of testcontainers and havent found a nice way to detect this. The problem is that testcontainers does environment validation in constructor, and not later in start phase. The relevant stale issue is there: testcontainers/testcontainers-java#343 , but there is no conclusion. To do this validation reliably requires a lot of changes in testcontainers. The simple approach could be just wrap it into try-catch, but this is ugly and totally unsafe, because it throws |
If this could work on all the OS I would be more than happy to merge, but in this way, this will work only on Windows and Mac and we are in a chicken-and-egg situation again :-D |
Maybe we could have an activation based on a property/env and win/mac people can add it to maven settings or env var and we should push testcontainers guys to provide a cross platform solution |
btw, does not work for me too |
We can also use maven os plugin, as we did for grpc tests enabling only on supported platforms, however the platforms list was slightly wide. |
ok Thanks for all reviewing this. i am closing this PR and revert back to old implementation having maven profiles to skip tests |
"CAMEL-13454:camel-testcontainers - Should build if no docker"