[Pico] Fixed missing calls to start, stop and dispose handles

CHANGELOG.md

@@ -13,6 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 - [TestNG] Update dependency org.testng:testng to v7.8.0
 
+### Fixed
+- [Pico] Fixed missing calls to start, stop and dispose handles ([#2772](https://github.com/cucumber/cucumber-jvm/pull/2772) Julien Kronegg)
+
 ## [7.12.1] - 2023-06-02
 ### Fixed
 - [Core] Set html report viewport width to device width ([html-formatter/#238](https://github.com/cucumber/html-formatter/pull/238) Tim Yao )

cucumber-picocontainer/src/main/java/io/cucumber/picocontainer/PicoFactory.java

@@ -5,6 +5,7 @@
 import org.picocontainer.MutablePicoContainer;
 import org.picocontainer.PicoBuilder;
 import org.picocontainer.behaviors.Cached;
+import org.picocontainer.lifecycle.DefaultLifecycleState;
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Modifier;
@@ -15,36 +16,45 @@
 public final class PicoFactory implements ObjectFactory {
 
  private final Set<Class<?>> classes = new HashSet<>();
- private final MutablePicoContainer pico = new PicoBuilder()
- .withCaching()
- .withLifecycle()
- .build();
-
- public PicoFactory() {
- this.pico.start();
- }
+ private MutablePicoContainer pico;
 
  private static boolean isInstantiable(Class<?> clazz) {
  boolean isNonStaticInnerClass = !Modifier.isStatic(clazz.getModifiers()) && clazz.getEnclosingClass() != null;
  return Modifier.isPublic(clazz.getModifiers()) && !Modifier.isAbstract(clazz.getModifiers())
  && !isNonStaticInnerClass;
  }
 
+ @Override
  public void start() {
- // do nothing (was already started in constructor)
+ if (pico == null) {
+ pico = new PicoBuilder()
+ .withCaching()
+ .withLifecycle()
+ .build();
+ for (Class<?> clazz : classes) {
+ pico.addComponent(clazz);
+ }
+ } else {
+ // we already get a pico container which is in "disposed" lifecycle,
+ // so recycle it by defining a new lifecycle and removing all
+ // instances
+ pico.setLifecycleState(new DefaultLifecycleState());
+ pico.getComponentAdapters()
+ .forEach(cached -> ((Cached<?>) cached).flush());
+ }
+ pico.start();
  }
 
  @Override
  public void stop() {
- pico.getComponentAdapters()
-  .forEach(cached -> ((Cached<?>) cached).flush());
+ pico.stop();
+ pico.dispose();
  }
 
  @Override
  public boolean addClass(Class<?> clazz) {
  if (isInstantiable(clazz) && classes.add(clazz)) {
  addConstructorDependencies(clazz);
- pico.addComponent(clazz);
  }
  return true;
  }

cucumber-picocontainer/src/test/java/io/cucumber/picocontainer/DisposableCucumberBelly.java

@@ -3,8 +3,11 @@
 import org.picocontainer.Disposable;
 import org.picocontainer.Startable;
 
+import java.util.ArrayList;
 import java.util.List;
 
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
 /**
  * A test helper class which simulates a class that holds system resources which
  * need disposing at the end of the test.
@@ -13,19 +16,20 @@
  */
 public class DisposableCucumberBelly
  implements Disposable, Startable {
+ static final List<String> events = new ArrayList<>();
 
  private List<String> contents;
  private boolean isDisposed = false;
  private boolean wasStarted = false;
  private boolean wasStopped = false;
 
  public List<String> getContents() {
- assert !isDisposed;
+ assertFalse(isDisposed);
  return contents;
  }
 
  public void setContents(List<String> contents) {
- assert !isDisposed;
+ assertFalse(isDisposed);
  this.contents = contents;
  }
 
@@ -36,6 +40,7 @@ public void setContents(List<String> contents) {
  */
  @Override
  public void dispose() {
+ events.add("Disposed");
  isDisposed = true;
  }
 
@@ -45,6 +50,7 @@ public boolean isDisposed() {
 
  @Override
  public void start() {
+ events.add("Started");
  wasStarted = true;
  }
 
@@ -54,6 +60,7 @@ public boolean wasStarted() {
 
  @Override
  public void stop() {
+ events.add("Stopped");
  wasStopped = true;
  }
 

cucumber-picocontainer/src/test/java/io/cucumber/picocontainer/StepDefinitions.java

@@ -1,8 +1,6 @@
 package io.cucumber.picocontainer;
 
-import io.cucumber.java.After;
-import io.cucumber.java.Before;
-import io.cucumber.java.Scenario;
+import io.cucumber.java.*;
 import io.cucumber.java.en.Given;
 import io.cucumber.java.en.Then;
 import io.cucumber.java.en.When;
@@ -11,11 +9,12 @@
 import java.util.Collections;
 import java.util.List;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.*;
 
 public class StepDefinitions {
 
  private final DisposableCucumberBelly belly;
+ private static int scenarioCount = 0;
 
  public StepDefinitions(DisposableCucumberBelly belly) {
  this.belly = belly;
@@ -35,9 +34,37 @@ public void gh20() {
 
  @After
  public void after() {
+ scenarioCount++;
  // We might need to clean up the belly here, if it represented an
  // external resource.
- assert !belly.isDisposed();
+
+ // Call order should be Started > After > Stopped > Disposed, so here we
+ // expect only Started
+ assertTrue(belly.wasStarted());
+ assertFalse(belly.wasStopped());
+ assertFalse(belly.isDisposed());
+ }
+
+ @BeforeAll
+ @SuppressWarnings("unused")
+ public static void beforeAll() {
+ // reset static variables
+ DisposableCucumberBelly.events.clear();
+ scenarioCount = 0;
+ }
+
+ @AfterAll
+ @SuppressWarnings("unused")
+ public static void afterAll() {
+ List<String> events = DisposableCucumberBelly.events;
+ // Call order should be Start > Stopped > Disposed, for each test
+ // scenario
+ assertEquals(3 * scenarioCount, events.size());
+ for (int i = 0; i < scenarioCount; i += 3) {
+ assertEquals("Started", events.get(i));
+ assertEquals("Stopped", events.get(i + 1));
+ assertEquals("Disposed", events.get(i + 2));
+ }
  }
 
  @Given("I have {int} {word} in my belly")