Skip to content
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

Add a JUnit 4 rule to skip tests #6445

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jtnord
Copy link

@jtnord jtnord commented Jan 30, 2023

if code makes use of GenericContainer or any other Test Containers based Rule then those rules will most likely throw an exception when docker is not available.

This leads people to add a BeforeClass annotation and test for docker, however this does not skip the tests, it skips the class. This has the unfortunate side effect that the report (from thinks like maven surefire and ant) that the test did not run when ingested into tools like Jenkins (as zero tests from the class either passed failed or skipped).

it is also not obvious on the command line.

by using a rule the individual tests will be marked as skipped so it becomes obvious that a test existed, but did not run for some reason (and the reason will be in the exception - that docker is not available)

fixes #4586 / #343

see also https://testcontainers.slack.com/archives/C1SUBPZK6/p1670598927610369?thread_ts=1670587111.250949&cid=C1SUBPZK6

Before comitting, run ./gradlew checkstyleMain checkstyleTest spotlessApply and fix any issues that occur.

243 changed files.... I've passed on adding all those changed files because the projects line ending configuration is incorrect.

if code makes use of GenericContainer or any other TestContainer based
rule then those rules will mostl likely throw an exception when docker
is not available.

This leads people to add a BeforeClass annotation and test for docker,
however this does not skip the tests, it skips the class.  This has the
unfortuante side effect that the report (from thinks like maven surefire
and ant) that the test did not run when ingested into tools like Jenkins
(as zero tests from the class either passed failed or skipped).

it is also not obvious on the command line.

by using a rule the individual tests will be marked as skipped so it
becomes obvious that a test existed, but did not run for some reason
(and the reason will be in the exception - that docker is not available)

fixes testcontainers#4586 / testcontainers#343
@jtnord jtnord requested a review from a team as a code owner January 30, 2023 15:39
}
* </code></pre>
*/
public class RequireContainerSupportRule implements TestRule {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went round the houses on the name of this a few times - in reality not sure if you require docker (the cli) at all, or just a configuration for docker (which could come from podman etc.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use docker, similar to what we have for Testcontainers annotation for JUnit Jupiter. See

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if DockerCheckRule make sense. I know naming is hard :)

@Override
public void evaluate() throws Throwable {
throw new AssumptionViolatedException(
"Docker support is not available and this test requires TestContainers which needs docker"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be Docker support... or Container support...

Copy link
Member

@eddumelendez eddumelendez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, @jtnord ! As it was discussed via slack, a new junit4 module would be great for this implementation. Also, some docs would be appreciated.

@Override
public void evaluate() throws Throwable {
throw new AssumptionViolatedException(
"Docker support is not available and this test requires TestContainers which needs docker"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Docker support is not available and this test requires TestContainers which needs docker"
"Docker is not available"

}
* </code></pre>
*/
public class RequireContainerSupportRule implements TestRule {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use docker, similar to what we have for Testcontainers annotation for JUnit Jupiter. See

}
* </code></pre>
*/
public class RequireContainerSupportRule implements TestRule {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if DockerCheckRule make sense. I know naming is hard :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

You can customize and ignore unit tests with testcontainer
2 participants