From ee2d3f32a68a9d3e5a94bb33bafcc1b1a51e2e21 Mon Sep 17 00:00:00 2001 From: CXwudi Date: Wed, 26 Oct 2022 21:53:54 -0400 Subject: [PATCH] [code+test] fix an issue where a pair of stepped literal and the step definition got the first difference from the parameter, a hard error is thrown. should close #12 --- .../cucumber/builder/JFeatureBuilder.java | 16 +++++-- .../builder/JStepParameterExtractor.java | 29 ++++++++++--- .../JStepParameterExtractorRegexTest.java | 38 +++++++++++++++-- .../pirates/BugFixedFromIssueBoardTest.java | 28 +++++++++++++ .../sample/pirates/MismatchBugStepDef.java | 18 +++++--- .../cucumber/sample/pirates/MismatchTest.java | 19 --------- .../sample/pirates/SimilarStepBugStepDef.java | 41 ++++++++++++++++++ .../SimilarStepBugStepDefWithFailure.java | 42 +++++++++++++++++++ .../jfeature/pirates/mismatch-bug.jfeature | 2 +- .../jfeature/pirates/similar-step.jfeature | 12 ++++++ 10 files changed, 207 insertions(+), 38 deletions(-) create mode 100644 src/test/java/scs/comp5903/cucumber/sample/pirates/BugFixedFromIssueBoardTest.java delete mode 100644 src/test/java/scs/comp5903/cucumber/sample/pirates/MismatchTest.java create mode 100644 src/test/java/scs/comp5903/cucumber/sample/pirates/SimilarStepBugStepDef.java create mode 100644 src/test/java/scs/comp5903/cucumber/sample/pirates/SimilarStepBugStepDefWithFailure.java create mode 100644 src/test/resources/sample/jfeature/pirates/similar-step.jfeature diff --git a/src/main/java/scs/comp5903/cucumber/builder/JFeatureBuilder.java b/src/main/java/scs/comp5903/cucumber/builder/JFeatureBuilder.java index 4ac62dc..ef7ff6e 100644 --- a/src/main/java/scs/comp5903/cucumber/builder/JFeatureBuilder.java +++ b/src/main/java/scs/comp5903/cucumber/builder/JFeatureBuilder.java @@ -86,9 +86,19 @@ private HashMap mapAllStepsToStepDefMethods(Set new EasyCucumberException(ErrorCode.EZCU013, "Step definition not found for: " + step - + ". Are you sure you implemented this step definition? Or did you forget to make the method public? " - + "Or did you forget to add the correct annotation to method?") + () -> { + log.error("No matching step definition found for step: {}", step); + var expMsg = String.format( + "Step definition not found for: %s. There can be many reasons for this failure, please check that:\n" + + "1. Are you sure you implemented this step definition?\n" + + "2. Did you forget to make the method public?\n" + + "3. Did you forget to add the correct annotation to method?\n" + + "4. Does the string in your annotation matches the step?\n" + + "5. Are all parameters declared in the step definition method correct?" + , step + ); + return new EasyCucumberException(ErrorCode.EZCU013, expMsg); + } ); results.put(step, matchResult); } diff --git a/src/main/java/scs/comp5903/cucumber/builder/JStepParameterExtractor.java b/src/main/java/scs/comp5903/cucumber/builder/JStepParameterExtractor.java index 9ad0c28..be6d51b 100644 --- a/src/main/java/scs/comp5903/cucumber/builder/JStepParameterExtractor.java +++ b/src/main/java/scs/comp5903/cucumber/builder/JStepParameterExtractor.java @@ -1,5 +1,6 @@ package scs.comp5903.cucumber.builder; +import org.slf4j.Logger; import scs.comp5903.cucumber.model.JStepDefMethodDetail; import scs.comp5903.cucumber.model.exception.EasyCucumberException; import scs.comp5903.cucumber.model.exception.ErrorCode; @@ -13,6 +14,8 @@ import java.util.Optional; import java.util.regex.Pattern; +import static org.slf4j.LoggerFactory.getLogger; + /** * It tries to match the {@link AbstractJStep} with the {@link JStepDefMethodDetail}
* if they match, it will extract out parameters @@ -21,6 +24,9 @@ * @date 2022-06-29 */ public class JStepParameterExtractor { + + private static final Logger log = getLogger(JStepParameterExtractor.class); + /** * simply match the digits with an optional minus sign */ @@ -108,10 +114,12 @@ Optional extractParameterValueAndGetNextIndex(String jStepStr, int j, C var matcher = INTEGER_PATTERN.matcher(jStepSubStr); var found = matcher.find(0); if (!found) { - throw new EasyCucumberException(ErrorCode.EZCU009, "Unable to find integer parameter as int: " + jStepSubStr); + log.debug(" Unable to find integer parameter as int from '{}'. Likely the step '{}' is not the matching step", jStepSubStr, jStepStr); + return Optional.empty(); } var intStr = matcher.group(); if (jStepSubStr.indexOf(intStr) != 0) { // failed to match from index 0 == miss match + log.debug(" The found int string '{}' is not at the beginning of the string '{}'. Likely the step '{}' is not the matching step", intStr, jStepSubStr, jStepStr); return Optional.empty(); } else { parameters.add(Integer.parseInt(intStr)); @@ -121,10 +129,12 @@ Optional extractParameterValueAndGetNextIndex(String jStepStr, int j, C matcher = FLOATING_POINT_PATTERN.matcher(jStepSubStr); found = matcher.find(0); if (!found) { - throw new EasyCucumberException(ErrorCode.EZCU027, "Unable to find floating point parameter as double: " + jStepSubStr); + log.debug(" Unable to find floating point parameter as double from '{}'. Likely the step '{}' is not the matching step", jStepSubStr, jStepStr); + return Optional.empty(); } var doubleStr = matcher.group(); if (jStepSubStr.indexOf(doubleStr) != 0) { // failed to match from index 0 == miss match + log.debug(" The found double string '{}' is not at the beginning of the string '{}'. Likely the step '{}' is not the matching step", doubleStr, jStepSubStr, jStepStr); return Optional.empty(); } else { parameters.add(Double.parseDouble(doubleStr)); @@ -134,11 +144,14 @@ Optional extractParameterValueAndGetNextIndex(String jStepStr, int j, C matcher = STRING_PATTERN.matcher(jStepSubStr); found = matcher.find(0); if (!found) { - throw new EasyCucumberException(ErrorCode.EZCU010, "Unable to find string literals: " + jStepSubStr + - ", the string literal should be in double or single quotes."); + log.debug(" Unable to find string parameter from '{}'. Likely the step '{}' is not the matching step, " + + "or maybe the user failed to add double/single quotes around the string literal", jStepSubStr, jStepStr); + return Optional.empty(); } var matchedStr = matcher.group(); if (jStepSubStr.indexOf(matchedStr) != 0) { // failed to match from index 0 == miss match + log.debug(" The found string literal '{}' is not at the beginning of the string '{}'. Likely the step '{}' is not the matching step, " + + "or maybe the user failed to add double/single quotes around the string literal", matchedStr, jStepSubStr, jStepStr); return Optional.empty(); } else { parameters.add(matchedStr.substring(1, matchedStr.length() - 1)); @@ -148,10 +161,12 @@ Optional extractParameterValueAndGetNextIndex(String jStepStr, int j, C matcher = INTEGER_PATTERN.matcher(jStepSubStr); found = matcher.find(0); if (!found) { - throw new EasyCucumberException(ErrorCode.EZCU026, "Unable to find integer parameter as big integer: " + jStepSubStr); + log.debug(" Unable to find integer parameter as big integer from '{}'. Likely the step '{}' is not the matching step", jStepSubStr, jStepStr); + return Optional.empty(); } intStr = matcher.group(); if (jStepSubStr.indexOf(intStr) != 0) { // failed to match from index 0 == miss match + log.debug(" The found big integer string '{}' is not at the beginning of the string '{}'. Likely the step '{}' is not the matching step", intStr, jStepSubStr, jStepStr); return Optional.empty(); } else { parameters.add(new BigInteger(intStr)); @@ -161,10 +176,12 @@ Optional extractParameterValueAndGetNextIndex(String jStepStr, int j, C matcher = FLOATING_POINT_PATTERN.matcher(jStepSubStr); found = matcher.find(0); if (!found) { - throw new EasyCucumberException(ErrorCode.EZCU028, "Unable to find floating point parameter as big decimal: " + jStepSubStr); + log.debug(" Unable to find floating point parameter as big decimal from '{}'. Likely the step '{}' is not the matching step", jStepSubStr, jStepStr); + return Optional.empty(); } doubleStr = matcher.group(); if (jStepSubStr.indexOf(doubleStr) != 0) { // failed to match from index 0 == miss match + log.debug(" The found big decimal string '{}' is not at the beginning of the string '{}'. Likely the step '{}' is not the matching step", doubleStr, jStepSubStr, jStepStr); return Optional.empty(); } else { parameters.add(new BigDecimal(doubleStr)); diff --git a/src/test/java/scs/comp5903/cucumber/builder/JStepParameterExtractorRegexTest.java b/src/test/java/scs/comp5903/cucumber/builder/JStepParameterExtractorRegexTest.java index 0f1b619..e7ed9c5 100644 --- a/src/test/java/scs/comp5903/cucumber/builder/JStepParameterExtractorRegexTest.java +++ b/src/test/java/scs/comp5903/cucumber/builder/JStepParameterExtractorRegexTest.java @@ -1,12 +1,16 @@ package scs.comp5903.cucumber.builder; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; import java.math.BigDecimal; import java.math.BigInteger; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.stream.Stream; @@ -19,12 +23,13 @@ class JStepParameterExtractorRegexTest { private final JStepParameterExtractor jStepParameterExtractor = new JStepParameterExtractor(); - static Stream argumentsStream() { + static Stream argumentsStreamForSuccessScenarios() { return Stream.of( // input: String jStepStr, int j, Character endingChar, String parameterType, // expected output: Object expectedExtractedValue, int expectedNextIndex, Class expectedClass Arguments.of("I have a \"string1\" and an int 5", 9, ' ', "string", "string1", 18, String.class), Arguments.of("I have a \"string1\" and another string \"string2\"", 9, ' ', "string", "string1", 18, String.class), + Arguments.of("I have a \"string1\" and another string \"string2\"", 38, null, "string", "string2", 47, String.class), Arguments.of("I have a \"string1\" and an int 5", 30, null, "int", 5, 31, Integer.class), Arguments.of("'multi\"quota\"str' should works", 0, ' ', "string", "multi\"quota\"str", 17, String.class), Arguments.of("\"multi'quota'str\" should works", 0, ' ', "string", "multi'quota'str", 17, String.class), @@ -43,8 +48,8 @@ static Stream argumentsStream() { } @ParameterizedTest - @MethodSource("argumentsStream") - void testExtractWithRegex(String jStepStr, int j, Character endingChar, String parameterType, Object expectedExtractedValue, int expectedNextIndex, Class expectedClass) { + @MethodSource("argumentsStreamForSuccessScenarios") + void successfulRegexTest(String jStepStr, int j, Character endingChar, String parameterType, Object expectedExtractedValue, int expectedNextIndex, Class expectedClass) { var results = new ArrayList<>(); var nextIndex = jStepParameterExtractor.extractParameterValueAndGetNextIndex(jStepStr, j, endingChar, parameterType, results); assertEquals(expectedClass, results.get(0).getClass()); @@ -52,7 +57,34 @@ void testExtractWithRegex(String jStepStr, int j, Character endingChar, String p assertEquals(expectedNextIndex, nextIndex.orElseThrow()); } + + static Stream argumentsStreamForFailingScenarios() { + return Stream.of( + // input: String jStepStr, int j, Character endingChar, String parameterType, + // expected output: expected partial logging message + Arguments.of("I have a \"string\"", 9, ' ', "int", "Unable to find integer parameter as int from '\"string\"'"), + Arguments.of("I have a \"string\" and an int 5", 9, ' ', "int", "The found int string '5' is not at the beginning of the string '\"string\" and an int 5'") + + ); + } + + @ParameterizedTest + @MethodSource("argumentsStreamForFailingScenarios") + void failingRegexTest(String jStepStr, int j, Character endingChar, String parameterType, String expectedPartialLoggingMessage) { + var stdOut = System.out; + var logCapture = new ByteArrayOutputStream(); + var logCaptureStream = new PrintStream(logCapture, true, StandardCharsets.UTF_8); + System.setOut(logCaptureStream); + var results = new ArrayList<>(); + var nextIndex = jStepParameterExtractor.extractParameterValueAndGetNextIndex(jStepStr, j, endingChar, parameterType, results); + System.setOut(stdOut); + assertTrue(nextIndex.isEmpty()); + assertTrue(logCapture.toString().contains(expectedPartialLoggingMessage)); + } + + @Test + @Disabled("already verified as working") void pocParsing() { assertThrows(NumberFormatException.class, () -> Integer.parseInt("1.1e2")); assertDoesNotThrow(() -> Double.parseDouble("1.1e-2")); diff --git a/src/test/java/scs/comp5903/cucumber/sample/pirates/BugFixedFromIssueBoardTest.java b/src/test/java/scs/comp5903/cucumber/sample/pirates/BugFixedFromIssueBoardTest.java new file mode 100644 index 0000000..161259b --- /dev/null +++ b/src/test/java/scs/comp5903/cucumber/sample/pirates/BugFixedFromIssueBoardTest.java @@ -0,0 +1,28 @@ +package scs.comp5903.cucumber.sample.pirates; + +import org.junit.jupiter.api.Test; +import scs.comp5903.cucumber.EasyCucumber; +import scs.comp5903.cucumber.model.exception.EasyCucumberException; +import scs.comp5903.cucumber.util.ResourceUtil; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * This test class is used to verify the bug fixed from the issue board from GitHub repo + * @author CX无敌 + * @date 2022-10-26 + */ +class BugFixedFromIssueBoardTest { + + @Test + void check1() { + assertDoesNotThrow(() -> EasyCucumber.build(ResourceUtil.getResourcePath("sample/jfeature/pirates/mismatch-bug.jfeature"), MismatchBugStepDef.class).executeAll()); + } + + @Test + void checkIssue12() { + assertDoesNotThrow(() -> EasyCucumber.build(ResourceUtil.getResourcePath("sample/jfeature/pirates/similar-step.jfeature"), SimilarStepBugStepDef.class).executeAll()); + var easyCucumberException = assertThrows(EasyCucumberException.class, () -> EasyCucumber.build(ResourceUtil.getResourcePath("sample/jfeature/pirates/similar-step.jfeature"), SimilarStepBugStepDefWithFailure.class)); + assertTrue(easyCucumberException.getMessage().contains("Step definition not found for: AndStep(stepString='Player3' gets disqualified)")); + } +} diff --git a/src/test/java/scs/comp5903/cucumber/sample/pirates/MismatchBugStepDef.java b/src/test/java/scs/comp5903/cucumber/sample/pirates/MismatchBugStepDef.java index 2f5515b..8ff3f4f 100644 --- a/src/test/java/scs/comp5903/cucumber/sample/pirates/MismatchBugStepDef.java +++ b/src/test/java/scs/comp5903/cucumber/sample/pirates/MismatchBugStepDef.java @@ -1,42 +1,48 @@ package scs.comp5903.cucumber.sample.pirates; +import org.slf4j.Logger; import scs.comp5903.cucumber.model.annotation.JAndStep; import scs.comp5903.cucumber.model.annotation.JGivenStep; import scs.comp5903.cucumber.model.annotation.JThenStep; import scs.comp5903.cucumber.model.annotation.JWhenStep; +import static org.slf4j.LoggerFactory.getLogger; + /** * @author CX无敌 * @date 2022-10-26 */ public class MismatchBugStepDef { + + private static final Logger log = getLogger(MismatchBugStepDef.class); + @JGivenStep("The game starts with {int} player") public void theGameStartsWithPlayer(int numPlayers){ - System.out.println("The game starts with " + numPlayers + " player"); + log.debug("The game starts with " + numPlayers + " player"); } @JAndStep("The player names are the following {string}") public void thePlayerNamesAreTheFollowing(String nameStr){ - System.out.println("The player names are the following " + nameStr); + log.debug("The player names are the following " + nameStr); } @JWhenStep("{string} gets {string} fortune card") public void playerGetsFortuneCard(String playerName, String card) { - System.out.println(playerName + " gets " + card + " fortune card"); + log.debug(playerName + " gets " + card + " fortune card"); } @JAndStep("{string} rolls the following {string}") public void playerRollsTheFollowing(String playerName, String rollStr) { - System.out.println(playerName + " rolls the following " + rollStr); + log.debug(playerName + " rolls the following " + rollStr); } @JAndStep("Player scores are the following {string}") public void playerScoresAreTheFollowing(String expectedStrScores) { - System.out.println("Player scores are the following " + expectedStrScores); + log.debug("Player scores are the following " + expectedStrScores); } @JThenStep("{string} gets disqualified") public void playerGetsDisqualified(String playerName) { - System.out.println(playerName + " gets disqualified"); + log.debug(playerName + " gets disqualified"); } } diff --git a/src/test/java/scs/comp5903/cucumber/sample/pirates/MismatchTest.java b/src/test/java/scs/comp5903/cucumber/sample/pirates/MismatchTest.java deleted file mode 100644 index cd75c14..0000000 --- a/src/test/java/scs/comp5903/cucumber/sample/pirates/MismatchTest.java +++ /dev/null @@ -1,19 +0,0 @@ -package scs.comp5903.cucumber.sample.pirates; - -import org.junit.jupiter.api.Test; -import scs.comp5903.cucumber.EasyCucumber; -import scs.comp5903.cucumber.util.ResourceUtil; - -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; - -/** - * @author CX无敌 - * @date 2022-10-26 - */ -class MismatchTest { - - @Test - void check() { - assertDoesNotThrow(() -> EasyCucumber.build(ResourceUtil.getResourcePath("sample/jfeature/pirates/mismatch-bug.jfeature"), MismatchBugStepDef.class).executeAll()); - } -} diff --git a/src/test/java/scs/comp5903/cucumber/sample/pirates/SimilarStepBugStepDef.java b/src/test/java/scs/comp5903/cucumber/sample/pirates/SimilarStepBugStepDef.java new file mode 100644 index 0000000..e9c9692 --- /dev/null +++ b/src/test/java/scs/comp5903/cucumber/sample/pirates/SimilarStepBugStepDef.java @@ -0,0 +1,41 @@ +package scs.comp5903.cucumber.sample.pirates; + +import org.slf4j.Logger; +import scs.comp5903.cucumber.model.annotation.JAndStep; +import scs.comp5903.cucumber.model.annotation.JGivenStep; +import scs.comp5903.cucumber.model.annotation.JThenStep; +import scs.comp5903.cucumber.model.annotation.JWhenStep; + +import static org.slf4j.LoggerFactory.getLogger; + +/** + * @author CX无敌 + * @date 2022-10-26 + */ +public class SimilarStepBugStepDef { + private static final Logger log = getLogger(SimilarStepBugStepDef.class); + @JGivenStep("Test given") + public void given() { + log.debug("Test given"); + } + + @JWhenStep("Test when") + public void when() { + log.debug("Test when"); + } + + @JThenStep("Test then") + public void then() { + log.debug("Test then"); + } + + @JAndStep("{string} gets {string} fortune card") + public void andTest(String player, String card) { + log.debug(player + " gets " + card + " fortune card"); + } + + @JAndStep("{string} gets disqualified") + public void disqualified(String player) { + log.debug(player + " gets disqualified"); + } +} diff --git a/src/test/java/scs/comp5903/cucumber/sample/pirates/SimilarStepBugStepDefWithFailure.java b/src/test/java/scs/comp5903/cucumber/sample/pirates/SimilarStepBugStepDefWithFailure.java new file mode 100644 index 0000000..250bc2c --- /dev/null +++ b/src/test/java/scs/comp5903/cucumber/sample/pirates/SimilarStepBugStepDefWithFailure.java @@ -0,0 +1,42 @@ +package scs.comp5903.cucumber.sample.pirates; + +import org.slf4j.Logger; +import scs.comp5903.cucumber.model.annotation.JAndStep; +import scs.comp5903.cucumber.model.annotation.JGivenStep; +import scs.comp5903.cucumber.model.annotation.JThenStep; +import scs.comp5903.cucumber.model.annotation.JWhenStep; + +import static org.slf4j.LoggerFactory.getLogger; + +/** + * Similar to {@link SimilarStepBugStepDef}, but with a mismatch in the {@link #disqualified(String)} method. + * @author CX无敌 + * @date 2022-10-26 + */ +public class SimilarStepBugStepDefWithFailure { + private static final Logger log = getLogger(SimilarStepBugStepDefWithFailure.class); + @JGivenStep("Test given") + public void given() { + log.debug("Test given"); + } + + @JWhenStep("Test when") + public void when() { + log.debug("Test when"); + } + + @JThenStep("Test then") + public void then() { + log.debug("Test then"); + } + + @JAndStep("{string} gets {string} fortune card") + public void andTest(String player, String card) { + log.debug(player + " gets " + card + " fortune card"); + } + + @JAndStep("{string} is disqualified") + public void disqualified(String player) { + log.debug(player + " is disqualified"); + } +} diff --git a/src/test/resources/sample/jfeature/pirates/mismatch-bug.jfeature b/src/test/resources/sample/jfeature/pirates/mismatch-bug.jfeature index 266e2fa..9064e27 100644 --- a/src/test/resources/sample/jfeature/pirates/mismatch-bug.jfeature +++ b/src/test/resources/sample/jfeature/pirates/mismatch-bug.jfeature @@ -1,5 +1,5 @@ Feature: Single Player Scoring - This feature tests and validates scoring for a single player + This is a test feature file that cap a bug where matching a regex would not be fixed in index 0 when using .fine(0) @R37 Scenario: Die with three skulls on first roll diff --git a/src/test/resources/sample/jfeature/pirates/similar-step.jfeature b/src/test/resources/sample/jfeature/pirates/similar-step.jfeature new file mode 100644 index 0000000..85468a0 --- /dev/null +++ b/src/test/resources/sample/jfeature/pirates/similar-step.jfeature @@ -0,0 +1,12 @@ +Feature: Test feature + This is the test feature file that try to cap the bug + when a pair of step literal and step definition got the first difference from the parameter + then in the old implementation, a hard error would be thrown + however, it is likely that there is another step definition that can match the step literal, or another way around + + Scenario: Test scenario + Given Test given + When Test when + And 'Player3' gets 'GOLD' fortune card + And 'Player3' gets disqualified + Then Test then \ No newline at end of file