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

Start shared containers in beforeAll callback (Jupiter) #1020

Merged
merged 1 commit into from
Dec 26, 2018

Conversation

kiview
Copy link
Member

@kiview kiview commented Dec 19, 2018

I've probably missed some edge cases, but all current tests are green.
We also explicitly don't support parallel test execution at this point.

Fixes #1019.

@michael-simons
Copy link
Contributor

I took the freedom to have a look at the extensions. Reads good from my point of view.

What I didn't try out: How is the behaviour in regard to AfterAll? The closing of the containers is left to JUnit itself, right?

I probably would expect the container still being running in "my" after all methods.

@kiview
Copy link
Member Author

kiview commented Dec 20, 2018

So we are storing all containers inside the ExtensionContext.Store and wrap them in a CloseableResource, so Jupiter will stop them automatically.

Not really sure about the specific point in time this will occur though.

@jrehwaldt
Copy link

I tested this branch and from all I can tell it would fix the issue outlined in #1017 (see #1017 (comment) clarifying the situation together with a reproducer).

@kiview
Copy link
Member Author

kiview commented Dec 21, 2018

@jrehwaldt Thanks, sounds great.
I'll ping the others for having a look at it and then we can probably merge it.

@Override
public void beforeAll(ExtensionContext context) {
Class<?> testClass = context.getTestClass()
.orElseThrow(() -> new ExtensionConfigurationException("TestcontainersExtension is only supported for classes."));
Copy link
Member

Choose a reason for hiding this comment

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

can we test it?

Copy link
Member

@bsideup bsideup left a comment

Choose a reason for hiding this comment

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

Looks good in general. Although would be nice to test orElseThrow

@testcontainers testcontainers deleted a comment Dec 21, 2018
@kiview kiview added this to the next milestone Dec 21, 2018
@kiview
Copy link
Member Author

kiview commented Dec 21, 2018

@bsideup Yeah, I will add another ignored test and check it by hand.

@kiview
Copy link
Member Author

kiview commented Dec 21, 2018

So I don't really know a way to force the exception, I kind of think, it can't happen for a user.
I tried to following code:

class WrongExtensionUsageTests {

    @Testcontainers
    enum myEnum {
        FOOBAR
    }

    @Test
    void extension_throws_exception() {
        assert true;
    }

}

This won't execute the TestcontainersExtension class, I wonder why?

@kiview kiview merged commit bf3b7bc into master Dec 26, 2018
@delete-merged-branch delete-merged-branch bot deleted the junit-jupiter-beforeAll branch December 26, 2018 20:44
@rnorth
Copy link
Member

rnorth commented Dec 28, 2018

Released in 1.10.4!

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

Successfully merging this pull request may close these issues.

None yet

5 participants