Skip to content

Commit

Permalink
fix: corrected build + tentative of caching glue for #2902
Browse files Browse the repository at this point in the history
  • Loading branch information
U117293 authored and U117293 committed Oct 16, 2024
1 parent ef7be7f commit 9521ea1
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 40 deletions.
125 changes: 104 additions & 21 deletions cucumber-core/src/main/java/io/cucumber/core/runner/CachingGlue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,15 @@
import io.cucumber.core.stepexpression.StepExpressionFactory;
import io.cucumber.core.stepexpression.StepTypeRegistry;
import io.cucumber.cucumberexpressions.CucumberExpression;
import io.cucumber.cucumberexpressions.DuplicateTypeNameException;
import io.cucumber.cucumberexpressions.Expression;
import io.cucumber.cucumberexpressions.ParameterByTypeTransformer;
import io.cucumber.cucumberexpressions.ParameterType;
import io.cucumber.cucumberexpressions.RegularExpression;
import io.cucumber.datatable.DuplicateTypeException;
import io.cucumber.datatable.TableCellByTypeTransformer;
import io.cucumber.datatable.TableEntryByTypeTransformer;
import io.cucumber.docstring.CucumberDocStringException;
import io.cucumber.messages.types.Envelope;
import io.cucumber.messages.types.Hook;
import io.cucumber.messages.types.JavaMethod;
Expand Down Expand Up @@ -83,6 +86,7 @@ final class CachingGlue implements Glue {

private final EventBus bus;
private StepTypeRegistry stepTypeRegistry;
private Locale locale = null;

CachingGlue(EventBus bus) {
this.bus = bus;
Expand Down Expand Up @@ -231,21 +235,98 @@ List<DocStringTypeDefinition> getDocStringTypeDefinitions() {
}

StepTypeRegistry getStepTypeRegistry() {
return null;
return stepTypeRegistry;
}
int parameterTypeDefinitionsHashCode = 0;
int stepDefinitionsHashCode = 0;
int dataTableTypeDefinitionsHashCode = 0;
int docStringTypeDefinitionsHashCode = 0;

StepExpressionFactory stepExpressionFactory = null;
void prepareGlue(Locale locale) throws DuplicateStepDefinitionException {
stepTypeRegistry = new StepTypeRegistry(locale);
StepExpressionFactory stepExpressionFactory = new StepExpressionFactory(stepTypeRegistry, bus);
int parameterTypeDefinitionsHashCodeNew = parameterTypeDefinitions.hashCode();
int dataTableTypeDefinitionsHashCodeNew = dataTableTypeDefinitions.hashCode();
int docStringTypeDefinitionsHashCodeNew = docStringTypeDefinitions.hashCode();
int stepDefinitionsHashCodeNew = stepDefinitions.hashCode();
boolean firstTime = stepTypeRegistry == null;
boolean languageChanged = firstTime || !locale.equals(this.locale);
boolean parameterTypesDefinitionsChanged = parameterTypeDefinitionsHashCode != parameterTypeDefinitionsHashCodeNew
|| parameterTypeDefinitionsHashCode == 0;
boolean docStringTypeDefinitionsChanged = docStringTypeDefinitionsHashCode != docStringTypeDefinitionsHashCodeNew
||
docStringTypeDefinitionsHashCode == 0;
boolean dataTableTypeDefinitionsChanged = dataTableTypeDefinitionsHashCode != dataTableTypeDefinitionsHashCodeNew
||
dataTableTypeDefinitionsHashCode == 0;
boolean stepDefinitionsChanged = stepDefinitionsHashCodeNew != stepDefinitionsHashCode
|| stepDefinitionsHashCode == 0;
if (firstTime || languageChanged ||
parameterTypesDefinitionsChanged || stepDefinitionsChanged ||
dataTableTypeDefinitionsChanged || docStringTypeDefinitionsChanged) {
// conditions changed => invalidate the glue cache
this.locale = locale;
stepTypeRegistry = new StepTypeRegistry(locale);
stepExpressionFactory = new StepExpressionFactory(stepTypeRegistry, bus);
parameterTypesDefinitionsChanged = true;
stepDefinitionsChanged = true;
dataTableTypeDefinitionsChanged = true;
docStringTypeDefinitionsChanged = true;
parameterTypeDefinitionsHashCode = parameterTypeDefinitionsHashCodeNew;
dataTableTypeDefinitionsHashCode = dataTableTypeDefinitionsHashCodeNew;
docStringTypeDefinitionsHashCode = docStringTypeDefinitionsHashCodeNew;
stepDefinitionsHashCode = stepDefinitionsHashCodeNew;
}

stepDefinitionsChanged = true; // TODO comment this line to make build
// fail, e.g. due to
// io.cucumber.core.plugin.PrettyFormatterTest.should_handle_scenario_outline

// TODO: separate prepared and unprepared glue into different classes
parameterTypeDefinitions.forEach(ptd -> {
ParameterType<?> parameterType = ptd.parameterType();
stepTypeRegistry.defineParameterType(parameterType);
emitParameterTypeDefined(ptd);
});
dataTableTypeDefinitions.forEach(dtd -> stepTypeRegistry.defineDataTableType(dtd.dataTableType()));
docStringTypeDefinitions.forEach(dtd -> stepTypeRegistry.defineDocStringType(dtd.docStringType()));
if (parameterTypesDefinitionsChanged) {
// parameters changed from the previous scenario => re-register them
parameterTypeDefinitions.forEach(ptd -> {
try {
ParameterType<?> parameterType = ptd.parameterType();
stepTypeRegistry.defineParameterType(parameterType);
emitParameterTypeDefined(ptd);
} catch (DuplicateTypeNameException e) {
// do nothing (intentionally)
// FIXME catching an exception is a dirty hack, so this
// should
// be replaced by another mechanism (this would probably
// require
// to change StepTypeRegistry API)
}
});
}
if (dataTableTypeDefinitionsChanged) {
dataTableTypeDefinitions.forEach(dtd -> {
try {
stepTypeRegistry.defineDataTableType(dtd.dataTableType());
} catch (DuplicateTypeException e) {
// do nothing (intentionally)
// FIXME catching an exception is a dirty hack, so this
// should
// be replaced by another mechanism (this would probably
// require
// to change StepTypeRegistry API)
}
});
}
if (docStringTypeDefinitionsChanged) {
docStringTypeDefinitions.forEach(dtd -> {
try {
stepTypeRegistry.defineDocStringType(dtd.docStringType());
} catch (CucumberDocStringException e) {
// do nothing (intentionally)
// FIXME catching an exception is a dirty hack, so this
// should
// be replaced by another mechanism (this would probably
// require
// to change StepTypeRegistry API)
}
});
}

if (defaultParameterTransformers.size() == 1) {
DefaultParameterTransformerDefinition definition = defaultParameterTransformers.get(0);
Expand Down Expand Up @@ -276,17 +357,19 @@ void prepareGlue(Locale locale) throws DuplicateStepDefinitionException {
beforeHooks.forEach(this::emitHook);
beforeStepHooks.forEach(this::emitHook);

stepDefinitions.forEach(stepDefinition -> {
StepExpression expression = stepExpressionFactory.createExpression(stepDefinition);
CoreStepDefinition coreStepDefinition = new CoreStepDefinition(bus.generateId(), stepDefinition,
expression);
CoreStepDefinition previous = stepDefinitionsByPattern.get(stepDefinition.getPattern());
if (previous != null) {
throw new DuplicateStepDefinitionException(previous, stepDefinition);
}
stepDefinitionsByPattern.put(coreStepDefinition.getExpression().getSource(), coreStepDefinition);
emitStepDefined(coreStepDefinition);
});
if (stepDefinitionsChanged) {
stepDefinitions.forEach(stepDefinition -> {
StepExpression expression = stepExpressionFactory.createExpression(stepDefinition);
CoreStepDefinition coreStepDefinition = new CoreStepDefinition(bus.generateId(), stepDefinition,
expression);
CoreStepDefinition previous = stepDefinitionsByPattern.get(stepDefinition.getPattern());
if (previous != null) {
throw new DuplicateStepDefinitionException(previous, stepDefinition);
}
stepDefinitionsByPattern.put(coreStepDefinition.getExpression().getSource(), coreStepDefinition);
emitStepDefined(coreStepDefinition);
});
}

afterStepHooks.forEach(this::emitHook);
afterHooks.forEach(this::emitHook);
Expand Down
38 changes: 19 additions & 19 deletions cucumber-core/src/test/java/io/cucumber/core/runner/CachingGlueTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import io.cucumber.core.gherkin.Feature;
import io.cucumber.core.gherkin.Step;
import io.cucumber.core.runtime.TimeServiceEventBus;
import io.cucumber.core.stepexpression.StepTypeRegistry;
import io.cucumber.cucumberexpressions.ParameterByTypeTransformer;
import io.cucumber.cucumberexpressions.ParameterType;
import io.cucumber.datatable.DataTable;
Expand All @@ -33,6 +32,7 @@
import java.time.Clock;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import java.util.UUID;
import java.util.stream.Collectors;
Expand All @@ -53,7 +53,7 @@

class CachingGlueTest {

private final StepTypeRegistry stepTypeRegistry = new StepTypeRegistry(ENGLISH);
public static final Locale LANGUAGE = ENGLISH;
private final CachingGlue glue = new CachingGlue(new TimeServiceEventBus(Clock.systemUTC(), UUID::randomUUID));

@Test
Expand All @@ -70,7 +70,7 @@ void throws_duplicate_error_on_dupe_stepdefs() {

DuplicateStepDefinitionException exception = assertThrows(
DuplicateStepDefinitionException.class,
() -> glue.prepareGlue(stepTypeRegistry));
() -> glue.prepareGlue(LANGUAGE));
assertThat(exception.getMessage(), equalTo("Duplicate step definitions in foo.bf:10 and bar.bf:90"));
}

Expand All @@ -81,7 +81,7 @@ void throws_on_duplicate_default_parameter_transformer() {

DuplicateDefaultParameterTransformers exception = assertThrows(
DuplicateDefaultParameterTransformers.class,
() -> glue.prepareGlue(stepTypeRegistry));
() -> glue.prepareGlue(LANGUAGE));
assertThat(exception.getMessage(), equalTo("" +
"There may not be more then one default parameter transformer. Found:\n" +
" - mocked default parameter transformer\n" +
Expand All @@ -95,7 +95,7 @@ void throws_on_duplicate_default_table_entry_transformer() {

DuplicateDefaultDataTableEntryTransformers exception = assertThrows(
DuplicateDefaultDataTableEntryTransformers.class,
() -> glue.prepareGlue(stepTypeRegistry));
() -> glue.prepareGlue(LANGUAGE));
assertThat(exception.getMessage(), equalTo("" +
"There may not be more then one default data table entry. Found:\n" +
" - mocked default data table entry transformer\n" +
Expand All @@ -109,7 +109,7 @@ void throws_on_duplicate_default_table_cell_transformer() {

DuplicateDefaultDataTableCellTransformers exception = assertThrows(
DuplicateDefaultDataTableCellTransformers.class,
() -> glue.prepareGlue(stepTypeRegistry));
() -> glue.prepareGlue(LANGUAGE));
assertThat(exception.getMessage(), equalTo("" +
"There may not be more then one default table cell transformers. Found:\n" +
" - mocked default data table cell transformer\n" +
Expand All @@ -135,7 +135,7 @@ void removes_glue_that_is_scenario_scoped() {
glue.addDefaultDataTableCellTransformer(new MockedDefaultDataTableCellTransformer());
glue.addDefaultDataTableEntryTransformer(new MockedDefaultDataTableEntryTransformer());

glue.prepareGlue(stepTypeRegistry);
glue.prepareGlue(LANGUAGE);

assertAll(
() -> assertThat(glue.getStepDefinitions().size(), is(equalTo(1))),
Expand Down Expand Up @@ -191,7 +191,7 @@ void returns_match_from_cache_if_single_found() throws AmbiguousStepDefinitionsE
StepDefinition stepDefinition2 = new MockedStepDefinition("^pattern2");
glue.addStepDefinition(stepDefinition1);
glue.addStepDefinition(stepDefinition2);
glue.prepareGlue(stepTypeRegistry);
glue.prepareGlue(LANGUAGE);

URI uri = URI.create("file:path/to.feature");
String stepText = "pattern1";
Expand Down Expand Up @@ -219,7 +219,7 @@ void returns_match_from_cache_for_step_with_table() throws AmbiguousStepDefiniti
StepDefinition stepDefinition2 = new MockedStepDefinition("^pattern2", DataTable.class);
glue.addStepDefinition(stepDefinition1);
glue.addStepDefinition(stepDefinition2);
glue.prepareGlue(stepTypeRegistry);
glue.prepareGlue(LANGUAGE);

URI uri = URI.create("file:path/to.feature");
String stepText = "pattern1";
Expand Down Expand Up @@ -260,7 +260,7 @@ void returns_match_from_cache_for_ste_with_doc_string() throws AmbiguousStepDefi
StepDefinition stepDefinition2 = new MockedStepDefinition("^pattern2", String.class);
glue.addStepDefinition(stepDefinition1);
glue.addStepDefinition(stepDefinition2);
glue.prepareGlue(stepTypeRegistry);
glue.prepareGlue(LANGUAGE);

URI uri = URI.create("file:path/to.feature");
String stepText = "pattern1";
Expand Down Expand Up @@ -304,7 +304,7 @@ void returns_fresh_match_from_cache_after_evicting_scenario_scoped() throws Ambi

StepDefinition stepDefinition1 = new MockedScenarioScopedStepDefinition("^pattern1");
glue.addStepDefinition(stepDefinition1);
glue.prepareGlue(stepTypeRegistry);
glue.prepareGlue(LANGUAGE);

PickleStepDefinitionMatch pickleStepDefinitionMatch = glue.stepDefinitionMatch(uri, pickleStep1);
assertThat(((CoreStepDefinition) pickleStepDefinitionMatch.getStepDefinition()).getStepDefinition(),
Expand All @@ -314,7 +314,7 @@ void returns_fresh_match_from_cache_after_evicting_scenario_scoped() throws Ambi

StepDefinition stepDefinition2 = new MockedScenarioScopedStepDefinition("^pattern1");
glue.addStepDefinition(stepDefinition2);
glue.prepareGlue(stepTypeRegistry);
glue.prepareGlue(LANGUAGE);

PickleStepDefinitionMatch pickleStepDefinitionMatch2 = glue.stepDefinitionMatch(uri, pickleStep1);
assertThat(((CoreStepDefinition) pickleStepDefinitionMatch2.getStepDefinition()).getStepDefinition(),
Expand Down Expand Up @@ -347,7 +347,7 @@ void disposes_of_scenario_scoped_beans() {
MockedDefaultParameterTransformer defaultParameterTransformer = new MockedDefaultParameterTransformer();
glue.addDefaultParameterTransformer(defaultParameterTransformer);

glue.prepareGlue(stepTypeRegistry);
glue.prepareGlue(LANGUAGE);
glue.removeScenarioScopedGlue();

assertThat(stepDefinition.isDisposed(), is(true));
Expand All @@ -371,15 +371,15 @@ void returns_no_match_after_evicting_scenario_scoped() throws AmbiguousStepDefin

StepDefinition stepDefinition1 = new MockedScenarioScopedStepDefinition("^pattern1");
glue.addStepDefinition(stepDefinition1);
glue.prepareGlue(stepTypeRegistry);
glue.prepareGlue(LANGUAGE);

PickleStepDefinitionMatch pickleStepDefinitionMatch = glue.stepDefinitionMatch(uri, pickleStep1);
assertThat(((CoreStepDefinition) pickleStepDefinitionMatch.getStepDefinition()).getStepDefinition(),
is(equalTo(stepDefinition1)));

glue.removeScenarioScopedGlue();

glue.prepareGlue(stepTypeRegistry);
glue.prepareGlue(LANGUAGE);

PickleStepDefinitionMatch pickleStepDefinitionMatch2 = glue.stepDefinitionMatch(uri, pickleStep1);
assertThat(pickleStepDefinitionMatch2, nullValue());
Expand All @@ -393,7 +393,7 @@ void throws_ambiguous_steps_def_exception_when_many_patterns_match() {
glue.addStepDefinition(stepDefinition1);
glue.addStepDefinition(stepDefinition2);
glue.addStepDefinition(stepDefinition3);
glue.prepareGlue(stepTypeRegistry);
glue.prepareGlue(LANGUAGE);

URI uri = URI.create("file:path/to.feature");

Expand Down Expand Up @@ -480,7 +480,7 @@ void emits_hook_messages_to_bus() {
glue.addBeforeStepHook(new MockedScenarioScopedHookDefinition());
glue.addAfterStepHook(new MockedScenarioScopedHookDefinition());

glue.prepareGlue(stepTypeRegistry);
glue.prepareGlue(LANGUAGE);
assertThat(events.size(), is(4));
}

Expand All @@ -497,7 +497,7 @@ void parameterTypeDefinition_without_source_reference_emits_parameterType_with_e
glue.addParameterType(new MockedParameterTypeDefinition());

// When
glue.prepareGlue(stepTypeRegistry);
glue.prepareGlue(LANGUAGE);

// Then
assertThat(events.size(), is(1));
Expand All @@ -519,7 +519,7 @@ void parameterTypeDefinition_with_source_reference_emits_parameterType_with_non_
glue.addParameterType(new MockedParameterTypeDefinitionWithSourceReference());

// When
glue.prepareGlue(stepTypeRegistry);
glue.prepareGlue(LANGUAGE);

// Then
assertThat(events.size(), is(1));
Expand Down

0 comments on commit 9521ea1

Please sign in to comment.