When docker isnt present

Would be nice if one could tell the subclass of GenericContainer that one is assume-ing that docker is present. So that if someone that doesn't have docker, get ignored tests instead of failing tests.

Due to the fact
DockerClient is instantiated during of GenericContainer. One has no ability to use the jUnit way of conditionally ignoring tests with assumThat and AssumptionExceptionViolation.

Instead one gets:

java.lang.ExceptionInInitializerError
	at sun.misc.Unsafe.ensureClassInitialized(Native Method)
	at sun.reflect.UnsafeFieldAccessorFactory.newFieldAccessor(UnsafeFieldAccessorFactory.java:43)
	at sun.reflect.ReflectionFactory.newFieldAccessor(ReflectionFactory.java:142)
	at java.lang.reflect.Field.acquireFieldAccessor(Field.java:1088)
	at java.lang.reflect.Field.getFieldAccessor(Field.java:1069)
	at java.lang.reflect.Field.get(Field.java:393)
	at org.junit.runners.model.FrameworkField.get(FrameworkField.java:73)
	at org.junit.runners.model.TestClass.getAnnotatedFieldValues(TestClass.java:230)
	at org.junit.runners.ParentRunner.classRules(ParentRunner.java:255)
	at org.junit.runners.ParentRunner.withClassRules(ParentRunner.java:244)
	at org.junit.runners.ParentRunner.classBlock(ParentRunner.java:194)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:362)

So create the DockerClient later should make it possible to use assumptions if one would like to.

Also noticed that there is a getName after setting name that causes issues with the creation of the object. One would say it's doing to much in its construction, there is a "start" phase.

Some discussion before I implement and PR?

IMO letting the test fail is the correct behavior.

If the test was actually to run docker I would agree.
But having issues with docker doesn't actually tell you anything about your actual test.

As an example. I'm using JDBCContainer to spin up a mysql db and then performing some operations.
I verify that said operations have resulted in a specific state.

Should I assume that the operations are failing which is the actual test?

jUnit Assume would mark them as ignored and as such would not require everyone working with the project to have docker installed. You just make sure that the CI env has docker.

@kiview

Thinking the same way: what if MySQL is not available for any reason ? Do you expect your tests to fail or to be ignored ?

I agree with @kiview that failure is the correct behavior.

Might have been unclear, since I don't want a breaking change I just want the ability to do something like:

@ClassRule
MysqlContainer mysqlContainer = new MysqlContainer().assumeDocker();

And this is since there is no good way of ignoring test that you know are going to fail.

The whole idea behind assumptions is that tests should not fail just because you made an assumption. They will not pass, they will not fail.

And adding this to ones project just makes the assumption that people aren't on Windows and that everybody has docker installed. (it does work on windows with limitation which seesm to be related to Compose, got the information from the slack channel)

Letting me as the user control this assumption would be a great thing IMO.

If the ability to assumeDocker or ability to create assumptions easy on your own is not wanted this can be closed.

I however strongly feel that failing a DAO test because of container failure or missing docker is wrong.
Since SUT(System/subject under test) is the DAO and not the testcontainers/docker.
Had it been a manual test, it would has been marked as Not run aka Ignored. Ignored is an unknown state in regards to the SUT. Fail tests where the SUT hasn't failed. Test results should report state of SUT and not the test and/or environment.

@rnorth

Let's wait for the opinion of @rnorth.

Thanks all, and I'm sorry I haven't replied on this thread until now. It's an interesting suggestion; I've used Assume once or twice but not come across use in the wild much, so you're the first person to ask for this. I can see how you'd find this useful, though. I can also see how Testcontainers has unique information about the Docker environment that is useful for such a Assumption. It won't be something that everyone wants to use, so I'd like to find a way to do it that's not too far-reaching. How about this: Could we perhaps proceed to do this separately from the container classes? E.g. Create something to allow either `assumeTrue(DockerClientFactory.isDockerAvailable())`, or `DockerAvailableAssumption.assumeDockerAvailable()`? Another reason for keeping it off the container classes is that I'd like to separate out any JUnit coupling from the container classes, so adding an assume method right now would seem to be a step in the wrong direction. Thoughts on the above approach? Richard

@rnorth

@rnorth
1, Due to the creation of DockerClient during the construction of the class, it sort of falls on TestContainer to support this.
2, I've already implemented this it's with an added fluent .assumeDocker() so it's opt-in
3, The container classes already implements TestRule from jUnit, so it's already coupled.
Since assume is just more or less a bubbling exception i just added a new "phase" in FailureDetectingExternalResource which is assumptions which is called before start. I was thinking of just using a list of Functions so that assumptions could be added willy-nilly. Just to prevent the start from happening (avoid starting the container realize that the jdbc-driver is missing retry for 120s and so on and so on)

Since I have my fork and jitpack exists, this is not much of an issue.

@rnorth

@rnorth @kiview @martin-g

SimpleMariaDBTest.java

Depends on what is considered "in the wild".
Should that assume be removed to get that failing test since it doesn't have a correct environment?

@npetzall

@npetzall Yes, I think this assumption should be removed. I see no reason this test to fail on Windows.
But I get your point!
I just don't agree that a test that uses TestContainers library should not fail when Docker is not available or fails to start for any reason. TestContainers library is about Docker! If Docker is not there then the whole test has no meaning.
To me it sounds like javac printing warnings while compiling the test in absence of JUnit in the classpath. It is an error!

@martin-g

@martin-g I don't care anymore.

But I agree with you "If Docker is not there then the whole test has no meaning" ergo Ignored, since Pass/Fail both have meaning.

@rnorth

@rnorth
You could do composition over inheritance and remove the implements TestRule and the starting(Description description). Create a new module junit and just have a ContainerRule which takes a supplier as it's constructor argument.

That sounds like a good idea and it's something we were already discussing, since we want to decouple TestContainers and JUnit (and there are some more parts in the codebase that would profit from using composition instead of inheritance). And once we've decoupled, I'd personally be fine to add more JUnit specific stuff to the JUnit part of the API.

AFAIK it's planned for version 2.0, but we might need to have a discussion regarding the final design first.

@npetzall

@npetzall all we're asking is not to add another JUnit-related method to GenericContainer.

As I said, we can see the value of what you're asking for, but please try and understand that for us this is something we have to maintain in the long term. If we add another JUnit-related feature to a core class then that just increases the likelihood of other users using it (not just yourself), and suffering when we move it (as we plan to do).

We're not ready to refactor JUnit out yet, so this is just the position we're in.

My suggested approach is this to have this in relevant tests for JUnit 4:

    @Rule
    public AssumptionRule assumptionRule = AssumptionRule.assumeDockerPresent();

I have a working implementation of this sitting right in front of me, and I think we can easily include it. If you don't want to use it, I can't stop you, but I would be grateful if you could adopt the approach that we're comfortable with supporting.

@rnorth

@rnorth I can't use it due to issue 344.

Also could you do the following test for me then.
Order of rules is undetermined according to junit javadoc for @Rule and @ClassRule Rule ClassRule

So order is somewhat unimportant.

  @ClassRule
  public static MysqlContainer mysqlContainer = new MysqlContainer<>();

  @ClassRule
  public static AssumptionRule assumptionRule = AssumptionRule.assumeDockerPresent();

Turn of docker so that It isn't present and run the test.
You might get some ignored test, but that's only because you get an error java.lang.ExceptionInInitializerError.

This is because construction of GenericContainer does to much. It fails when creating the MysqlContainer. The AssumptionRule might not even be evaluated.

As for the refactoring out. You can add @Deprecated to the junit stuff in core right now and create the junit module. There is no reason why they can not co-exist.

You should use RuleChain when you need to order your rules

…

On Jun 3, 2017 10:58 PM, "Nils Petzäll" ***@***.***> wrote: @rnorth <https://github.com/rnorth> I can't use it due to issue 344. Also could you do the following test for me then. Order of rules is undetermined according to junit javadoc for @rule and @ClassRule Rule <http://junit.org/junit4/javadoc/4.12/org/junit/Rule.html> [ClassRule] (http://junit.org/junit4/javadoc/4.12/org/junit/ClassRule.html ) So order is somewhat unimportant. @ClassRule public static MysqlContainer mysqlContainer = new MysqlContainer<>(); @ClassRule public static AssumptionRule assumptionRule = AssumptionRule.assumeDockerPresent(); Turn of docker so that It isn't present and run the test. You might get some ignored test, but that's only because you get an error java.lang.ExceptionInInitializerError. This is because construction of GenericContainer does to much. It fails when creating the MysqlContainer. The AssumptionRule might not even be evaluated. As for the refactoring out. You can add @deprecated to the junit stuff in core right now and create the junit module. There is no reason why they can not co-exist. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#343 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOKQl2IqwYtnGDbcy8u_gscLFvc7Pysks5sAbrlgaJpZM4NjOZ8> .

@martin-g

@martin-g that's correct it says so in the linked Javadoc. But it doesn't matter in this case since the test fails due to inability to instantiate the field that has the @ClassRule/@Rule annotation

@rnorth

@rnorth
I see what you did there, lazy init of DockerClient I like it, and the removal of getDockerName.

To take a step away from junit in core i've done this: issue_343_junit

New module junit module with GenericContainerRule which handles assumptions and such. It will allow you to add an assumption on the JdbcDriver as well. Which will avoid start of the container.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe this is a mistake, please reply to this comment to keep it open. If there isn't one already, a PR to fix or at least reproduce the problem in a test case will always help us get back on track to tackle this.

This issue has been automatically closed due to inactivity. We apologise if this is still an active problem for you, and would ask you to re-open the issue if this is the case.

I'd maybe suggest to start with the most basic thing: a way to check whether Docker is actually available or not, as a simple static boolean function, e.g. public static boolean DockerClient.isDockerAvailable(). That would allow us to use JUnit/TestNG-specific ways to skip tests if need be.

There may be another use case to consider as well: someone wants to (maybe just temporary) deactivate testcontainers in his build

see --> #2833 (comment)
and --> #2833 (comment)

We (Eclipse Jetty) build and test on a huge variety of OS's, architectures, and hardware combinations.
Not all environments that we build/test on have docker, which is fine.
It's also not possible to have all of our possible configurations tested at the same time during a single test run.

Can we at least get a unique exception like org.testcontainers.dockerclient.DockerUnavailableException so that we can make the decision on pass/ignore/fail on our vast junit5 test suite easier?

Currently, we get an IllegalStateException, which is useless to work with in this situation.

java.lang.IllegalStateException: Could not find a valid Docker environment. Please see logs and check configuration
	at org.testcontainers.dockerclient.DockerClientProviderStrategy.lambda$getFirstValidStrategy$7(DockerClientProviderStrategy.java:215)
	at java.base/java.util.Optional.orElseThrow(Optional.java:408)
	at org.testcontainers.dockerclient.DockerClientProviderStrategy.getFirstValidStrategy(DockerClientProviderStrategy.java:207)
	at org.testcontainers.DockerClientFactory.getOrInitializeStrategy(DockerClientFactory.java:136)
	at org.testcontainers.DockerClientFactory.client(DockerClientFactory.java:178)
	at org.testcontainers.LazyDockerClient.getDockerClient(LazyDockerClient.java:14)
	at org.testcontainers.LazyDockerClient.authConfig(LazyDockerClient.java:12)
	at org.testcontainers.containers.GenericContainer.start(GenericContainer.java:310)
	at org.eclipse.jetty.tests.distribution.HazelcastSessionDistributionTests.testHazelcastRemoteOnlyClient(HazelcastSessionDistributionTests.java:65)

@joakime

@joakime that's an interesting idea. Also, have you considered failing fast by checking DockerClientFactory#isDockerAvailable?

@bsideup

@bsideup I can report that the following junit5 behavior works on the various architectures and OSs that have proven difficult. (think ARM64 and new/experimental/alternative OS's)

@Test
public void testSomethingThatNeedsDocker() {
    Assumptions.assumeTrue(DockerClientFactory.instance().isDockerAvailable(), "Docker is not available on this environment");
    // do something that needs docker here
}

It's a shame that the docker resolution occurs twice though.
Once for our direct usage of DockerClientFactory, and again for the deep usage of testcontainers.
However some of our usage of testcontainers is other libs that use testcontainers, so we don't have access to modify behavior within those libs (if the new DockerUnavailableException (extended from IllegalStateException?) existed then we can at least know why the testing lib had an issue in a programmatic way.

Add a JUnit 4 rule to skip tests

Navigation Menu

Repository navigation

When docker isnt present #343

4 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

When docker isnt present #343

Description

Activity

npetzall commented on May 23, 2017

kiview commented on May 23, 2017

npetzall commented on May 23, 2017

martin-g commented on May 23, 2017

npetzall commented on May 23, 2017

npetzall commented on May 24, 2017

npetzall commented on May 26, 2017

kiview commented on May 26, 2017

rnorth commented on May 26, 2017

npetzall commented on May 26, 2017

npetzall commented on May 26, 2017

martin-g commented on May 29, 2017

npetzall commented on May 29, 2017

npetzall commented on May 30, 2017

kiview commented on May 30, 2017

4 remaining items

rnorth commented on Jun 3, 2017

npetzall commented on Jun 3, 2017

martin-g commented on Jun 4, 2017

npetzall commented on Jun 4, 2017

npetzall commented on Jun 5, 2017

stale commented on Oct 28, 2018

stale commented on Nov 11, 2018

mvysny commented on Jan 23, 2020

wlad commented on Sep 2, 2020

joakime commented on Aug 11, 2021

bsideup commented on Aug 11, 2021

joakime commented on Aug 11, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions