junit5 icon indicating copy to clipboard operation
junit5 copied to clipboard

Execute `@BeforeAll`/`@AfterAll` once for each path in `@Nested` hierarchy

Open sewe opened this issue 6 years ago • 8 comments

I have a series of tests which are rather expensive to set up and build on each other. ATM, I use nested tests like so:

@DisplayName("An empty repository")
class RepositoryTest {

  @BeforeEach
  void setUpEmptyRepository() { /* expensive I/O */ }

  @DisplayName("has no revisions")
  void testGetRevisions() { ... }

  @DisplayName("has no files in head revision")
  void testGetFiles() { ... }

  @Nested
  @DisplayName("after adding a file")
  class WhenFileAdded {

    @BeforeEach
    void addFile() { /* expensive I/O */ }

    @DisplayName("has a single revisions")
    void testGetRevisions() { ... }

    @DisplayName("has the file in its head revision")
    void testGetFiles() { ... }

      @Nested
      @DisplayName("after removing the file again")
      class WhenFileRemovedAgain { /* expensive @BeforeEach */ ... }

      @Nested
      @DisplayName("after adding another file")
      class WhenAnotherFileAdded { /* expensive @BeforeEach */ ... }
  }
}

This works, but does expensive set-up operations along each path in the @Nested hierarchy over and over again, once for each test method:

  • RepositoryTest
    • testGetRevisons
    • testGetFiles
    • WhenFileAdded
      • testGetRevisons
      • testGetFiles
      • WhenFileRemovedAgain
        • testGetRevisons
        • testGetFiles
      • WhenAnotherFileAdded
        • testGetRevisons
        • testGetFiles

Executing WhenAnotherFileAdded.testGetRevisons performs the @BeforeEach set up for RepositoryTest, WhenFileAdded, and WhenAnotherFileAdded. And WhenAnotherFileAdded.testGetFiles does so again – even though the test methods are "read only" (only the set up does heavy I/O).

Unfortunately, switching to @BeforeAll for the set-up methods does not work, as in that case the set-up methods are just executed once, not once for each path in the @Nested hierarchy.

For example, when executing WhenAnotherFileAdded.testGetRevisons it might be that the set-up methods executed are, in this order:

  1. RepositoryTest.setUpEmptyRepository
  2. WhenFileAdded.setUp
  3. WhenFileRemoveAgain.setUp
  4. WhenAnotherFileAdded.setUp

This is obviously wrong: WhenFileRemovedAgain.setUp was meant for a different branch of the @Nested hierarchy. Thus the assumption that, say, WhenAnotherFileAdded.testGetFiles makes no longer hold, as some file was removed.

It would hence be nice if the behavior of @BeforeAll/@AfterAll were changed (or at least tweakable) for @Nested tests.

I am aware that some amount of set-up work needs to be duplicated, e.g., as both the tests in RepositoryTest.WhenFileAdded.WhenFileRemovedAgain and RepositoryTest.WhenFileAdded.WhenAnotherFileAdded have some overlap in their nesting context, but it would be nice to at least avoid the set-up for all tests that share the same nesting context (for lack of a better term).

Deliverables

  • [ ] Either switch the behavior of @BeforeAll and @AfterAll to be executed once for each nesting context (rather than once for each class, regardless of nesting context)
  • [ ] Or introduce a new TestInstance.Lifecycle constant to switch to such a behavior.

sewe avatar Aug 27 '19 09:08 sewe

For example, when executing WhenAnotherFileAdded.testGetRevisons it might be that the set-up methods executed are, in this order:

  1. RepositoryTest.setUpEmptyRepository
  2. WhenFileAdded.setUp
  3. WhenFileRemoveAgain.setUp
  4. WhenAnotherFileAdded.setUp

This is obviously wrong: WhenFileRemovedAgain.setUp was meant for a different branch of the @Nested hierarchy. Thus the assumption that, say, WhenAnotherFileAdded.testGetFiles makes no longer hold, as some file was removed.

What do you mean when you say "it might be"?

If WhenAnotherFileAdded.testGetFiles is explicitly selected as the only thing to execute, then WhenFileRemovedAgain.setUp should not be invoked.

Otherwise, that would be a bug.

sbrannen avatar Aug 27 '19 12:08 sbrannen

If WhenAnotherFileAdded.testGetFiles is explicitly selected as the only thing to execute, then WhenFileRemovedAgain.setUp should not be invoked.

@sbrannen Thanks for the quick reply. In that scenario, WhenFileRemovedAgain.setUp is indeed not invoked.

What I meant was this:

If I run all tests, they might be executed in the following order:

  1. RepositoryTest.setUpEmptyRepository
  2. RepositoryTest.testGetRevisions
  3. RepositoryTest.testGetFiles
  4. WhenFileAdded.setUp
  5. WhenFileAdded.testGetRevisions
  6. WhenFileAdded.testGetFiles
  7. WhenFileRemoveAgain.setUp
  8. WhenFileRemoveAgain.testGetRevisions
  9. WhenFileRemoveAgain.testGetFiles
  10. WhenAnotherFileAdded.setUp
  11. WhenAnotherFileAdded.testGetRevisions
  12. WhenAnotherFileAdded.testGetFiles

Thus, when WhenAnotherFileAdded.testGetFiles executes, these four set-up methods have executed:

  1. RepositoryTest.setUpEmptyRepository
  2. WhenFileAdded.setUp
  3. WhenFileRemoveAgain.setUp
  4. WhenAnotherFileAdded.setUp

Whereas I want only these three to execute, just as if I had explicitly selected WhenAnotherFileAdded.testGetFiles as the only thing to execute:

  1. RepositoryTest.setUpEmptyRepository
  2. WhenFileAdded.setUp
  3. WhenAnotherFileAdded.setUp

AFAICT, this is at the moment only possible (in the execute-all scenario) when using @BeforeEach on all set-up methods, which is rather wasteful as in my case the @Test methods in the same class can easily share their set-up. Alas, @BeforeAll doesn't work due to the above behavior.

I hope this explains things satisfactorily.

sewe avatar Aug 27 '19 12:08 sewe

Tentatively slated for the 5.6 backlog for team discussion and further investigation

sbrannen avatar Aug 27 '19 12:08 sbrannen

Tentatively slated for the 5.6 backlog for team discussion and further investigation

Sounds good.

If you need further input about my use cases, feel free to ask. I absolutely love using nested tests, in particular for things with rather longish set-ups (the repository-use case above, or UI tests). But ideally I want my @Test methods to have just a single assertion, instead verifying unrelated properties in different @Test methods. Alas, I pay a rather heavy @BeforeEach set-up tax for this right now.

sewe avatar Aug 27 '19 12:08 sewe

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. Thank you for your contribution.

stale[bot] avatar May 13 '21 19:05 stale[bot]

Tentatively slated for the 5.6 backlog for team discussion and further investigation

As we are past 5.6 (and 5.7) now, has this ever been discussed? If not, is there any clarifying information you require? Would hate to this this issue closed as stale.

sewe avatar May 17 '21 06:05 sewe

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. Thank you for your contribution.

stale[bot] avatar Jun 19 '22 20:06 stale[bot]

Still interested in this, as @Nested tests are great, but the set up tax we have to pay for them is rather expensive. So I would really appreciate if this could at least be discussed. If you then say "this is too tricky to implement and/or overly complicated," then fine, so be it.

sewe avatar Jun 22 '22 12:06 sewe