Closed LMnet closed 5 years ago
I think the whole misunderstanding comes from this block:
- If I want to do something before or after the test, I need to place this logic in a method with @Before/@After annotation.
- But If I need to do something with the container, I need to place this logic in the methods inside the container itself.
These methods will be executed by the testing framework (e.g. see #1326 ), and this is why we need an interface - to provide a generic set of methods to be executed by the testing frameworks.
Integration simplification. With TestLifecycleAware developers from the Testcontainers team should think about the integration of TestLifecycleAware with the test framework. It's some extra code, a bit of extra complexity.
I think you are weighing this aspect differently than the Testcontainers team.
It's a good thing that TestLifecycleAware
should be considered (and ideally implemented) when writing integrations for other test frameworks. This allows us to specify test related container behavior once and have it directly available in all test framework integrations.
You are right that the only class making use of this feature at the moment is BrowserWebdriverContainer
. However this feature is valued highly by many users (taking user feedback in account) and lack of TestLifecycleAware
support in test framework integrations (like in the Spock integration) is quickly discovered by users, much to their dismay.
In addition, and as already mentioned in the Slack discussion, we have plans to implement further functionality that will make use of those callbacks, like Database and filesystem snapshots.
I'm not sure if we can come to an agreement, since we definitely put a focus on different aspects here.
2 is absolutely incorrect and we never advocate this.
Even if there is no explicit recommendation about it, the existence of 2 different places for logic before/after test could confuse newcomers:
beforeTest
and afterTest
in the containers when I already have hooks from my test framework?TestLifecycleAware
? And in which cases should I use hooks from my test framework?TestLifecycleAware
?The methods exist so that the container's implementation can do something before/after a test, not the end user.
Why containers should contains this logic inside? It's not a container logic, it's a test logic. A test logic should be placed inside tests, or inside some test helpers. A developer should be able to have full control under his tests. No need to impose some test behavior to the developer.
These methods will be executed by the testing framework (e.g. see #1326 ), and this is why we need an interface - to provide a generic set of methods to be executed by the testing frameworks.
If containers will just provide methods with the same logic as we have now in the TestLifecycleAware
methods, the developer will still be able to use this logic. But this logic will not be imposed on a developer. As I said before, this is a test logic, not container logic. A developer should control it, not a container.
I think you are weighing this aspect differently than the Testcontainers team.
Yes, it's not a big deal to add this integration. But it worth mentioning.
You are right that the only class making use of this feature at the moment is
BrowserWebdriverContainer
.
I don't think that this is important. Number of integrations with the TestLifecycleAware
does not affect on my arguments from the proposal
However this feature is valued highly by many users (taking user feedback in account) and lack of
TestLifecycleAware
support in test framework integrations (like in the Spock integration) is quickly discovered by users, much to their dismay.
I'm not sure about it. As I already mentioned, TestLifecycleAware
doesn't provide any new or unique functionality. Developers can use functionality from their test frameworks to solve the same problems.
In addition, and as already mentioned in the Slack discussion, we have plans to implement further functionality that will make use of those callbacks, like Database and filesystem snapshots.
That's great. Just provide this functionality as a methods without assumptions about caller use cases. Let's take database snapshots for example. To give this functionality you can add a method like saveShapshot(file)
. And that's it. Developers will be able to use in inside tests. Or after tests. Or before tests. It's fully under developer control.
I'm not sure if we can come to an agreement, since we definitely put a focus on different aspects here.
That's why I created this proposal. I believe that even if we have different views or use cases we can still come to the right decision.
I think you are concentrating too much on a current situation and historical reasons. Of course, testcontainers has features to do some stuff before/after tests for a while. This features is familiar and feels usable. But just imagine that testcontainers doen't have any kind of this functionality. And all other features are in the library. Would you add something like TestLifecycleAware
in the library in this case? I don't think so. TestLifecycleAware
doesn't provide any unique functionality and doesn't solve any real issue. You can solve all problems that TestLifecycleAware
trying to solve without it.
That's the main reason why I propose to delete TestLifecycleAware
. It's just redundant. And deletion of TestLifecycleAware
in the current library state will give us a few advantages I mentioned before.
the existence of 2 different places for logic before/after test could confuse newcomers
I think you're speculating. Newcomers will never know about that interface. We have it, so that the testing frameworks can integrate with Testcontainers. If a newcomer writes an integration, he is considered an advanced user.
Why there is a beforeTest and afterTest in the containers when I already have hooks from my test framework?
Because they serve different purposes. You, as a user of your test framework, use the hooks to do your stuff. Your test framework, if integrated with Testcontainers, uses beforeTest
/afterTest
to match your expectations (e.g. @Rule
in JUnit 4 or @ExtendWith() + @Container
in JUnit 5).
beforeTest
/afterTest
should not be used in the tests' code.
In which cases should I use methods from the TestLifecycleAware? And in which cases should I use hooks from my test framework?
Never, as a user. As mentioned already, you use them only then you integrate with a testing framework.
What If I don't want to use behavior from methods from the TestLifecycleAware?
Then you don't use the rules / @Container
annotation, etc. As a user, you don't know about TestLifecycleAware
. You only know about your testing framework, and that the @Rule
marked field, for instance, will be controlled by JUnit. You don't want it to be controlled - you remove framework's ways of controlling it.
Why containers should contains this logic inside? It's not a container logic, it's a test logic. No need to impose some test behavior to the developer.
Take a look at the project's name. It will give you a hint ;)
I'm not sure about it
We are. Testcontainers is the result of many years of development, community communication and feedback listening. We would not become the most popular Docker testing library out there without it. Just think about it.
Just provide this functionality as a methods without assumptions about caller use cases. Let's take database snapshots for example. To give this functionality you can add a method like saveShapshot(file). And that's it. Developers will be able to use in inside tests. Or after tests. Or before tests. It's fully under developer control.
One of the most beloved aspect of Testcontainers is that you create a container, annotate it with @Rule
, for instance, and it works. With the recording or any other feature. That is how most of the users are using it.
If you are an advanced user and want to control everything, the methods are there (e.g. .start()
, .stop()
or saveShapshot(file)
if/when we add it). Just we make it easier for the ones who want to solve their testing problem instead of controlling everything.
I believe that even if we have different views or use cases we can still come to the right decision. I think you are concentrating too much on a current situation and historical reasons TestLifecycleAware doesn't provide any unique functionality and doesn't solve any real issue
I suggest you to think about it differently. We have our view on the problem because we were solving a problem. Meanwhile, you're trying to push your idea, with your only argument is "it doesn't provide any unique functionality". If you really want to help the project, interview a few QA people about their experience with the browser container and how it "magically works without them doing anything because they are not hardcore developers".
give us a few advantages I mentioned before
That's a speculation. "Less code" is never an advantage per se, as long as we're (as a team) ready to maintain it.
Also, when you say "us" - what does it mean to you?
I fully agree with @bsideup and @kiview here.
A lot of what Testcontainers is for is to stop developers having to write/extend boilerplate classes. If we were being strict about giving full control to developers and having no test framework/lifecycle coupling, we wouldn't have @Rule
, for example. Instead, we took an opinionated position on having various elements of test framework integration that we think saves people effort.
The trouble with opinionated design decisions is that they tend to have disadvantages, and tend to face disagreement at some time. I think that this is the case here, but unfortunately I don't see a way to find a middle ground. I'm sorry, but we might have to 'agree to disagree'.
FWIW, If/when we get around to creating a non-testing container library, with Testcontainers becoming a testing-related library on top of it, this interface is likely to move and/or change. I'm afraid that now is a bit too soon to know exactly what we'll do with it though.
BTW I do really appreciate that you've put a lot of time and thought into this proposal, and been civil throughout on a topic you feel strongly about. Thank you for this.
I think you're speculating. Newcomers will never know about that interface. We have it, so that the testing frameworks can integrate with Testcontainers. If a newcomer writes an integration, he is considered an advanced user.
I'm talking about developers who saw TestLifecycleAware
for the first time. I think they could be surprised by this.
Your test framework, if integrated with Testcontainers, uses beforeTest/afterTest to match your expectations
I'm not sure about what expectations you are talking about. Could you give an example?
beforeTest/afterTest should not be used in the tests' code.
Maybe I was not 100% clear, but I never proposed this. I'm talking about logic inside these methods. This logic should be available for the developers without TestLifecycleAware
too.
Never, as a user. As mentioned already, you use them only then you integrate with a testing framework.
You are talking about the usage part. But I was talking about the implementation of containers. In which situation should I put the logic after test in the container, and in which in the test-framework hook? This is the confusing part in my opinion.
Then you don't use the rules / @Container annotation, etc.
But in that case, I will lose start/stop functionality too, which is always useful.
We have our view on the problem because we were solving a problem.
About which problem you are talking about?
interview a few QA people about their experience with the browser container and how it "magically works without them doing anything because they are not hardcore developers".
Well, yes, I don't have enough side look about this. But for me, there is no difference between doing something inside the test or after the test. If QA people can call methods on the container inside the test body, they can do the same after the test, using the test-framework feature.
"Less code" is never an advantage per se, as long as we're (as a team) ready to maintain it.
This is a bit philosophical, but I do not agree with your opinion. Less code is always better than any code. You could have bugs in this code. You should evolve this code. But it's not really important. When I'm talking about "few advantages" I, first of all, mean less opinionated core.
Also, when you say "us" - what does it mean to you?
It's contextual. I will try to be more clear.
If we were being strict about giving full control to developers and having no test framework/lifecycle coupling, we wouldn't have @Rule, for example.
I think integration with test frameworks could use only start/stop. And it's enough for most cases.
Instead, we took an opinionated position on having various elements of test framework integration that we think saves people effort.
As far as I know, at the current moment, only Startable
and TestLifecycleAware
are taking part in the integration. The Startable
is, in any case, is necessary for all scenarios with containers. But in my opinion TestLifecycleAware
is not.
So, your position is that testcontainers provides docker containers which are useful mostly for tests. And also, testcontainers provides some default behavior inside the tests with the methods from TestLifecycleAware
. In the case of BrowserWebDriverContainer
it's a behavior of saving videos after a test. In the case of DB containers, it could be saving snapshots, as was already mentioned by @kiview. Am I right?
@LMnet I respect the time and the thought you put into this discussion, but TBH, this whole argument seems like the ultimate example in bikeshedding and I therefore won't go on participating in it any further.
We all have limited capacities for supporting and developing the project and this is not the hill I plan to die on. It's all about prioritization and pragmatism in the face of limited resources and if you look at the open issues and feature requests, I'd assume you agree, there are more valuable things to spend energy on. Not mentioning the hassle for our users when pushing a breaking change.
Are there any concrete problems you are facing in your developer life because of TestLifecycleAware
?
I want to give some prehistory. I'm working on a new Scala API in the testcontainers-scala. Current Scala API uses starting
, succeeded
, failed
and finished
methods. These methods are deprecated. Instead of them, there is a new approach with the TestLifecycleAware
. But in the new Scala API, I wanted to make the core module of testcontainers-scala as generic as possible. TestLifecycleAware
is a hard coupling with the test scenario. I started to think about it and discussed it. And as more I thought about it, the more I convinced that TestLifecycleAware
is a redundant functionality.
Without TestLifecycleAware
I could achieve my goal to create a generic core. On top of this core, I could create a more opinionated module with some test logic. On top of this module, I could create a bunch of integrations with test frameworks. I believe that this is the right direction for the library API. And for testcontainers-java too. But to split one module (current state) to 2+n modules in the testcontainers-java library authors needs to spend pretty much time. This is not an option for now.
But if you ignore deprecated stuff and see on the current state of a testcontainers-java you could see, that the only obstacle on the way of the generic core module is TestLifecycleAware
. And even without this, removing of TestLifecycleAware
could give some advantages to the testcontainers-java in my opinion.
That's why I created this proposal. I didn't mention testcontainers-scala a lot because it's an internal course of a testcontainers-scala and this course should not affect testcontainers-java. I tried to use arguments specific only for the java library.
I can accept and understand that testcontainers-java is not ready now for this separation to 2+n modules. But as I already said, we now only one step away from the generic core. If testcontainers-java will continue its opinionated course, it potentially means more test-specific functionality (like more containers with methods from the TestLifecycleAware
). And it means that in future we, potentially, will not have a possibility to split the library to the 2+n modules without huge breaking changes.
I wanted to make the core module of testcontainers-scala as generic as possible. Without TestLifecycleAware I could achieve my goal to create a generic core. On top of this core, I could create a more opinionated module with some test logic. On top of this module, I could create a bunch of integrations with test frameworks.
I don't think you should be doing it in testcontainers-scala. That library is just a wrapper (or a Scala language binding) around the main one.
We do plan to create "containercore" library), but it will take time and it is quite pointless to achieve it in a separate library given the main project is still heavily test-oriented (not just the lifecycle, but also the cleanup at JVM exit, for example, or random names).
But as I already said, we now only one step away from the generic core
Sorry, but you're missing the bigger picture. Please leave it to the Testcontainers team to decide.
Sorry, but you're missing the bigger picture.
It's absolutely true and I could miss some parts.
I don't think you should be doing it in testcontainers-scala.
This is not a final decision atm. But it looks like Scala API could progress a lot faster because it's a lot smaller. That's why I think I could achieve this in the next few releases.
We do plan to create "containercore" library), but it will take time and it is quite pointless to achieve it in a separate library given the main project is still heavily test-oriented (not just the lifecycle, but also the cleanup at JVM exit, for example, or random names).
Well, from the messages in this discussion it looks like this is pretty long plans. But if this is a current long-term strategy, I want to help these plans come true earlier.
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.
This proposal based on a recent slack discussion: https://testcontainers.slack.com/archives/C9EJ7TVT7/p1553315996086400.
I propose to delete from the core project
TestLifecycleAware
class.Current state
As far as I understand, current project direction looks like this:
I fully support this course and I believe that it will simplify the code base and make it more maintainable. This will allow the library to grow faster in the future. In addition, I'm a user and contributor to the testcontainers-scala library, which relies on a testcontainers-java. For the scala facade, the simple and test-framework agnostic core will be a huge win. It will lead to a simpler integration and will give the possibility to delete some non-elegant code from the testcontainers-scala.
One of the steps in this direction was deprecation of a
starting
,succeeded
,failed
andfinished
methods. These methods describe container lifecycle in the tests. They are declared in theFailureDetectingExternalResource
class. This is aTestRule
for the JUnit 4. All containers are based on this class. When library user wants to add some logic before or after tests, he could use one of those methods.Disadvantage of the
FailureDetectingExternalResource
was in hard coupling with JUnit 4. This test rule will work only with this test framework.As a replacement for the
FailureDetectingExternalResource
were introducedStartable
andTestLifecycleAware
interfaces. InStartable
there is 2 methods:start
andstop
, for start and stop logic of a container. InTestLifecycleAware
there isbeforeTest
andafterTest
, for the test lifecycle logic.This new approach is better because it's test-framework agnostic. All test-framework specific logic will be in the integration layer, and this layer will use methods from the
Startable
andTestLifecycleAware
interfaces.But, I think,
Startable
is enough, and we don't need to haveTestLifecycleAware
in the core.Motivation
To describe why we can remove
TestLifecycleAware
and don't lose any functionality, I would like to make a digression.Let's think about typical developer experience with the test frameworks. For examples I will use JUnit 4.
If I want to create a test with JUnit 4, I will create a class with methods, place my test logic in these methods, and mark them with
@Test
annotation. Like this:But it would work only for the simplest cases. Sometimes I need to do something before or after each test. For this JUnit 4 gives me a possibility to mark methods in the test class with the
@Before
and@After
annotations. A code inside these methods will be called before/after each test. For example:But sometimes I need to do something before or after all tests in the test class. For this case, JUnit 4 gives me
@BeforeClass
and@AfterClass
annotations. For example:But sometimes it's not enough to do some unconditional code after the test. Sometimes I need to do something only for failed tests, or only for succeeded. For this case, JUnit 4 gives me
TestWatcher
rule. For example:We could find similar functionality in other test frameworks too. In JUnit 5 we have
@Before/AfterEach
,@Before/AfterAll
andTestWatcher
. In TestNG we have a lot of before/after annotations, including@Before/AfterMethod
and@Before/AfterClass
. In scalatest, which I use with the testcontainers-scala, I also have similar functionality. And I believe, that almost all test frameworks have some sort of this. Developers are familiar with the patterns from their test framework. These patterns are described in the documentation. They could be googled easily.Now, let's imagine, that developer decided to use testcontainers for some reason. And, for example, he needs to do something after each test, and he uses JUnit 4. A familiar pattern for the developer is using framework functionality. So, he would use
@After
annotation for this:As you could see in the example, the developer could place any logic in the
after
method. He also could somehow interact with the container: call some methods, get some info from it.With this example, I want to show that containers are not something special from the test framework view and from the developer view. It's just a java object with some methods, which developer could use in the different places in the test class. As I said before, this is a familiar pattern for the developer.
Now, let's return to the
TestLifecycleAware
. It's supposed that methods from theTestLifecycleAware
will be called by the test framework, not a developer. And I think this is the main breaking moment of aTestLifecycleAware
.So, with all other objects (fixtures, counters, instances to test, etc) developer should interact in one way, but with the containers in some other way. Idiomatic logic for testcontainers, in that case, would look like this:
@Before/@After
annotation.For example:
It means that instead of using familiar and unconditional pattern, which is recommended by the test framework, testcontainers force developers to add an exception to these patterns in case of containers.
Of course, I could just not use
TestLifecycleAware
at all, and use familiar patterns from my test framework. But what If I want to use the container with already implemented methods from theTestLifecycleAware
? This container will impose this behavior. And In the case when I want another behavior I will have to create my own container through inheritance and override this behavior.Disadvantages of the
TestLifecycleAware
Let's sum up all disadvantages:
TestLifecycleAware
impose me behavior from these methods. If I don't need it, I have to override it.TestLifecycleAware
methods. So,TestLifecycleAware
is useful only for some scenarios.TestLifecycleAware
on multiple containers. For some scenarios, it could be important.TestLifecycleAware
itself doesn't solve any problem. All problems already solved on a test-framework side.TestLifecycleAware
just adds some extra level of abstraction.Alternative approach
I propose to just not use
TestLifecycleAware
at all. Instead, just use functionality and patterns from your test framework. They already have all the things we need. We don't need to reimplement this in the testcontainers.Also, without
TestLifecycleAware
we could deleteTestDescription
. It needs only for theTestLifecycleAware
.Advantages of the alternative approach
TestLifecycleAware
is not big or complex part of a project, but any deletion from the core is always a good thing for the library.TestLifecycleAware
developers from the testcontainers team should think about the integration ofTestLifecycleAware
with the test framework. It's some extra code, a bit of extra complexity. For example, I'm now working on a new API for a testcontainers-scala. And integration with theTestLifecycleAware
takes ~20 lines of code. It's not too much, but all integration is ~100 lines of code. So,TestLifecycleAware
is 1/5 of my current integration code. I think it's a noticeable number.TestLifecycleAware
(excluding deprecated code) somehow tie core with the testing case. WithoutTestLifecycleAware
core module will not have any assumption of a use case. The core will provide containers in the form of java objects. These containers will have some fields and methods. A developer could interact with these containers in a way he wants — containers not assume anything about the use case. It means less coupling with the test scenario, more wide use case for the core. Of course, you could use testcontainers in that way today too. But the existence ofTestLifecycleAware
in that case looks like a redundant, leaky abstraction.Disdvantages of the alternative approach
I can see these disadvantages:
TestLifecycleAware
methods, and we removed these methods, developers will have to reimplement this logic on their side. This problem may be relevant to theBrowserWebDriverContainer
. This is the only container in the library with this logic. Possible solution: improveBrowserWebDriverContainer
to give a simple way to reimplement this logic on the user side. Ideally, with the single method call. Another solution, which could be used with the previous one, is to provide functionality from theBrowserWebDriverContainerю.afterTest
on the integration level.Summary
I believe that we just don't need
TestLifecycleAware
. And if we deleteTestLifecycleAware
we don't lose anything, and only get some advantages I described above.