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

Undividable Test Units #87

Open
Vampire opened this issue May 8, 2023 · 3 comments
Open

Undividable Test Units #87

Vampire opened this issue May 8, 2023 · 3 comments

Comments

@Vampire
Copy link
Contributor

Vampire commented May 8, 2023

I'm sorry if this is not an issue with JUnit 5, but I'm not sure what the point with the old logic was, so I'm not sure how to test whether it is a problem.

In the JUnit 4 test unit finder, there is some logic to not have one test unit per test method if there are certain conditions like if there is a BeforeClass or AfterClass annotation or if there is a class rule applied to a method or field.

In the JUnit 5 codebase I have not seen such code but it seems it is always split into the leaf tests.

Whatever was the cause for that code for JUnit 4, I'd guess it is also necessary for Jupiter or Spock (where I had a PR open for the JUnit 4 based Spock 1.x, as PIT just never split Spock specifications.)

Or is the situation different here?

@Vampire
Copy link
Contributor Author

Vampire commented May 8, 2023

Maybe #46 is actually also caused by this?

@hcoles
Copy link
Contributor

hcoles commented Jun 2, 2023

The idea of a test unit is that each one is a minimal atomic "unit of testing", that can be independently run. If there were no overhead invovled in creating one, making them as small as possible would give the best performance as this allows pitest to run the minimal amount of code against a mutant.

Pitest was originally written to work with junit 3 and 4. The "no overhead involved" caveat largely held for those test frameworks, except when BeforeClass and AfterClass methods were involved.

When a test has one, there are two choices

  1. Wrap every test method into a unit, and run the BeforeClass and AfterClass for each one
  2. Treat the whole class as a unit, so all the tests are run in order until one fails

Which one performs better depends on the codebase. If the before/after class methods do serious heavy lifting, treating the whole class as a unit will generally give better performance.

If the code in the before/after is lightweight (which some teams inexplicably do, despite having to make things static), splitting into units performs better.

Pitest went with the first option as it performed best for the codebases I maintained at the time. Sometimes quite significantly.

JUnit 5 invalidates much of this as

  1. There is now a more significant cost to creating each "unit"
  2. Identifying where the logical boundaries of a minimal unit of test is much harder due to dynamic tests and the essentially open ended behaviour

So it's not clear that splitting the tests up is the right thing to do for JUnit 5.

This does seem relevant for #46, but I'm not entirely sure it's worth trying to support tests with an order dependency. If they are modifying state in a way that requires a certain order, the chances of retained state causing a test to fail when challenging a mutant are quite high, even if that order is maintained up until the point that a test fails within each individual class.

@Vampire
Copy link
Contributor Author

Vampire commented Jun 9, 2023

Hm, ok, thanks for the clarification.
I think it is also a matter of correctness though.
At least for Spock, hence #93.
For example if a spec has a @Shared field, chances are high multiple features or iterations work together somehow and should run as one unit.
Or if a spec or feature is @Stepwise, the sub-nodes should really be run in that order and not standalone.
Indeed it will not be standalone, but if you have stepwise A -> B -> C and all are relevant for a mutant, then if you trigger them separately, then first A is run, then A and B, then A and B and C, as the stepwise also makes sure the preconditions are run too.

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

No branches or pull requests

2 participants