Skip to content

Introduce support for before/after callbacks once per entire test run #19

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

Closed
wants to merge 2 commits into from

Conversation

ttddyy
Copy link

@ttddyy ttddyy commented Feb 2, 2016

Initially wrote it here.

Nice to have callback interfaces once per entire test run or engine/tag/etc. (e.g.: @BeforeAllTests/@AfterAllTests)
Would be nice that they receive TestExecutionContext in argument, so that implementations can populate beans which could be resolved/used/injected across all tests.

@sbrannen
Copy link
Member

@ttddyy, this sounds more like an extension point rather than part of the standard programming model.

In other words, if we support global callbacks like this I would envision them being implemented as a TestExtension instead of residing in some class that has methods annotated with @BeforeAllTests/@AfterAllTests.

For example, we could consider introducing an EngineCallbacks API for extensions and then allow them to be registered globally for the engine (e.g., perhaps via the ServiceLoader mechanism, analogous to how we currently detect engines, or via some other means that is not tied to a particular user class).

What are your thoughts?

Cheers,

Sam

@schauder
Copy link
Contributor

schauder commented Dec 1, 2015

+1 for such a feature (no opinion yet about the TestExecutionContext)

@ttddyy
Copy link
Author

ttddyy commented Dec 1, 2015

In other words, if we support global callbacks like this I would envision them being implemented as a TestExtension instead of residing in some class that has methods annotated with @BeforeAllTests/@AfterAllTests.

I think this makes sense since global callbacks will probably not have direct interaction with specific test instances.

For example, we could consider introducing an EngineCallbacks API for extensions and then allow them to be registered globally for the engine (e.g., perhaps via the ServiceLoader mechanism, analogous to how we currently detect engines, or via some other means that is not tied to a particular user class).

Yes, I like the idea using ServiceLoader for global callbacks which align with how engines are registered. Any other implementation would be great as well.

@hwellmann
Copy link

A concrete use case for such "global" listeners: frameworks like Arquillian or Pax Exam that start some sort of test container or server, run any number of test classes within that container and finally shut down the container.

By "global", I mean "for all test classes", not only per engine. E.g. take a mixed collection of JUnit 4 and JUnit 5 test classes - it would not really make sense to start and stop the test server per engine, you'd want to start and stop it globally.

I understand that TestPlan is the JUnit 5 term for "all tests", so it seems a TestPlanExecutionListener would do the job of starting and stopping the test server "globally", in testPlanExecutionStarted() and testPlanExecutionFinished(), respectively.

The only open issue is how to register the TestPlanExecutionListener. At the moment, it seems this can only be done programmatically by invoking Launcher.registerTestPlanExecutionListeners().

The problem is, when running tests from Eclipse or Maven or any other IDE or build tool, the Launcher is not under the user's control, and rather than expecting the environment (Eclipse, Maven, etc.) to discover and register listeners on the Launcher, this should be done centrally by the Launcher itself.

So I suggest the Launcher should not just lookupAllTestEngines(), but also lookupAllTestPlanExecutionListeners() via java.util.ServiceLoader and register them on itself.

@marcphilipp
Copy link
Member

I think registering a TestExecutionListener (as it is called in the current master) using ServiceLoader makes perfect sense.

@codecov-io
Copy link

Current coverage is 87.75%

Merging #19 into master will decrease coverage by -0.33% as of 880739b

@@            master     #19   diff @@
======================================
  Files          143     147     +4
  Stmts         3147    3200    +53
  Branches       327     328     +1
  Methods          0       0       
======================================
+ Hit           2772    2808    +36
- Partial         88      89     +1
- Missed         287     303    +16

Review entire Coverage Diff as of 880739b

Powered by Codecov. Updated on successful CI builds.

@ttddyy
Copy link
Author

ttddyy commented Feb 2, 2016

Hi,

I wrote POC(or a pull request?) for per engine extension point as well as global extension point using ServiceLoader.
I attached the commits to this issue above.

I have mostly focused on how to populate ExtensionContext#Store.
Many times, I had usecases to create objects very beginning of all tests (and may clean-up at the very end).
For example, create a HttpClient and uses(inject) it into all tests(may be resolve to method parameter in junit5 way).

So, it doesn't do anything with TestExecutionListener but may be tweaked to do something about it or have separate mechanism for listeners.

some of the items need to think about:

  • should this be named extension point?
    • mechanism is in a way different from other extension points.
  • which package to place extension point APIs?

Thanks,

@jlink
Copy link
Contributor

jlink commented Feb 2, 2016

I might be missing the right context but a global/per engine callback should IMO be outside the JUnit 5 API so ExtensionContext wouldn't be touched at all. Instead it would live in the launcher or engine module.

Am 02.02.2016 um 09:46 schrieb Tadaya Tsuyukubo [email protected]:

Hi,

I wrote POC(or a pull request) for per engine extension point as well as global extension point using ServiceLoader.
I attache the commit to this issue above.

I have mostly focused on how to populate ExtensionContext#Store.
Many times, I had usecases to create objects very beginning of all tests (and may clean-up at the very end).
For example, create a HttpClient and uses(inject) it into all tests(may be resolve to method parameter in junit5 way).

So, it doesn't do anything with TestExecutionListener but may be tweaked to do something about it or have separate mechanism for listeners.

some of the items need to think about:

should this be named extension point?
mechanism is in a way different from other extension points.
which package to place extension point APIs?
Thanks,


Reply to this email directly or view it on GitHub.

@sbrannen
Copy link
Member

sbrannen commented Feb 2, 2016

@jlink, there are numerous use cases where a global/per engine callback would need to provide something to tests or extensions (e.g., server name, port number, JDBC connection URL, a physical DataSource, etc.).

This is what @ttddyy is talking about with regard to the store in the ExtensionContext. Such a global callback may not interact directly with the ExtensionContext, but it would need a mechanism for propagating information to the tests and extensions.

@sbrannen sbrannen added this to the 5.0 Backlog milestone Feb 2, 2016
@ttddyy
Copy link
Author

ttddyy commented Feb 2, 2016

@sbrannen Thanks for the explanation. Yes, that was my intention.

For global callback, I used ExecutionRequest#getAttributes, which is not used in current master, as a means to pass objects from callback to root level ExtensionContext.

@ttddyy ttddyy force-pushed the issue19-global-extension branch from 65beb90 to e9cc300 Compare February 2, 2016 18:56
@jlink
Copy link
Contributor

jlink commented Feb 4, 2016

Sorry for my premature comment earlier. I had a look at the code now and think I get the basic point.

It's a big change in so far that it introduces a new - and IMO very strong - coupling, namly pushing information, uncontrolled by JUnit, from the outside to an inside test extension. This has a couple of drawbacks:

  • A test extension might only work if a global execution point has succeeded to do its task and put in the proper information using the proper key. This will make it more difficult to run individual tests or part of a test suite.
  • A global extension that does essential things - like starting a web server etc - is doing magic from the POV of the test code itself. A reader will not see this dependency just by reading the code.
  • Some tools, e.g. maven-sure-fire, have means to parallelize test execution by starting individual JVMs. This opens a whole new can of potential problems with global extension points.
  • Registering global extensions using a service provider makes it difficult to switch them on and off in cases of conflict.

I think we'll have to discuss this suggestion thoroughly to see if the advantages outweigh the drawbacks.

@ttddyy
Copy link
Author

ttddyy commented Feb 4, 2016

Aside from how to implement, I believe there are two requirements for this:

  • callback(or extension point) for before/after entire test run
    • may or may not be analogous to @BeforeSuite, @AfterSuite in TestNG
  • having parent/root ExtensionContext (parent of ClassBasedContainerExtensionContext) and populate/pass it

I think these two points can be combined together or can be separated.

Another approach I can think of is having interfaces like this:

// extension point for global before/after
public interface GlobalCallback {
  void beforeExecute(...);
  void afterExecute(...);
}


public interface ExtensionContextSourceProvider {

  // return maybe Map<String, Object> but additionally needs to give id/name of it
  ExtensionContext createTestContext();
}


// this class needs to be registered or discovered in discovery phase
public class MyTestContextProvider implements GlobalCallback, ExtensionContextSourceProvider {

  @Override
  public void beforeExecute(...) {
  }

  @Override
  public TestContext createTestContext() {
     // create context & populate data for ExtensionContext#store
     // the context should have id/name.
     //   ex: "my-context" specified in MyExtensionPoint below
     return ...;
  }

  @Override
  public void afterExecute(...) {
  }

}


@ParentExtensionContext("my-context")  // specify parent context
public class MyExtensionPoint implements BeforeAllExtensionPoint, AfterAllExtensionPoint {

   @Override
   void beforeAll(ContainerExtensionContext context) throws Exception {
       // the given context should have parent context set
   };

   ....
}

All classes that implements GlobalCallback needs to be discovered/registered before test execution, so that it can be invoked before running tests. Any tool that runs test class needs to be aware of the class which implements GlobalCallback. (IDE, surefire, etc when specifying single test class/method to run)

ExtensionContextSourceProvider is to return ExtensionContext or just a map which goes to ExtensionContext#Store.

@ParentExtensionContext annotation is used in class level ExtensionPoint (BeforeAllExtensionPoint or AfterAllExtensionPoint).
When it is used, specified name of ExtensionContext or map(ExtensionContext#Store) would be set as a parent ExtensionContext.
Now ExtensionPoint annotated with @ParentExtensionContext has dependency to the specified context, ExtensionContextSourceProvider needs to be called beforehand, which I think it is natural that all ExtensionContextSourceProvider get invoked before starting tests which overwraps to the feature of GlobalCallback.

Some tools, e.g. maven-sure-fire, have means to parallelize test execution by starting individual JVMs. This opens a whole new can of potential problems with global extension points.

I believe this is certainly a good point to discuss.
I just wonder how it is handling @BeforeSuite and @AfterSuite in TestNG. I haven't verified but my guess is each JVM performs @BeforeSuite and @AfterSuite.

@marcphilipp
Copy link
Member

Closed in favor of #671.

@marcphilipp marcphilipp closed this May 6, 2017
@marcphilipp marcphilipp removed this from the 5.0 M5 milestone May 6, 2017
@RomanIovlev
Copy link

@marcphilipp Is any way in JUnit 5 now to Run some code once per test run? Or you completely deny this ability?

@marcphilipp
Copy link
Member

@RomanIovlev Yes, there is a way to do that: you can write an extension like the following and register it globally.

public class OncePerTestRunExtension implements BeforeAllCallback {
    @Override
    public void beforeAll(ExtensionContext context) throws Exception {
        context.getRoot().getStore(ExtensionContext.Namespace.GLOBAL)
            .getOrComputeIfAbsent(MyFixture.class);
    }

    static class MyFixture implements ExtensionContext.Store.CloseableResource {
        public MyFixture() {
            // setup
        }

        @Override
        public void close() {
            // teardown
        }
    }
}

@RomanIovlev
Copy link

Thanks, but Why you don't want to implement this as Annotation?
Call it BeforeSuite/AfterSuite or BeforeTestRun/AfterTestRun whatever...
In testing this is quite often to have something before all tests in test run and sometimes more important cleanup this data after all tests. code before/after all tests in class not so popular...

@marcphilipp
Copy link
Member

How would we discover classes with such annotations? It sounds like we'd have to scan the classpath even if only a single class was requested.

@SergiiNik
Copy link

SergiiNik commented Jun 1, 2019

hi All!

Above advises do not work for me, so I solved this problem like this:

Add to your Base abstract class (I mean abstract class where you initialize your driver in setUpDriver() method) this part of code:

    private static boolean started = false;
    static{
        if (!started) {
            started = true;
            try {
                setUpDriver();  //method where you initialize your driver
            } catch (MalformedURLException e) {
            }
        }
    }

And now, if your test classes will extends from Base abstract class -> setUpDriver() method will be executed before first @test only ONE time per project run.

@jbwyatt4
Copy link

jbwyatt4 commented Jul 17, 2019

I have written an extension. Just to be sure it isn't a pom.xml issue. How would you add the junit.jupiter.extensions.autodetection.enabled property=true to the maven-surefire-plugin?

This is the XML I have so far in pom.xml

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-surefire-plugin</artifactId>
    <version>3.0.0-M3</version>
    <configuration>
        <systemPropertyVariables>
            <junit.jupiter.extensions.autodetection.enabled>true</junit.jupiter.extensions.autodetection.enabled>
        </systemPropertyVariables>
    </configuration>
</plugin>

No errors reported. The call back before each test is not executed.

@marcphilipp
Copy link
Member

@jbwyatt4 That looks like it should work. Could you debug or print all system properties in your test to see if Surefire hands them to the forked JVM?

@jbwyatt4
Copy link

jbwyatt4 commented Jul 17, 2019

The XML property reports being true.

junit.jupiter.extensions.autodetection.enabled: true

Posting code for extension class:

@ExtendWith({YourJUnitExtension.class})
public class YourJUnitExtension implements BeforeAllCallback, ExtensionContext.Store.CloseableResource {
  private static boolean started = false;
  
  @Override
  public void beforeAll(ExtensionContext context) {
    if (!started) {
      started = true;
      CU.enable(); // custom class to control which print statements print
      CU.print("Print from Extension Context");
      CU.disable();
    }
  }
  
  @Override
  public void close() {
    // Your "after all tests" logic goes here
  }
}

@marcphilipp
Copy link
Member

marcphilipp commented Jul 19, 2019

If so, the beforeAll method should be called. Are you sure it isn't?

However, for close to be called, the resource needs to be added to an ExtensionContext.Store. Moreover, the @ExtendWith on the extension does not make sense as you cannot yet extend extensions and to extend sth. with itself would put you into an endless loop. 😉

This should work and eventually be documented in #1555:

public class YourJUnitExtension implements BeforeAllCallback {

  @Override
  public void beforeAll(ExtensionContext context) {
    ExtensionContext.Store rootStore = context.getRoot().getStore(ExtensionContext.Namespace.GLOBAL);
    rootStore.getOrComputeIfAbsent("myResource", key -> {
      CU.enable(); // custom class to control which print statements print
      CU.print("Print from Extension Context");
      CU.disable();
      return new MyResource();
    });
  }

  static class MyResource implements ExtensionContext.Store.CloseableResource {
    @Override
    public void close() throws Throwable {
      // Your "after all tests" logic goes here
    }
  }
}

@marcphilipp
Copy link
Member

@jbwyatt4 If you have additional questions, please let's continue the discussion in #1555 since this issue is closed.

@jbwyatt4
Copy link

Just got the notification from GitHub on this.

Your code works. I believe you were right about @ExtendWith.

Thank you for your help @marcphilipp.

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

Successfully merging this pull request may close these issues.

10 participants