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

TestcontainersExtension should be public to allow ordering extension in JUnit5 #2045

Closed
loicmathieu opened this issue Nov 7, 2019 · 21 comments

Comments

@loicmathieu
Copy link

When we use multiple JUnit5 extensions, the way to order them is via the @ExtendsWith({FirstExtension.class, SecondExtension.class}) annotation.
But TestcontainersExtension is not public so we can only use the @Testcontainers annotion and we are not able to order the extension.

See the following documentation on JUnit5: https://junit.org/junit5/docs/current/user-guide/#extensions-registration-declarative

Some microservices frameworks have extension to start the application for integration testing, whe using test containers, we need to be sure that the testcontainers extension run first to start the containers before the apps starts.

@HaMatthias
Copy link
Contributor

I am not sure but maybe have a look at the @RegisterExtension Junit5 Annotation. It allows you to specifiy any Order.

here: https://junit.org/junit5/docs/current/user-guide/#extensions-registration-programmatic

I also need some setup like the registry authentication config and therefore just place the @ExtendsWith before the @Testcontainers Annotation at top level class.

HTH

@loicmathieu
Copy link
Author

@HaMatthias surely I can use @RegisterExtension instead of @ExtendsWith, but in both cases the TestContainersExtension needs to be public in order to use them.

@stale
Copy link

stale bot commented Feb 12, 2020

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 stale bot added the stale label Feb 12, 2020
@loicmathieu
Copy link
Author

@HaMatthias @bedla send a PR for this. I still think it is useful ...

@stale stale bot removed the stale label Feb 12, 2020
hmatt1 added a commit to hmatt1/testcontainers-java that referenced this issue Apr 26, 2022
Proposed fix for testcontainers#2045

When we use multiple JUnit5 extensions, the way to order them is via the @ExtendsWith({FirstExtension.class, SecondExtension.class}) annotation.
But TestcontainersExtension is not public so we can only use the @testcontainers annotion and we are not able to order the extension.

See the following documentation on JUnit5: https://junit.org/junit5/docs/current/user-guide/#extensions-registration-declarative
@hmatt1
Copy link
Contributor

hmatt1 commented Jul 7, 2022

@loicmathieu @HaMatthias please see the discussion here: #5285

If you have any concrete examples of what you're looking for, that would be helpful!

@loicmathieu
Copy link
Author

@hmatt1 I thought I clearly describe the use case: having the ability to order JUnit extensions used together on the same test case. The documented way to do this from the JUnit guide is to use @ExtendsWith({FirstExtension.class, SecondExtension.class}) which is not possible as the TestcontainersExtension is not public.

If you have any concrete examples of what you're looking for, that would be helpful!

I just want it to be public to be able to manage extensions order as it is intended.

@hmatt1
Copy link
Contributor

hmatt1 commented Jul 8, 2022

@loicmathieu thanks for the quick response!

Which other extensions are you using that require ordering?

I opened a PR for this change, but the reviewer is considered not merging it because someone from JUnit recommended that it is not a good practice to require ordering between extensions. They were wondering if there was another solution to whatever problem you're trying to solve with the order of extensions.

@loicmathieu
Copy link
Author

Having testcontainer containers starts before my Quarkus test.

Even if JUnit didn't recommend to order between extensions, we sometimes needs to do it, and as testcontainers purpose is to launch dependencies as containers, it is very frequent that we need to start it before other extensions.

@tburch
Copy link

tburch commented Jul 8, 2022

I also ran into this and wanted to support ordering so that @Testcontainers was started before @MicronautTest.

@kiview
Copy link
Member

kiview commented Jul 11, 2022

Hey @loicmathieu, thanks for responding. We were looking for a concrete example, rather than @ExtendsWith({FooExtension.class, BarExtension.class}), since this is rarely useful for understanding an actual use case (it will only lead to adding more generic capabilities to a framework).

With special regards to the JUnit5 extension, overusing can lead to a widespread usage of container-per-class or container-per-method, which is generally considered an anti-pattern by the Testcontainers community. So we are careful to add features to the library, that make it even easier to fall into this anti-pattern. Therefore, we really want to understand the actual use case and context.

@tburch This is a good example, thank you. When looking at the Micronaut docs (https://guides.micronaut.io/latest/micronaut-kafka-gradle-groovy.html) I can see that when using @MicronautTest in a Spock test, usage of @Testcontainers is not required and Micronaut instruments the containers accordingly. I wonder why this is not the case for the JUnit5 test and maybe this is something that should be considered in the @MicronautTest JUnit5 extension.

I find the integration with microservice frameworks especially interesting and an important topic to discuss, since e.g. @SpringBootTest integration with Testcontainers via @DynamicPropertySource does not require any order whatsoever. A further example of what makes me conscious to understand what the use case for manually controlling the order is and whether opening up is the right approach.

@tburch
Copy link

tburch commented Jul 11, 2022

It's interesting that @DynamicPropertySource doesn't require any order. With Micronaut, I've been implementing TestPropertyProvider, which is order-dependent because I need the address/port from the container to set the properties correctly (for now manually starting the test container(s) prior to @MicronautTest doing its thing).

@superdrenner
Copy link

@kiview I have an example that is not ordering related. I need to be able to enable/disable the extension based on a condition in our CI pipeline. Because the class is not public, I can't do it. See the end of this JUnit thread for their examples: junit-team/junit5#1242

abstract class MyBaseTest {
    companion object {
        @JvmField
        @RegisterExtension
        val someExtension = if (someCondition) {
            TestcontainersExtension()
        } else {
            DoNothingExtension()
        }      
    }
}

@kiview
Copy link
Member

kiview commented Aug 15, 2022

@drennerpfc6 Can you please elaborate on why using @Testcontainers(disabledWithoutDocker = true) is not sufficient for your use case?

@superdrenner
Copy link

@kiview because I don't want the test disabled.

@kiview
Copy link
Member

kiview commented Aug 15, 2022

How do you provide the integration test dependencies in the other case? And what is the condition of the CI pipeline? Sorry, but we have to understand the actual use case context in order to make informed decisions on this.

@superdrenner
Copy link

superdrenner commented Aug 15, 2022

@kiview we simply run against real environments for regressions and the hope is to also run the same set of tests against testcontainers more frequently throughout the day. If I have to just manage the container lifecycle myself then I'll just do that but it's annoying.

Many other test frameworks expose their extension class so my hope was that it would be something you were open to changing.

@kiview
Copy link
Member

kiview commented Aug 15, 2022

I am not fundamentally against exposing them, just want to understand the use case and ideally avoid making falling into bad practices easier.

In the case of your example, I'd assume if you run against the real environment, you will run against the same infrastructure processes for the entirety of the test suite execution, correct? To correctly model this behavior, you should indeed not use any of the test framework integrations, but instead fallback to using the Singleton Container Pattern (which unfortunately has no direct support in the test framework extensions, because of current lack of integration points for such behavior). This approach will bind the lifecycle of the container to the JVM process lifecycle, which to me sounds much more similar to your conditional fallback and should therefore be preferred.

@superdrenner
Copy link

@kiview that's what I'm working through right now. If it works then I guess I'll write my own extension to mange this since I do not want to duplicate code across 100+ applications.

@kiview
Copy link
Member

kiview commented Aug 15, 2022

What do you want to manage? Bind the container to the test-suite lifecycle instead of the test-class lifecycle? I am not completely up-to-date on any progress on the JUnit5 side of things around this topic, but issues like this suggest to me, that exactly this topic is still WIP: junit-team/junit5#2816

I do not want to duplicate code across 100+ applications.

Wouldn't you need to duplicate the conditional extension registry code you shared above across your applications as well?

@superdrenner
Copy link

@kiview the only thing I want is to be able to disable testcontainers and not disable the actual tests based on some condition. Duplicating an if () statement is not a problem, I didn't want to duplicate the reset of it.

@kiview kiview closed this as completed in 9275bc5 Aug 16, 2022
@kiview
Copy link
Member

kiview commented Aug 16, 2022

Thanks for everyone providing valuable feedback and context on their use cases. We decided to make the extension public, to give more freedom to our users, on how to use and integrate Testcontainers.

The change is merged in #5285.

@kiview kiview added this to the next milestone Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants