Testcontainers-java: When docker isnt present

[npetzall]

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.

[npetzall]

Some discussion before I implement and PR?

[kiview]

IMO letting the test fail is the correct behavior.

[npetzall]

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.

[martin-g]

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.

[npetzall]

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.

[npetzall]

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.

[npetzall]

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.

[kiview]

Let's wait for the opinion of @rnorth.

[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

[npetzall]

@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.

[npetzall]

@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?

[martin-g]

@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!

[npetzall]

@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.

[npetzall]

@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.

[kiview]

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]

@kiview So really this can be implemented now, since it will later be refactored and moved to a junit module?

[kiview]

If schemaspy otherwise has to fallback to using the Jitpack build of it's own fork of TestContainers, it might be the more sensible approach to have some trade-off solution at this point in time. Depending on the current progress of the refactoring, I'd be fine with it, but I think @rnorth or @bsideup should decide, since they are more involved in the core.

[npetzall]

The fork also solves #344, #345 so only doing this will not super-seed the fork.
However there is nothing stopping you from creating the junit module today I guess.
Which means that you currently can deprecate the direct usage of Container keep backwards compatability. And just incline user to switch to the junit module before 2.0.

[npetzall]

And the reason might be, that it's currently working for all of us, so this seems like a bit of yak shaving? :wink:

Really nice attitude.

But then again why is a fork a problem. Isn't the whole point that if we have different opinions in regards to the code, one can make a fork and customize it to ones liking?

To me it just seems silly that you don't want the behaviour. I fork and suddenly it's a bit interesting.

[rnorth]

@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.

[npetzall]

@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.

[martin-g]

You should use RuleChain when you need to order your rules

On Jun 3, 2017 10:58 PM, "Nils Petzäll" [e-mail] 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
https://github.com/testcontainers/testcontainers-java/issues/343#issuecomment-305997935,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAOKQl2IqwYtnGDbcy8u_gscLFvc7Pysks5sAbrlgaJpZM4NjOZ8
.

[npetzall]

@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

[npetzall]

@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.

[stale[bot]]

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.

[stale[bot]]

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.

[mvysny]

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.

[wlad]

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

see --> https://github.com/testcontainers/testcontainers-java/issues/2833#issue-630565097
and --> https://github.com/testcontainers/testcontainers-java/issues/2833#issuecomment-685992783