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

Document that @BeforeAll & @AfterAll are unsupported in @Nested test classes #88

Closed
3 tasks done
sbrannen opened this issue Jan 5, 2016 · 8 comments
Closed
3 tasks done
Assignees
Milestone

Comments

@sbrannen
Copy link
Member

sbrannen commented Jan 5, 2016

Status Quo

Since support for configuring the test instance lifecycle was dropped after the prototype, @BeforeAll and @AfterAll methods are now required to be static. Consequently such methods can no longer be used in @Nested test classes. Any attempt to do so will result in a compilation error similar to the following.

The method myBeforeAllMethod cannot be declared static; static methods can only be declared in a static or top level type.

See the nested test classes in com.example.HierarchyTestCase for concrete examples.

Deliverables

  • Decide how to handle @BeforeAll and @AfterAll methods in nested test classes.
    • The team decided that this is simply not supported since the Java language doesn't support it.
  • Document that @BeforeAll and @AfterAll are unsupported in nested test classes.
  • Delete com.example.HierarchyTestCase.
@sbrannen sbrannen added this to the Alpha 1 milestone Jan 5, 2016
sbrannen added a commit that referenced this issue Jan 5, 2016
This commit comments out @BeforeAll/@afterall declarations in @nested
test classes until issue #88 is resolved.

Issue: #88

------------------------------------------------------------------------
On behalf of the community, the JUnit Lambda Team thanks msg systems ag
(http://www.msg-systems.com) for supporting the JUnit crowdfunding
campaign!
------------------------------------------------------------------------
@sbrannen
Copy link
Member Author

sbrannen commented Jan 5, 2016

@junit-team/junit-lambda, this is a gotcha that we will need to discuss amongst the group.

@jlink
Copy link
Contributor

jlink commented Jan 5, 2016

I see several options. No need for desperation ;)

The simplest thing might just be to disallow @Before/AfterAll in nested tests since nested tests IMO are a means to group tests by their domain context not by their technical context. If someone really really needs a before/afterAll for technical reasons, they can easily use an extension.

@sbrannen sbrannen self-assigned this Jan 12, 2016
@sbrannen sbrannen changed the title @BeforeAll & @AfterAll cannot be used in nested test classes Document that @BeforeAll & @AfterAll are unsupported in nested test classes Jan 20, 2016
@sbrannen sbrannen changed the title Document that @BeforeAll & @AfterAll are unsupported in nested test classes Document that @BeforeAll & @AfterAll are unsupported in @Nested test classes Jan 20, 2016
sbrannen added a commit that referenced this issue Jan 20, 2016
sbrannen added a commit that referenced this issue Jan 20, 2016
@sbrannen
Copy link
Member Author

Resolve in master. See associated commits for details.

@jlink jlink reopened this Jan 27, 2016
@jlink jlink closed this as completed Jan 27, 2016
@mkobit
Copy link
Contributor

mkobit commented Jul 7, 2016

After reading through this and trying to look at the other issues, does it still make sense that @BeforeAll and @AfterAll have to be static?

@sbrannen
Copy link
Member Author

Without "test instance per test class" semantics it is required that @BeforeAll and @AfterAll methods be static. See the description of #48 for details.

However, once #48 is resolved, the static restriction will possibly be removed for scenarios in which that makes sense.

@silkentrance
Copy link

silkentrance commented Jan 30, 2018

@sbrannen a valid position against @BeforeAll and @AfterAll being static would be the Spring Framework or any such framework that provides for dependency injection of either configuration or components or services used during the tests.

And, considering that during @BeforeAll one would establish a required set of data (aka fixtures) in for example a database, one cannot accomplish that when using static methods unless one would set up a spring ioc before hand and use that only for the purpose of the @BeforeAll and @AfterAll, aka the test set up and tear down methods.

I know there exists the SpringExtension, however, this only works on the test instance, and the static versions of both @BeforeAll and @AfterAll are not supported here.

@silkentrance
Copy link

silkentrance commented Jan 30, 2018

@sbrannen and i do not see why we would need the Scenario abstraction as proposed by #48. the latter seems to be a poor man's implementation of what cucumber already provides. And we do not need such functionality in the core.

The main reason for @Before/AfterAll are still static is the fact that on instance level one will override existing behaviour inherited from a base test class, e.g.

class BaseTest {
  @BeforeAll
  public static void setUp() { ... }

  @AfterAll
  public static void tearDown() { ... }

  @BeforeAll
  public void setUp() { ... }

  @AfterAll
  public void tearDown() { ... }
}

public ConcreteTest extends BaseTest {
  @BeforeAll
  public static void setUp() { ... }

  @AfterAll
  public static void tearDown() { ... }

  @BeforeAll
  public void setUp() { ... super.setUp(); ... }

  @AfterAll
  public void tearDown() { ... super.tearDown(); ... }
}

So here, the concrete test class' setUp method will override the base test class' method.
Why not use standard java idioms here? Allow the derived class to either call upon super or simply override its behaviour.

And given the new feature of nested tests, why not extend this feature to nested tests also?
Having both instance level @Before/AfterAll and static versions of both?

Traversing the inheritance hierarchy from top to bottom first, level by level, resolving all static versions of @BeforeAll, and, on each level, allowing the user to establish a breadth first resolve of the @BeforeAll on instance level by calling super or not. The same with @AfterAll on both static methods and instance methods, again allowing the user to call upon super or not, however this time it will resolve the @AfterAll from the lowest level in the inheritance hierarchy, traversing upwards.

And this should also encompass extensions, which would then always provide their @Before/AfterAlls on a static level.

And if the user messes up, who cares? Either they get it or they don't.

So, in the above example, the before all / after all would be called in the following fashion

BaseTest.setUp
ConcreteTest.setUp
ConcreteTestInstance.setUp -> BaseTestInstance.setUp

... running tests here

ConcreteTestInstance.tearDown -> BaseTestInstance.tearDown
ConcreteTest.tearDown
BaseTest.tearDown

Having such a feature will allow us to use auto wired instances of either configurations a/o services a/o components during both instance level tear down and set up without us having to provide for such mechanisms at the static level, requiring additional and mostly redundant code.

@sbrannen
Copy link
Member Author

Hi @silkentrance,

For starters, this issue was only pertinent to the Alpha release of JUnit 5 and was closed on Jan 20, 2016 (over two years ago).

More importantly, this issue was superseded by #419 which was closed on Jun 30, 2017.

Thus, it has been possible to have non-static @BeforeAll and @AfterAll methods for quite some time now.

Please inform yourself of the status quo in JUnit Jupiter 5.0.x, and if you have further concerns please open a new issue to address them.

I will now lock this conversation since it pertains to an outdated version of the framework.

@junit-team junit-team locked as resolved and limited conversation to collaborators Jan 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants