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

Change default behavior to before each test rather than per class #74

Open
ghost opened this issue Jun 27, 2019 · 4 comments
Open

Change default behavior to before each test rather than per class #74

ghost opened this issue Jun 27, 2019 · 4 comments
Labels

Comments

@ghost
Copy link

ghost commented Jun 27, 2019

I'm extremely puzzled by the default behavior of @FlywayTest when a class is annotated with it. Fundamental testing principles state that tests should never depend on each other. This means that any tester should want and expect a clean state for every test! The be quite frank I cannot fathom why it should default to a per class setup rather than a per test setup in any setting. I do not want to - nor should I ever have to - think about if a previous test modifies state in a way that affects the outcome.

One should always be able to change the DB in any way in any way, without it affecting the next test. One should for instance try to put erroneous date in the database and see how ones application handles it, without risking breaking the rest of the world. Temporal logic between tests is almost always (if not always) extremely bad!

Even if it works, unit testing frameworks does not guarantee the order of execution. Even if the tests works now, there is no guarantee that it will stay like that - a very real problem by the way, the Java framework have at least once changed the implementation of sets, which indeed broke the tests several projects - (because they did things wrong! The documentation and testing principles did say they shouldn't do it like this).

It is thus not only a surprising behavior, it is actually broken by design - even if no current tests reveals it.

So please, change the default behavior to what is and should be expected. Do not motivate people to use tests in a wrong manner. Of course if people actually want this (highly questionable) behavior they could do it on a @BeforeClass method, then they at least do it with an explicit intent and then the responsibility of potentially broken tests is their own. I do not believe any framework though should default to a broken behavior.

@squaregoldfish
Copy link

Is there any reason why you're annotating the class with @FlywayTest rather than the individual test methods within it?

@ghost
Copy link
Author

ghost commented Jun 28, 2019

@squaregoldfish I'm not annotating the class, because annotating the class is non-idiomatic. The point is that it should be idiomatic. I don't want to annotate each singe test method with @FlywayTest. so currently I'm using the workaround here: #57 (comment) however, it is a workaround. The entire point of this issue is that the default behavior should be the idiomatic behavior. There might be a point in allowing non-idiomatic usage (although I'd consider using it highly questionable), but defaulting to it, and having the easiest way of using the plugin being non-idiomatic is just poor design, it motivates both badly designed tests and provides surprising behavior - programming should be predictable, not surprising.

@squaregoldfish
Copy link

I'm not a dev of this project, so I'll have to leave the ultimate response to someone who is. But the idea that annotating a test will work on the test, and annotating a class will work on the class, seems fairly logical to me.

@FlorianGWE
Copy link
Collaborator

First off all I will not change a default behavior that exist since 2011 and break the test off all depending projects.

All other is a different discussion about how to write good test together with DBs and why you need a tool like flyway-test-extensions. I will give you a answer when I’m back on ‘real computer’ and not using a mobile device.

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

No branches or pull requests

2 participants