fix: disabling cache if scenario-scoped glue is present for #2971

cucumber · Feb 3, 2025 · f3efd5d · f3efd5d
1 parent ff2468c
commit f3efd5d
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 13 deletions.
diff --git a/cucumber-core/src/main/java/io/cucumber/core/runner/CachingGlue.java b/cucumber-core/src/main/java/io/cucumber/core/runner/CachingGlue.java
@@ -87,6 +87,7 @@ final class CachingGlue implements Glue {
     private Locale locale = null;
     private StepExpressionFactory stepExpressionFactory = null;
     private boolean dirtyCache = false;
+    private boolean hasScenarioScopedGlue = false;
 
     CachingGlue(EventBus bus) {
         this.bus = bus;
@@ -108,42 +109,47 @@ public void addAfterAllHook(StaticHookDefinition afterAllHook) {
     public void addStepDefinition(StepDefinition stepDefinition) {
         stepDefinitions.add(stepDefinition);
         dirtyCache = true;
+        hasScenarioScopedGlue |= stepDefinition instanceof ScenarioScoped;
     }
 
     @Override
     public void addBeforeHook(HookDefinition hookDefinition) {
         beforeHooks.add(CoreHookDefinition.create(hookDefinition, bus::generateId));
         beforeHooks.sort(HOOK_ORDER_ASCENDING);
+        hasScenarioScopedGlue |= hookDefinition instanceof ScenarioScoped;
     }
 
     @Override
     public void addAfterHook(HookDefinition hookDefinition) {
         afterHooks.add(CoreHookDefinition.create(hookDefinition, bus::generateId));
         afterHooks.sort(HOOK_ORDER_ASCENDING);
+        hasScenarioScopedGlue |= hookDefinition instanceof ScenarioScoped;
     }
 
     @Override
     public void addBeforeStepHook(HookDefinition hookDefinition) {
         beforeStepHooks.add(CoreHookDefinition.create(hookDefinition, bus::generateId));
         beforeStepHooks.sort(HOOK_ORDER_ASCENDING);
+        hasScenarioScopedGlue |= hookDefinition instanceof ScenarioScoped;
     }
 
     @Override
     public void addAfterStepHook(HookDefinition hookDefinition) {
         afterStepHooks.add(CoreHookDefinition.create(hookDefinition, bus::generateId));
         afterStepHooks.sort(HOOK_ORDER_ASCENDING);
+        hasScenarioScopedGlue |= hookDefinition instanceof ScenarioScoped;
     }
 
     @Override
     public void addParameterType(ParameterTypeDefinition parameterType) {
         parameterTypeDefinitions.add(parameterType);
         dirtyCache = true;
+        hasScenarioScopedGlue |= parameterType instanceof ScenarioScoped;
     }
 
     @Override
     public void addDataTableType(DataTableTypeDefinition dataTableType) {
         dataTableTypeDefinitions.add(dataTableType);
-        dirtyCache = true;
     }
 
     @Override
@@ -169,7 +175,6 @@ public void addDefaultDataTableCellTransformer(
     @Override
     public void addDocStringType(DocStringTypeDefinition docStringType) {
         docStringTypeDefinitions.add(docStringType);
-        dirtyCache = true;
     }
 
     List<StaticHookDefinition> getBeforeAllHooks() {
@@ -246,8 +251,10 @@ void prepareGlue(Locale locale) throws DuplicateStepDefinitionException {
         boolean firstTime = stepTypeRegistry == null;
         boolean languageChanged = !locale.equals(this.locale);
         boolean mustRebuildCache = false;
-        if (firstTime || languageChanged || dirtyCache) {
+        if (firstTime || languageChanged || dirtyCache || hasScenarioScopedGlue) {
             // conditions changed => invalidate the glue cache
+            // Note: we have a prudent approach of avoiding caching if
+            // scenario-scoped glue exist (e.g. cucumber-java8).
             this.locale = locale;
             stepTypeRegistry = new StepTypeRegistry(locale);
             stepExpressionFactory = new StepExpressionFactory(stepTypeRegistry, bus);
@@ -483,14 +490,8 @@ void removeScenarioScopedGlue() {
             dirty = true;
             dirtyCache = true;
         }
-        if (removeScenarioScopedGlue(dataTableTypeDefinitions)) {
-            dirty = true;
-            dirtyCache = true;
-        }
-        if (removeScenarioScopedGlue(docStringTypeDefinitions)) {
-            dirty = true;
-            dirtyCache = true;
-        }
+        dirty |= removeScenarioScopedGlue(dataTableTypeDefinitions);
+        dirty |= removeScenarioScopedGlue(docStringTypeDefinitions);
         if (removeScenarioScopedGlue(parameterTypeDefinitions)) {
             dirty = true;
             dirtyCache = true;
@@ -501,6 +502,7 @@ void removeScenarioScopedGlue() {
         if (dirty) {
             stepDefinitionsByPattern.clear();
         }
+        hasScenarioScopedGlue = false;
     }
 
     private boolean removeScenarioScopedGlue(Iterable<?> glues) {

diff --git a/cucumber-core/src/test/java/io/cucumber/core/runner/CachingGlueTest.java b/cucumber-core/src/test/java/io/cucumber/core/runner/CachingGlueTest.java
@@ -599,7 +599,7 @@ void prepareGlue_cache_evicted_when_dataTableType_added() {
         StepTypeRegistry stepTypeRegistry2 = glue.getStepTypeRegistry();
 
         // Then
-        assertThat(stepTypeRegistry1 != stepTypeRegistry2, is(true));
+        assertThat(stepTypeRegistry1 == stepTypeRegistry2, is(true));
     }
 
     @Test
@@ -614,7 +614,7 @@ void prepareGlue_cache_evicted_when_docString_added() {
         StepTypeRegistry stepTypeRegistry2 = glue.getStepTypeRegistry();
 
         // Then
-        assertThat(stepTypeRegistry1 != stepTypeRegistry2, is(true));
+        assertThat(stepTypeRegistry1 == stepTypeRegistry2, is(true));
     }
 
     private static class MockedScenarioScopedStepDefinition extends StubStepDefinition implements ScenarioScoped {