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.

Sign up for GitHub

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

[vividus-engine] Fix logging of deprecated steps without parameters inside composite steps #4682

Merged
merged 1 commit into from
Dec 21, 2023
Merged

[vividus-engine] Fix logging of deprecated steps without parameters inside composite steps #4682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions vividus-engine/src/main/java/org/vividus/replacement/DeprecatedStepNotificationFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.Comparator;
import java.util.Formatter;
import java.util.List;
import java.util.Optional;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

Expand All @@ -45,8 +46,8 @@ public DeprecatedStepNotificationFactory(RunTestContext runTestContext, Configur
protected String createDeprecatedStepNotification(String versionToRemove, String replacementFormatPattern)
{
String runningDeprecatedStep = runTestContext.getRunningStory().getRunningSteps().getFirst();
Matcher stepMatcher = getStepMatcher(runningDeprecatedStep);
String[] stepParams = getParametersFromStep(stepMatcher);
Optional<Matcher> stepMatcher = getStepMatcher(runningDeprecatedStep);
String[] stepParams = stepMatcher.map(this::getParametersFromStep).orElseGet(() -> new String[0]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a fix of the reason, but not root cause, any plans to do root cause analysis?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a fix of the reason, but not root cause, any plans to do root cause analysis?

The root of the problem is that steps within composite steps can be processed by separate logic (without using stepMonitor which we use to collect patterns of the current steps from tests) on the JBehave side.
https://github.com/jbehave/jbehave-core/blob/9510692f56f31551aa52e0eeddc03cc426deaea5/jbehave-core/src/main/java/org/jbehave/core/steps/StepCandidate.java#L242-L243

I'll think about how to make the "monitoring" of matched steps more unified on the JBehave side.


String notification;
try (Formatter newStep = new Formatter())
Expand All @@ -59,7 +60,7 @@ protected String createDeprecatedStepNotification(String versionToRemove, String
return notification;
}

private Matcher getStepMatcher(String rawDeprecatedStep)
private Optional<Matcher> getStepMatcher(String rawDeprecatedStep)
{
List<Matcher> matchers = new ArrayList<>();
Keywords keywords = configuration.keywords();
Expand All @@ -73,7 +74,7 @@ private Matcher getStepMatcher(String rawDeprecatedStep)
matchers.add(matcher);
}
}
return matchers.stream().max(Comparator.comparing(Matcher::groupCount)).orElse(null);
return matchers.stream().max(Comparator.comparing(Matcher::groupCount));
}

private String[] getParametersFromStep(Matcher stepMatcher)
Expand Down
23 changes: 20 additions & 3 deletions ...-engine/src/test/java/org/vividus/replacement/DeprecatedStepNotificationFactoryTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

import org.jbehave.core.configuration.Configuration;
import org.jbehave.core.configuration.Keywords;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
Expand All @@ -42,6 +43,9 @@
@ExtendWith(MockitoExtension.class)
class DeprecatedStepNotificationFactoryTests
{
private static final String DEPRECATION_MSG_FORMAT =
"The step: \"%s\" is deprecated and will be removed in VIVIDUS %s. Use step: \"%s\"";

private static final String REMOVE_VERSION = "0.7.0";

private static final String NOT_PARAMETERIZED_STEP_VALUE = "step without parameters deprecated";
Expand Down Expand Up @@ -93,13 +97,26 @@ void shouldGetNotificationForRunningDeprecatedStep(String deprecatedStepValue, S
when(collectingStepPatternsMonitor.getRegisteredStepPatterns()).thenReturn(stepPatternsCache);

mockRunTestContext(deprecatedStep);
String expectedNotification = String.format(
"The step: \"%s\" is deprecated and will be removed in VIVIDUS %s. Use step: \"%s\"", deprecatedStep,
REMOVE_VERSION, expectedActualStep);
String expectedNotification = String.format(DEPRECATION_MSG_FORMAT, deprecatedStep, REMOVE_VERSION,
expectedActualStep);
assertEquals(expectedNotification, deprecatedStepNotificationFactory
.createDeprecatedStepNotification(REMOVE_VERSION, formatPattern));
}

@Test
void shouldGetNotificationForRunningDeprecatedStepWithoutPattern()
{
Map<String, Pattern> stepPatternsCache = new ConcurrentHashMap<>();
when(configuration.keywords()).thenReturn(new Keywords());
when(collectingStepPatternsMonitor.getRegisteredStepPatterns()).thenReturn(stepPatternsCache);

mockRunTestContext(NOT_PARAMETERIZED_STEP_DEPR);
String expectedNotification = String.format(DEPRECATION_MSG_FORMAT, NOT_PARAMETERIZED_STEP_DEPR, REMOVE_VERSION,
NOT_PARAMETERIZED_STEP_ACTUAL);
assertEquals(expectedNotification, deprecatedStepNotificationFactory
.createDeprecatedStepNotification(REMOVE_VERSION, NOT_PARAMETERIZED_STEP_ACTUAL));
}

private void mockRunTestContext(String deprecatedStep)
{
var runningStory = mock(RunningStory.class);
Expand Down
Loading