From d9b7ba6515fd3fa6ea9663c9a06b3e34b785e7cf Mon Sep 17 00:00:00 2001 From: tal66 <77445020+tal66@users.noreply.github.com> Date: Tue, 3 May 2022 14:29:56 +0300 Subject: [PATCH 01/13] Warn when feature / glue paths passed as file schemes --- .../io/cucumber/core/feature/FeaturePath.java | 19 ++++++++ .../io/cucumber/core/feature/GluePath.java | 23 ++++++++++ .../core/logging/LogRecordListener.java | 5 +++ .../CucumberOptionsAnnotationParserTest.java | 44 ++++++++++++++++++- .../FeaturePathFeatureSupplierTest.java | 10 ++--- 5 files changed, 93 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/io/cucumber/core/feature/FeaturePath.java b/core/src/main/java/io/cucumber/core/feature/FeaturePath.java index ed2f1ae301..5c305c591b 100644 --- a/core/src/main/java/io/cucumber/core/feature/FeaturePath.java +++ b/core/src/main/java/io/cucumber/core/feature/FeaturePath.java @@ -1,8 +1,13 @@ package io.cucumber.core.feature; +import io.cucumber.core.logging.Logger; +import io.cucumber.core.logging.LoggerFactory; + import java.io.File; import java.net.URI; import java.util.Locale; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import static io.cucumber.core.resource.ClasspathSupport.CLASSPATH_SCHEME_PREFIX; import static io.cucumber.core.resource.ClasspathSupport.RESOURCE_SEPARATOR_CHAR; @@ -27,6 +32,10 @@ */ public class FeaturePath { + private static final Logger log = LoggerFactory.getLogger(FeaturePath.class); + + private static final Pattern FILESYSTEM_PATH_TO_RESOURCES = Pattern.compile("/*src/(?:main|test)/resources/(.*)"); + private FeaturePath() { } @@ -69,6 +78,7 @@ private static String replaceNonStandardPathSeparator(String featureIdentifier) } private static URI parseAssumeFileScheme(String featureIdentifier) { + warnWhenFileSystemPathToResources(featureIdentifier); File featureFile = new File(featureIdentifier); return featureFile.toURI(); } @@ -101,4 +111,13 @@ private static String normalize(final String value) { return value.toLowerCase(Locale.US).replaceAll("[^a-z0-9]+", ""); } + private static void warnWhenFileSystemPathToResources(String featureIdentifier) { + Matcher matcher = FILESYSTEM_PATH_TO_RESOURCES.matcher(featureIdentifier); + if (matcher.matches()) { + log.warn(() -> String.format("Please replace feature path '%s' with the classpath '%s' to avoid ambiguity.", + featureIdentifier, + matcher.replaceAll("classpath:$1"))); + } + } + } diff --git a/core/src/main/java/io/cucumber/core/feature/GluePath.java b/core/src/main/java/io/cucumber/core/feature/GluePath.java index 2585daa213..322c04ce19 100644 --- a/core/src/main/java/io/cucumber/core/feature/GluePath.java +++ b/core/src/main/java/io/cucumber/core/feature/GluePath.java @@ -1,8 +1,13 @@ package io.cucumber.core.feature; +import io.cucumber.core.logging.Logger; +import io.cucumber.core.logging.LoggerFactory; + import java.io.File; import java.net.URI; import java.net.URISyntaxException; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import static io.cucumber.core.resource.ClasspathSupport.CLASSPATH_SCHEME; import static io.cucumber.core.resource.ClasspathSupport.CLASSPATH_SCHEME_PREFIX; @@ -29,6 +34,11 @@ */ public class GluePath { + private static final Logger log = LoggerFactory.getLogger(GluePath.class); + + private static final Pattern FILESYSTEM_PATH_TO_PACKAGES = Pattern + .compile("/*src/(?:main|test)/(?:java|kotlin|scala|groovy)/(.*)"); + private GluePath() { } @@ -70,6 +80,8 @@ private static String replaceNonStandardPathSeparator(String featureIdentifier) private static URI parseAssumeClasspathScheme(String gluePath) { URI uri = URI.create(gluePath); + warnWhenFileSystemPath(gluePath); + String schemeSpecificPart = uri.getSchemeSpecificPart(); if (!isValidIdentifier(schemeSpecificPart)) { throw new IllegalArgumentException("The glue path contained invalid identifiers " + uri); @@ -92,6 +104,17 @@ private static URI parseAssumeClasspathScheme(String gluePath) { return uri; } + private static void warnWhenFileSystemPath(String gluePath) { + Matcher matcher = FILESYSTEM_PATH_TO_PACKAGES.matcher(gluePath); + if (matcher.matches()) { + log.warn(() -> String.format( + "Please replace glue path '%s' with the corresponding package name ('%s') " + + "in order to use already compiled files.", + gluePath, + matcher.replaceAll("$1").replaceAll("/", "."))); + } + } + private static boolean isProbablyPackage(String gluePath) { return gluePath.contains(PACKAGE_SEPARATOR_STRING) && !gluePath.contains(RESOURCE_SEPARATOR_STRING); diff --git a/core/src/main/java/io/cucumber/core/logging/LogRecordListener.java b/core/src/main/java/io/cucumber/core/logging/LogRecordListener.java index d8c4b227ee..48d6ebf6ce 100644 --- a/core/src/main/java/io/cucumber/core/logging/LogRecordListener.java +++ b/core/src/main/java/io/cucumber/core/logging/LogRecordListener.java @@ -17,4 +17,9 @@ public List getLogRecords() { return Arrays.asList(logRecords.toArray(new LogRecord[0])); } + public static boolean anyRecordMessageMatch(LogRecordListener listener, String regex) { + return listener.getLogRecords().stream() + .anyMatch(r -> r.getMessage().matches(regex)); + } + } diff --git a/core/src/test/java/io/cucumber/core/options/CucumberOptionsAnnotationParserTest.java b/core/src/test/java/io/cucumber/core/options/CucumberOptionsAnnotationParserTest.java index 189d3b278f..40c3d828bf 100644 --- a/core/src/test/java/io/cucumber/core/options/CucumberOptionsAnnotationParserTest.java +++ b/core/src/test/java/io/cucumber/core/options/CucumberOptionsAnnotationParserTest.java @@ -2,7 +2,10 @@ import io.cucumber.core.backend.ObjectFactory; import io.cucumber.core.exception.CucumberException; -import io.cucumber.core.plugin.DefaultSummaryPrinter; +import io.cucumber.core.feature.FeaturePath; +import io.cucumber.core.feature.GluePath; +import io.cucumber.core.logging.LogRecordListener; +import io.cucumber.core.logging.LoggerFactory; import io.cucumber.core.plugin.HtmlFormatter; import io.cucumber.core.plugin.NoPublishFormatter; import io.cucumber.core.plugin.PluginFactory; @@ -13,7 +16,6 @@ import io.cucumber.core.snippets.SnippetType; import io.cucumber.plugin.Plugin; import io.cucumber.tagexpressions.TagExpressionException; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; @@ -218,6 +220,44 @@ void create_with_glue() { contains(uri("classpath:/app/features/user/registration"), uri("classpath:/app/features/hooks"))); } + @Test + void warn_when_features_as_filesystem_path_to_resources() { + // warn when 'src/{test,main}/resources' is used + + LogRecordListener logRecordListener = new LogRecordListener(); + LoggerFactory.addListener(logRecordListener); + + FeaturePath.parse("src/test/resources/features"); + FeaturePath.parse("src/main/resources/features/feature1"); + + LoggerFactory.removeListener(logRecordListener); + + assertAll( + () -> assertTrue(LogRecordListener.anyRecordMessageMatch(logRecordListener, + ".*replace.*src/test/resources/features.*'classpath:features'.*")), + () -> assertTrue(LogRecordListener.anyRecordMessageMatch(logRecordListener, + ".*replace.*src/main/resources/features/feature1.*'classpath:features/feature1'.*"))); + } + + @Test + void warn_when_glue_as_filesystem_path() { + // warn when 'src/{test,main}/{java,kotlin,scala,groovy}' is used + + LogRecordListener logRecordListener = new LogRecordListener(); + LoggerFactory.addListener(logRecordListener); + + GluePath.parse("src/main/java/com/example"); + GluePath.parse("src/test/java/com/package/other_package"); + + LoggerFactory.removeListener(logRecordListener); + + assertAll( + () -> assertTrue(LogRecordListener.anyRecordMessageMatch(logRecordListener, + ".*replace.*src/main/java/com/example.*'com.example'.*")), + () -> assertTrue(LogRecordListener.anyRecordMessageMatch(logRecordListener, + ".*replace.*src/test/java/com/package/other_package.*'com.package.other_package'.*"))); + } + @Test void create_with_extra_glue() { RuntimeOptions runtimeOptions = parser().parse(ClassWithExtraGlue.class).build(); diff --git a/core/src/test/java/io/cucumber/core/runtime/FeaturePathFeatureSupplierTest.java b/core/src/test/java/io/cucumber/core/runtime/FeaturePathFeatureSupplierTest.java index e2b59a983b..129e213e2c 100644 --- a/core/src/test/java/io/cucumber/core/runtime/FeaturePathFeatureSupplierTest.java +++ b/core/src/test/java/io/cucumber/core/runtime/FeaturePathFeatureSupplierTest.java @@ -18,7 +18,6 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; class FeaturePathFeatureSupplierTest { @@ -44,11 +43,10 @@ void logs_message_if_no_features_are_found() { FeaturePathFeatureSupplier supplier = new FeaturePathFeatureSupplier(classLoader, featureOptions, parser); supplier.get(); - assertAll( - () -> assertThat(logRecordListener.getLogRecords().get(1).getMessage(), - containsString("No features found at file:")), - () -> assertThat(logRecordListener.getLogRecords().get(1).getMessage(), - containsString("src/test/resources/io/cucumber/core/options"))); + + assertThat("log contains pattern", + LogRecordListener.anyRecordMessageMatch(logRecordListener, + ".*No features found at file.*src/test/resources/io/cucumber/core/options/?")); } @Test From 2293afbcc28fc0429184ced434d9e16b0f6d513a Mon Sep 17 00:00:00 2001 From: tal66 <77445020+tal66@users.noreply.github.com> Date: Wed, 4 May 2022 00:47:18 +0300 Subject: [PATCH 02/13] Warn when feature / glue paths passed as file schemes - 2 --- .../io/cucumber/core/feature/FeaturePath.java | 2 +- .../io/cucumber/core/feature/GluePath.java | 17 +++++--- .../core/logging/LogRecordListener.java | 5 --- .../core/feature/FeaturePathTest.java | 31 ++++++++++++++ .../cucumber/core/feature/GluePathTest.java | 32 ++++++++++++++ .../CucumberOptionsAnnotationParserTest.java | 42 ------------------- .../FeaturePathFeatureSupplierTest.java | 14 +++---- 7 files changed, 82 insertions(+), 61 deletions(-) diff --git a/core/src/main/java/io/cucumber/core/feature/FeaturePath.java b/core/src/main/java/io/cucumber/core/feature/FeaturePath.java index 5c305c591b..420ea99e32 100644 --- a/core/src/main/java/io/cucumber/core/feature/FeaturePath.java +++ b/core/src/main/java/io/cucumber/core/feature/FeaturePath.java @@ -34,7 +34,7 @@ public class FeaturePath { private static final Logger log = LoggerFactory.getLogger(FeaturePath.class); - private static final Pattern FILESYSTEM_PATH_TO_RESOURCES = Pattern.compile("/*src/(?:main|test)/resources/(.*)"); + private static final Pattern FILESYSTEM_PATH_TO_RESOURCES = Pattern.compile("src/(?:main|test)/resources/?(.*)"); private FeaturePath() { diff --git a/core/src/main/java/io/cucumber/core/feature/GluePath.java b/core/src/main/java/io/cucumber/core/feature/GluePath.java index 322c04ce19..72f6249884 100644 --- a/core/src/main/java/io/cucumber/core/feature/GluePath.java +++ b/core/src/main/java/io/cucumber/core/feature/GluePath.java @@ -37,7 +37,7 @@ public class GluePath { private static final Logger log = LoggerFactory.getLogger(GluePath.class); private static final Pattern FILESYSTEM_PATH_TO_PACKAGES = Pattern - .compile("/*src/(?:main|test)/(?:java|kotlin|scala|groovy)/(.*)"); + .compile("src/(?:main|test)/(?:java|kotlin|scala|groovy)/?(.*)"); private GluePath() { @@ -107,11 +107,16 @@ private static URI parseAssumeClasspathScheme(String gluePath) { private static void warnWhenFileSystemPath(String gluePath) { Matcher matcher = FILESYSTEM_PATH_TO_PACKAGES.matcher(gluePath); if (matcher.matches()) { - log.warn(() -> String.format( - "Please replace glue path '%s' with the corresponding package name ('%s') " + - "in order to use already compiled files.", - gluePath, - matcher.replaceAll("$1").replaceAll("/", "."))); + String _package = matcher.replaceAll("$1").replaceAll("/", "."); + if (_package.isEmpty()) { + log.warn(() -> String.format( + "%s is not a package. Please insert a package as glue path (e.g come.example)", gluePath)); + } else { + log.warn( + () -> String.format("Please replace glue path '%s' with the corresponding package name ('%s') " + + "in order to use already compiled files.", + gluePath, _package)); + } } } diff --git a/core/src/main/java/io/cucumber/core/logging/LogRecordListener.java b/core/src/main/java/io/cucumber/core/logging/LogRecordListener.java index 48d6ebf6ce..d8c4b227ee 100644 --- a/core/src/main/java/io/cucumber/core/logging/LogRecordListener.java +++ b/core/src/main/java/io/cucumber/core/logging/LogRecordListener.java @@ -17,9 +17,4 @@ public List getLogRecords() { return Arrays.asList(logRecords.toArray(new LogRecord[0])); } - public static boolean anyRecordMessageMatch(LogRecordListener listener, String regex) { - return listener.getLogRecords().stream() - .anyMatch(r -> r.getMessage().matches(regex)); - } - } diff --git a/core/src/test/java/io/cucumber/core/feature/FeaturePathTest.java b/core/src/test/java/io/cucumber/core/feature/FeaturePathTest.java index 64ea3375ee..6bce91fec8 100644 --- a/core/src/test/java/io/cucumber/core/feature/FeaturePathTest.java +++ b/core/src/test/java/io/cucumber/core/feature/FeaturePathTest.java @@ -1,18 +1,26 @@ package io.cucumber.core.feature; +import io.cucumber.core.logging.LogRecordListener; +import io.cucumber.core.logging.LoggerFactory; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledOnOs; import org.junit.jupiter.api.condition.EnabledOnOs; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import java.io.File; import java.net.URI; +import java.util.stream.Stream; import static org.hamcrest.CoreMatchers.endsWith; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.Is.is; +import static org.hamcrest.text.MatchesPattern.matchesPattern; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.condition.OS.WINDOWS; +import static org.junit.jupiter.params.provider.Arguments.arguments; class FeaturePathTest { @@ -146,4 +154,27 @@ void can_parse_windows_file_path_with_standard_file_separator() { () -> assertThat(uri.getSchemeSpecificPart(), is("/C:/path/to/file.feature"))); } + @ParameterizedTest + @MethodSource("FeaturePathAndPatternProvider") + void warn_when_features_as_filesystem_path_to_resources(String featurePath, String logPattern) { + // warn when 'src/{test,main}/resources' is used + + LogRecordListener logRecordListener = new LogRecordListener(); + LoggerFactory.addListener(logRecordListener); + + FeaturePath.parse(featurePath); + + LoggerFactory.removeListener(logRecordListener); + + assertThat(logRecordListener.getLogRecords().get(0).getMessage(), + matchesPattern(logPattern)); + } + + static Stream FeaturePathAndPatternProvider() { + return Stream.of( + arguments("src/test/resources/", ".*replace.*src/test/resources/.*'classpath:'.*"), + arguments("src/test/resources/features", ".*replace.*src/test/resources/features.*'classpath:features'.*"), + arguments("src/main/resources/features/feature1", + ".*replace.*src/main/resources/features/feature1.*'classpath:features/feature1'.*")); + } } diff --git a/core/src/test/java/io/cucumber/core/feature/GluePathTest.java b/core/src/test/java/io/cucumber/core/feature/GluePathTest.java index 1134446f1b..ee72b23c4a 100644 --- a/core/src/test/java/io/cucumber/core/feature/GluePathTest.java +++ b/core/src/test/java/io/cucumber/core/feature/GluePathTest.java @@ -1,17 +1,25 @@ package io.cucumber.core.feature; +import io.cucumber.core.logging.LogRecordListener; +import io.cucumber.core.logging.LoggerFactory; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledOnOs; import org.junit.jupiter.api.condition.OS; import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import java.net.URI; +import java.util.stream.Stream; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.matchesPattern; import static org.hamcrest.core.Is.is; import static org.hamcrest.core.IsEqual.equalTo; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.params.provider.Arguments.arguments; class GluePathTest { @@ -114,4 +122,28 @@ void absolute_windows_path_form_is_not_valid() { "The glue path must have a classpath scheme C:/com/example/app"))); } + @ParameterizedTest + @MethodSource("GluePathAndPatternProvider") + void warn_when_glue_as_filesystem_path(String gluePath, String logPattern) { + // warn when 'src/{test,main}/{java,kotlin,scala,groovy}' is used + + LogRecordListener logRecordListener = new LogRecordListener(); + LoggerFactory.addListener(logRecordListener); + + GluePath.parse(gluePath); + + LoggerFactory.removeListener(logRecordListener); + + assertThat(logRecordListener.getLogRecords().get(0).getMessage(), + matchesPattern(logPattern)); + } + + static Stream GluePathAndPatternProvider() { + return Stream.of( + arguments("src/main/java", ".*not a package.*"), + arguments("src/main/java/com/example", ".*replace.*src/main/java/com/example.*'com.example'.*"), + arguments("src/test/java/com/package/other_package", + ".*replace.*src/test/java/com/package/other_package.*'com.package.other_package'.*")); + } + } diff --git a/core/src/test/java/io/cucumber/core/options/CucumberOptionsAnnotationParserTest.java b/core/src/test/java/io/cucumber/core/options/CucumberOptionsAnnotationParserTest.java index 40c3d828bf..43fb536e0b 100644 --- a/core/src/test/java/io/cucumber/core/options/CucumberOptionsAnnotationParserTest.java +++ b/core/src/test/java/io/cucumber/core/options/CucumberOptionsAnnotationParserTest.java @@ -2,10 +2,6 @@ import io.cucumber.core.backend.ObjectFactory; import io.cucumber.core.exception.CucumberException; -import io.cucumber.core.feature.FeaturePath; -import io.cucumber.core.feature.GluePath; -import io.cucumber.core.logging.LogRecordListener; -import io.cucumber.core.logging.LoggerFactory; import io.cucumber.core.plugin.HtmlFormatter; import io.cucumber.core.plugin.NoPublishFormatter; import io.cucumber.core.plugin.PluginFactory; @@ -220,44 +216,6 @@ void create_with_glue() { contains(uri("classpath:/app/features/user/registration"), uri("classpath:/app/features/hooks"))); } - @Test - void warn_when_features_as_filesystem_path_to_resources() { - // warn when 'src/{test,main}/resources' is used - - LogRecordListener logRecordListener = new LogRecordListener(); - LoggerFactory.addListener(logRecordListener); - - FeaturePath.parse("src/test/resources/features"); - FeaturePath.parse("src/main/resources/features/feature1"); - - LoggerFactory.removeListener(logRecordListener); - - assertAll( - () -> assertTrue(LogRecordListener.anyRecordMessageMatch(logRecordListener, - ".*replace.*src/test/resources/features.*'classpath:features'.*")), - () -> assertTrue(LogRecordListener.anyRecordMessageMatch(logRecordListener, - ".*replace.*src/main/resources/features/feature1.*'classpath:features/feature1'.*"))); - } - - @Test - void warn_when_glue_as_filesystem_path() { - // warn when 'src/{test,main}/{java,kotlin,scala,groovy}' is used - - LogRecordListener logRecordListener = new LogRecordListener(); - LoggerFactory.addListener(logRecordListener); - - GluePath.parse("src/main/java/com/example"); - GluePath.parse("src/test/java/com/package/other_package"); - - LoggerFactory.removeListener(logRecordListener); - - assertAll( - () -> assertTrue(LogRecordListener.anyRecordMessageMatch(logRecordListener, - ".*replace.*src/main/java/com/example.*'com.example'.*")), - () -> assertTrue(LogRecordListener.anyRecordMessageMatch(logRecordListener, - ".*replace.*src/test/java/com/package/other_package.*'com.package.other_package'.*"))); - } - @Test void create_with_extra_glue() { RuntimeOptions runtimeOptions = parser().parse(ClassWithExtraGlue.class).build(); diff --git a/core/src/test/java/io/cucumber/core/runtime/FeaturePathFeatureSupplierTest.java b/core/src/test/java/io/cucumber/core/runtime/FeaturePathFeatureSupplierTest.java index 129e213e2c..75974df595 100644 --- a/core/src/test/java/io/cucumber/core/runtime/FeaturePathFeatureSupplierTest.java +++ b/core/src/test/java/io/cucumber/core/runtime/FeaturePathFeatureSupplierTest.java @@ -14,10 +14,11 @@ import java.util.function.Supplier; import static java.util.Collections.singletonList; -import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.matchesPattern; +import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertThrows; class FeaturePathFeatureSupplierTest { @@ -39,14 +40,13 @@ void tearDown() { @Test void logs_message_if_no_features_are_found() { - Options featureOptions = () -> singletonList(FeaturePath.parse("src/test/resources/io/cucumber/core/options")); + Options featureOptions = () -> singletonList(FeaturePath.parse("classpath:io/cucumber/core/options")); FeaturePathFeatureSupplier supplier = new FeaturePathFeatureSupplier(classLoader, featureOptions, parser); supplier.get(); - assertThat("log contains pattern", - LogRecordListener.anyRecordMessageMatch(logRecordListener, - ".*No features found at file.*src/test/resources/io/cucumber/core/options/?")); + assertThat(logRecordListener.getLogRecords().get(1).getMessage(), + matchesPattern(".*No features found at.*io/cucumber/core/options/?")); } @Test From 2fd153a9dac9f856c3deb8256536889a464e5a72 Mon Sep 17 00:00:00 2001 From: tal66 <77445020+tal66@users.noreply.github.com> Date: Fri, 6 May 2022 17:06:56 +0300 Subject: [PATCH 03/13] Warn when feature / glue paths passed as file schemes - 3 --- .../io/cucumber/core/feature/FeaturePath.java | 12 ++++++++---- .../java/io/cucumber/core/feature/GluePath.java | 8 ++++---- .../cucumber/core/feature/FeaturePathTest.java | 16 +++++++++++----- .../io/cucumber/core/feature/GluePathTest.java | 13 ++++++++++--- 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/io/cucumber/core/feature/FeaturePath.java b/core/src/main/java/io/cucumber/core/feature/FeaturePath.java index 420ea99e32..16d17ef16a 100644 --- a/core/src/main/java/io/cucumber/core/feature/FeaturePath.java +++ b/core/src/main/java/io/cucumber/core/feature/FeaturePath.java @@ -34,7 +34,7 @@ public class FeaturePath { private static final Logger log = LoggerFactory.getLogger(FeaturePath.class); - private static final Pattern FILESYSTEM_PATH_TO_RESOURCES = Pattern.compile("src/(?:main|test)/resources/?(.*)"); + private static final Pattern FILESYSTEM_PATH_TO_RESOURCES = Pattern.compile("src/(?:main|test)/resources(/?)(.*)"); private FeaturePath() { @@ -114,9 +114,13 @@ private static String normalize(final String value) { private static void warnWhenFileSystemPathToResources(String featureIdentifier) { Matcher matcher = FILESYSTEM_PATH_TO_RESOURCES.matcher(featureIdentifier); if (matcher.matches()) { - log.warn(() -> String.format("Please replace feature path '%s' with the classpath '%s' to avoid ambiguity.", - featureIdentifier, - matcher.replaceAll("classpath:$1"))); + if (!matcher.group(1).equals("/") && matcher.group(2).length() > 0) { + return; + } + log.warn( + () -> String.format("Consider replacing feature path '%s' with '%s'.", + featureIdentifier, + matcher.replaceAll("classpath:$2"))); } } diff --git a/core/src/main/java/io/cucumber/core/feature/GluePath.java b/core/src/main/java/io/cucumber/core/feature/GluePath.java index 72f6249884..da52f86e05 100644 --- a/core/src/main/java/io/cucumber/core/feature/GluePath.java +++ b/core/src/main/java/io/cucumber/core/feature/GluePath.java @@ -37,7 +37,7 @@ public class GluePath { private static final Logger log = LoggerFactory.getLogger(GluePath.class); private static final Pattern FILESYSTEM_PATH_TO_PACKAGES = Pattern - .compile("src/(?:main|test)/(?:java|kotlin|scala|groovy)/?(.*)"); + .compile("src/(?:main|test)/(?:java|kotlin|scala|groovy)(/?)(.*)"); private GluePath() { @@ -107,11 +107,11 @@ private static URI parseAssumeClasspathScheme(String gluePath) { private static void warnWhenFileSystemPath(String gluePath) { Matcher matcher = FILESYSTEM_PATH_TO_PACKAGES.matcher(gluePath); if (matcher.matches()) { - String _package = matcher.replaceAll("$1").replaceAll("/", "."); - if (_package.isEmpty()) { + String _package = matcher.group(2).replaceAll("/", "."); + if (_package.isEmpty() || _package.equals(".")) { log.warn(() -> String.format( "%s is not a package. Please insert a package as glue path (e.g come.example)", gluePath)); - } else { + } else if (matcher.group(1).equals("/")) { log.warn( () -> String.format("Please replace glue path '%s' with the corresponding package name ('%s') " + "in order to use already compiled files.", diff --git a/core/src/test/java/io/cucumber/core/feature/FeaturePathTest.java b/core/src/test/java/io/cucumber/core/feature/FeaturePathTest.java index 6bce91fec8..3628a78b8f 100644 --- a/core/src/test/java/io/cucumber/core/feature/FeaturePathTest.java +++ b/core/src/test/java/io/cucumber/core/feature/FeaturePathTest.java @@ -166,15 +166,21 @@ void warn_when_features_as_filesystem_path_to_resources(String featurePath, Stri LoggerFactory.removeListener(logRecordListener); - assertThat(logRecordListener.getLogRecords().get(0).getMessage(), - matchesPattern(logPattern)); + String logMessage = ""; + if (!logRecordListener.getLogRecords().isEmpty()) { + logMessage = logRecordListener.getLogRecords().get(0).getMessage(); + } + + assertThat(logMessage, matchesPattern(logPattern)); } static Stream FeaturePathAndPatternProvider() { return Stream.of( - arguments("src/test/resources/", ".*replace.*src/test/resources/.*'classpath:'.*"), - arguments("src/test/resources/features", ".*replace.*src/test/resources/features.*'classpath:features'.*"), + arguments("src/test/resources_abcd", ""), + arguments("src/test/resources/", ".*replacing.*src/test/resources/.*'classpath:'.*"), + arguments("src/test/resources/features", + ".*replacing.*src/test/resources/features.*'classpath:features'.*"), arguments("src/main/resources/features/feature1", - ".*replace.*src/main/resources/features/feature1.*'classpath:features/feature1'.*")); + ".*replacing.*src/main/resources/features/feature1.*'classpath:features/feature1'.*")); } } diff --git a/core/src/test/java/io/cucumber/core/feature/GluePathTest.java b/core/src/test/java/io/cucumber/core/feature/GluePathTest.java index ee72b23c4a..7c7836b28f 100644 --- a/core/src/test/java/io/cucumber/core/feature/GluePathTest.java +++ b/core/src/test/java/io/cucumber/core/feature/GluePathTest.java @@ -134,14 +134,21 @@ void warn_when_glue_as_filesystem_path(String gluePath, String logPattern) { LoggerFactory.removeListener(logRecordListener); - assertThat(logRecordListener.getLogRecords().get(0).getMessage(), - matchesPattern(logPattern)); + String logMessage = ""; + if (!logRecordListener.getLogRecords().isEmpty()) { + logMessage = logRecordListener.getLogRecords().get(0).getMessage(); + } + + assertThat(logMessage, matchesPattern(logPattern)); } static Stream GluePathAndPatternProvider() { return Stream.of( arguments("src/main/java", ".*not a package.*"), - arguments("src/main/java/com/example", ".*replace.*src/main/java/com/example.*'com.example'.*"), + arguments("src/main/scala_other", ""), + arguments("src/main/javaaaaa", ""), + arguments("src/main/abcd", ""), + arguments("src/main/groovy/com/example", ".*replace.*src/main/groovy/com/example.*'com.example'.*"), arguments("src/test/java/com/package/other_package", ".*replace.*src/test/java/com/package/other_package.*'com.package.other_package'.*")); } From 269e79af363df53c6ee8c7f2695df36bee473067 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Sat, 7 May 2022 19:06:43 +0200 Subject: [PATCH 04/13] Use early-out / guard clause pattern By returning immediately when a pre-condition doesn't hold we can reduce the nesting and make the code more readable. --- .../io/cucumber/core/feature/GluePath.java | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/io/cucumber/core/feature/GluePath.java b/core/src/main/java/io/cucumber/core/feature/GluePath.java index da52f86e05..fca101276f 100644 --- a/core/src/main/java/io/cucumber/core/feature/GluePath.java +++ b/core/src/main/java/io/cucumber/core/feature/GluePath.java @@ -106,17 +106,18 @@ private static URI parseAssumeClasspathScheme(String gluePath) { private static void warnWhenFileSystemPath(String gluePath) { Matcher matcher = FILESYSTEM_PATH_TO_PACKAGES.matcher(gluePath); - if (matcher.matches()) { - String _package = matcher.group(2).replaceAll("/", "."); - if (_package.isEmpty() || _package.equals(".")) { - log.warn(() -> String.format( - "%s is not a package. Please insert a package as glue path (e.g come.example)", gluePath)); - } else if (matcher.group(1).equals("/")) { - log.warn( - () -> String.format("Please replace glue path '%s' with the corresponding package name ('%s') " + - "in order to use already compiled files.", - gluePath, _package)); - } + if (!matcher.matches()) { + return; + } + String _package = matcher.group(2).replaceAll("/", "."); + if (_package.isEmpty() || _package.equals(".")) { + log.warn(() -> String.format( + "%s is not a package. Please insert a package as glue path (e.g come.example)", gluePath)); + } else if (matcher.group(1).equals("/")) { + log.warn( + () -> String.format("Please replace glue path '%s' with the corresponding package name ('%s') " + + "in order to use already compiled files.", + gluePath, _package)); } } From eeeeab5458d7e24e5a73e422dce5e66e43127d8b Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Sat, 7 May 2022 19:46:28 +0200 Subject: [PATCH 05/13] Tweak the regex to make turning the class path resource into package name easier --- .../io/cucumber/core/feature/GluePath.java | 28 ++++--- .../cucumber/core/feature/GluePathTest.java | 78 +++++++++++-------- 2 files changed, 64 insertions(+), 42 deletions(-) diff --git a/core/src/main/java/io/cucumber/core/feature/GluePath.java b/core/src/main/java/io/cucumber/core/feature/GluePath.java index fca101276f..517c68f8fe 100644 --- a/core/src/main/java/io/cucumber/core/feature/GluePath.java +++ b/core/src/main/java/io/cucumber/core/feature/GluePath.java @@ -37,7 +37,7 @@ public class GluePath { private static final Logger log = LoggerFactory.getLogger(GluePath.class); private static final Pattern FILESYSTEM_PATH_TO_PACKAGES = Pattern - .compile("src/(?:main|test)/(?:java|kotlin|scala|groovy)(/?)(.*)"); + .compile("src/(?:main|test)/(?:java|kotlin|scala|groovy)(|/|/.+)"); private GluePath() { @@ -109,16 +109,24 @@ private static void warnWhenFileSystemPath(String gluePath) { if (!matcher.matches()) { return; } - String _package = matcher.group(2).replaceAll("/", "."); - if (_package.isEmpty() || _package.equals(".")) { - log.warn(() -> String.format( - "%s is not a package. Please insert a package as glue path (e.g come.example)", gluePath)); - } else if (matcher.group(1).equals("/")) { - log.warn( - () -> String.format("Please replace glue path '%s' with the corresponding package name ('%s') " + - "in order to use already compiled files.", - gluePath, _package)); + String classPathResource = matcher.group(1); + if (classPathResource.startsWith("/")) { + classPathResource = classPathResource.substring(1); } + if (classPathResource.endsWith("/")) { + classPathResource = classPathResource.substring(0, classPathResource.length() - 1); + } + + String packageName = classPathResource.replaceAll("/","."); + log.warn(() -> { + String message = "" + + "Cucumber was given the glue path '%s'. This path points to a source directory\n" + + "in your project. However cucumber looks for glue (i.e. step definitions) on the\n" + + "classpath.\n" + + "\n" + + "To avoid confusion consider using a package name instead for example: '%s'\n"; + return String.format(message, gluePath, packageName); + }); } private static boolean isProbablyPackage(String gluePath) { diff --git a/core/src/test/java/io/cucumber/core/feature/GluePathTest.java b/core/src/test/java/io/cucumber/core/feature/GluePathTest.java index 7c7836b28f..09912bdafa 100644 --- a/core/src/test/java/io/cucumber/core/feature/GluePathTest.java +++ b/core/src/test/java/io/cucumber/core/feature/GluePathTest.java @@ -2,6 +2,9 @@ import io.cucumber.core.logging.LogRecordListener; import io.cucumber.core.logging.LoggerFactory; +import org.hamcrest.CoreMatchers; +import org.hamcrest.Matcher; +import org.hamcrest.text.IsEmptyString; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledOnOs; import org.junit.jupiter.api.condition.OS; @@ -11,12 +14,17 @@ import org.junit.jupiter.params.provider.MethodSource; import java.net.URI; +import java.util.logging.LogRecord; import java.util.stream.Stream; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.isEmptyString; import static org.hamcrest.Matchers.matchesPattern; import static org.hamcrest.core.Is.is; import static org.hamcrest.core.IsEqual.equalTo; +import static org.hamcrest.text.IsEmptyString.emptyOrNullString; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.params.provider.Arguments.arguments; @@ -28,8 +36,8 @@ void can_parse_empty_glue_path() { URI uri = GluePath.parse(""); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is("/"))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is("/"))); } @Test @@ -37,8 +45,8 @@ void can_parse_root_package() { URI uri = GluePath.parse("classpath:/"); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is("/"))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is("/"))); } @Test @@ -47,8 +55,8 @@ void can_parse_eclipse_plugin_default_glue() { URI uri = GluePath.parse("classpath:"); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is("/"))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is("/"))); } @Test @@ -56,8 +64,8 @@ void can_parse_classpath_form() { URI uri = GluePath.parse("classpath:com/example/app"); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is("com/example/app"))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is("com/example/app"))); } @Test @@ -65,8 +73,8 @@ void can_parse_relative_path_form() { URI uri = GluePath.parse("com/example/app"); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is("/com/example/app"))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is("/com/example/app"))); } @Test @@ -74,8 +82,8 @@ void can_parse_absolute_path_form() { URI uri = GluePath.parse("/com/example/app"); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is("/com/example/app"))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is("/com/example/app"))); } @Test @@ -83,8 +91,8 @@ void can_parse_package_form() { URI uri = GluePath.parse("com.example.app"); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is("/com/example/app"))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is("/com/example/app"))); } @Test @@ -92,7 +100,7 @@ void glue_path_must_have_class_path_scheme() { Executable testMethod = () -> GluePath.parse("file:com/example/app"); IllegalArgumentException actualThrown = assertThrows(IllegalArgumentException.class, testMethod); assertThat("Unexpected exception message", actualThrown.getMessage(), is(equalTo( - "The glue path must have a classpath scheme file:com/example/app"))); + "The glue path must have a classpath scheme file:com/example/app"))); } @Test @@ -100,7 +108,7 @@ void glue_path_must_have_valid_identifier_parts() { Executable testMethod = () -> GluePath.parse("01-examples"); IllegalArgumentException actualThrown = assertThrows(IllegalArgumentException.class, testMethod); assertThat("Unexpected exception message", actualThrown.getMessage(), is(equalTo( - "The glue path contained invalid identifiers 01-examples"))); + "The glue path contained invalid identifiers 01-examples"))); } @Test @@ -109,8 +117,8 @@ void can_parse_windows_path_form() { URI uri = GluePath.parse("com\\example\\app"); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is(equalTo("/com/example/app")))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is(equalTo("/com/example/app")))); } @Test @@ -119,12 +127,12 @@ void absolute_windows_path_form_is_not_valid() { Executable testMethod = () -> GluePath.parse("C:\\com\\example\\app"); IllegalArgumentException actualThrown = assertThrows(IllegalArgumentException.class, testMethod); assertThat("Unexpected exception message", actualThrown.getMessage(), is(equalTo( - "The glue path must have a classpath scheme C:/com/example/app"))); + "The glue path must have a classpath scheme C:/com/example/app"))); } @ParameterizedTest @MethodSource("GluePathAndPatternProvider") - void warn_when_glue_as_filesystem_path(String gluePath, String logPattern) { + void warn_when_glue_as_filesystem_path(String gluePath, Matcher logPattern) { // warn when 'src/{test,main}/{java,kotlin,scala,groovy}' is used LogRecordListener logRecordListener = new LogRecordListener(); @@ -134,23 +142,29 @@ void warn_when_glue_as_filesystem_path(String gluePath, String logPattern) { LoggerFactory.removeListener(logRecordListener); - String logMessage = ""; - if (!logRecordListener.getLogRecords().isEmpty()) { - logMessage = logRecordListener.getLogRecords().get(0).getMessage(); - } + String logMessage = logRecordListener.getLogRecords() + .stream() + .findFirst() + .map(LogRecord::getMessage) + .orElse(null); - assertThat(logMessage, matchesPattern(logPattern)); + assertThat(logMessage, logPattern); } static Stream GluePathAndPatternProvider() { return Stream.of( - arguments("src/main/java", ".*not a package.*"), - arguments("src/main/scala_other", ""), - arguments("src/main/javaaaaa", ""), - arguments("src/main/abcd", ""), - arguments("src/main/groovy/com/example", ".*replace.*src/main/groovy/com/example.*'com.example'.*"), - arguments("src/test/java/com/package/other_package", - ".*replace.*src/test/java/com/package/other_package.*'com.package.other_package'.*")); + arguments("src/main/java", containsString("for example: ''")), + arguments("src/main/java/", containsString("for example: ''")), + arguments("src/main/java_other", nullValue()), + arguments("src/main/other", nullValue()), + arguments("src/main/java/com", containsString("for example: 'com'")), + arguments("src/main/java/com/", containsString("for example: 'com'")), + arguments("src/main/groovy/com", containsString("for example: 'com'")), + arguments("src/main/java/com/example", containsString("for example: 'com.example'")), + arguments("src/main/java/com/example/", containsString("for example: 'com.example'")), + arguments("src/main/java/com/example/package", containsString("for example: 'com.example.package'")) + + ); } } From 9634e68f86fbcbb209892394e70b5e1fca342022 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Sat, 7 May 2022 19:47:09 +0200 Subject: [PATCH 06/13] Do string mangling inside lambda for efficiency If nothing is logged, lambda is not invoked. --- .../java/io/cucumber/core/feature/GluePath.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/io/cucumber/core/feature/GluePath.java b/core/src/main/java/io/cucumber/core/feature/GluePath.java index 517c68f8fe..3571363b9f 100644 --- a/core/src/main/java/io/cucumber/core/feature/GluePath.java +++ b/core/src/main/java/io/cucumber/core/feature/GluePath.java @@ -109,16 +109,15 @@ private static void warnWhenFileSystemPath(String gluePath) { if (!matcher.matches()) { return; } - String classPathResource = matcher.group(1); - if (classPathResource.startsWith("/")) { - classPathResource = classPathResource.substring(1); - } - if (classPathResource.endsWith("/")) { - classPathResource = classPathResource.substring(0, classPathResource.length() - 1); - } - - String packageName = classPathResource.replaceAll("/","."); log.warn(() -> { + String classPathResource = matcher.group(1); + if (classPathResource.startsWith("/")) { + classPathResource = classPathResource.substring(1); + } + if (classPathResource.endsWith("/")) { + classPathResource = classPathResource.substring(0, classPathResource.length() - 1); + } + String packageName = classPathResource.replaceAll("/","."); String message = "" + "Cucumber was given the glue path '%s'. This path points to a source directory\n" + "in your project. However cucumber looks for glue (i.e. step definitions) on the\n" + From 0d800dda199ebecbe934233be581a8546ff062b9 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Sat, 7 May 2022 19:57:30 +0200 Subject: [PATCH 07/13] Fix spotless --- .../io/cucumber/core/feature/GluePath.java | 2 +- .../cucumber/core/feature/GluePathTest.java | 58 +++++++++---------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/core/src/main/java/io/cucumber/core/feature/GluePath.java b/core/src/main/java/io/cucumber/core/feature/GluePath.java index 3571363b9f..cd336566f3 100644 --- a/core/src/main/java/io/cucumber/core/feature/GluePath.java +++ b/core/src/main/java/io/cucumber/core/feature/GluePath.java @@ -117,7 +117,7 @@ private static void warnWhenFileSystemPath(String gluePath) { if (classPathResource.endsWith("/")) { classPathResource = classPathResource.substring(0, classPathResource.length() - 1); } - String packageName = classPathResource.replaceAll("/","."); + String packageName = classPathResource.replaceAll("/", "."); String message = "" + "Cucumber was given the glue path '%s'. This path points to a source directory\n" + "in your project. However cucumber looks for glue (i.e. step definitions) on the\n" + diff --git a/core/src/test/java/io/cucumber/core/feature/GluePathTest.java b/core/src/test/java/io/cucumber/core/feature/GluePathTest.java index 09912bdafa..acf3f2b9bb 100644 --- a/core/src/test/java/io/cucumber/core/feature/GluePathTest.java +++ b/core/src/test/java/io/cucumber/core/feature/GluePathTest.java @@ -36,8 +36,8 @@ void can_parse_empty_glue_path() { URI uri = GluePath.parse(""); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is("/"))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is("/"))); } @Test @@ -45,8 +45,8 @@ void can_parse_root_package() { URI uri = GluePath.parse("classpath:/"); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is("/"))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is("/"))); } @Test @@ -55,8 +55,8 @@ void can_parse_eclipse_plugin_default_glue() { URI uri = GluePath.parse("classpath:"); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is("/"))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is("/"))); } @Test @@ -64,8 +64,8 @@ void can_parse_classpath_form() { URI uri = GluePath.parse("classpath:com/example/app"); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is("com/example/app"))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is("com/example/app"))); } @Test @@ -73,8 +73,8 @@ void can_parse_relative_path_form() { URI uri = GluePath.parse("com/example/app"); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is("/com/example/app"))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is("/com/example/app"))); } @Test @@ -82,8 +82,8 @@ void can_parse_absolute_path_form() { URI uri = GluePath.parse("/com/example/app"); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is("/com/example/app"))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is("/com/example/app"))); } @Test @@ -91,8 +91,8 @@ void can_parse_package_form() { URI uri = GluePath.parse("com.example.app"); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is("/com/example/app"))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is("/com/example/app"))); } @Test @@ -100,7 +100,7 @@ void glue_path_must_have_class_path_scheme() { Executable testMethod = () -> GluePath.parse("file:com/example/app"); IllegalArgumentException actualThrown = assertThrows(IllegalArgumentException.class, testMethod); assertThat("Unexpected exception message", actualThrown.getMessage(), is(equalTo( - "The glue path must have a classpath scheme file:com/example/app"))); + "The glue path must have a classpath scheme file:com/example/app"))); } @Test @@ -108,7 +108,7 @@ void glue_path_must_have_valid_identifier_parts() { Executable testMethod = () -> GluePath.parse("01-examples"); IllegalArgumentException actualThrown = assertThrows(IllegalArgumentException.class, testMethod); assertThat("Unexpected exception message", actualThrown.getMessage(), is(equalTo( - "The glue path contained invalid identifiers 01-examples"))); + "The glue path contained invalid identifiers 01-examples"))); } @Test @@ -117,8 +117,8 @@ void can_parse_windows_path_form() { URI uri = GluePath.parse("com\\example\\app"); assertAll( - () -> assertThat(uri.getScheme(), is("classpath")), - () -> assertThat(uri.getSchemeSpecificPart(), is(equalTo("/com/example/app")))); + () -> assertThat(uri.getScheme(), is("classpath")), + () -> assertThat(uri.getSchemeSpecificPart(), is(equalTo("/com/example/app")))); } @Test @@ -127,7 +127,7 @@ void absolute_windows_path_form_is_not_valid() { Executable testMethod = () -> GluePath.parse("C:\\com\\example\\app"); IllegalArgumentException actualThrown = assertThrows(IllegalArgumentException.class, testMethod); assertThat("Unexpected exception message", actualThrown.getMessage(), is(equalTo( - "The glue path must have a classpath scheme C:/com/example/app"))); + "The glue path must have a classpath scheme C:/com/example/app"))); } @ParameterizedTest @@ -153,16 +153,16 @@ void warn_when_glue_as_filesystem_path(String gluePath, Matcher logPatte static Stream GluePathAndPatternProvider() { return Stream.of( - arguments("src/main/java", containsString("for example: ''")), - arguments("src/main/java/", containsString("for example: ''")), - arguments("src/main/java_other", nullValue()), - arguments("src/main/other", nullValue()), - arguments("src/main/java/com", containsString("for example: 'com'")), - arguments("src/main/java/com/", containsString("for example: 'com'")), - arguments("src/main/groovy/com", containsString("for example: 'com'")), - arguments("src/main/java/com/example", containsString("for example: 'com.example'")), - arguments("src/main/java/com/example/", containsString("for example: 'com.example'")), - arguments("src/main/java/com/example/package", containsString("for example: 'com.example.package'")) + arguments("src/main/java", containsString("for example: ''")), + arguments("src/main/java/", containsString("for example: ''")), + arguments("src/main/java_other", nullValue()), + arguments("src/main/other", nullValue()), + arguments("src/main/java/com", containsString("for example: 'com'")), + arguments("src/main/java/com/", containsString("for example: 'com'")), + arguments("src/main/groovy/com", containsString("for example: 'com'")), + arguments("src/main/java/com/example", containsString("for example: 'com.example'")), + arguments("src/main/java/com/example/", containsString("for example: 'com.example'")), + arguments("src/main/java/com/example/package", containsString("for example: 'com.example.package'")) ); } From ea75401c3b43f83df5c9a85bf713a5b1f2f87a9a Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Sun, 8 May 2022 23:03:46 +0200 Subject: [PATCH 08/13] Improve message and naming --- CONTRIBUTING.md | 2 +- .../io/cucumber/core/feature/GluePath.java | 16 ++++----- .../cucumber/core/feature/GluePathTest.java | 33 ++++++++++--------- 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 72da15c3c2..bb7654d083 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,7 +15,7 @@ We appreciate that. Do keep the following in mind: To automatically format the java source code run ``` -mvn spotless:apply +mvn initialize spotless:apply ``` To configure Intelij IDEA/Eclipse use the configuration files in `.spotless/`. diff --git a/core/src/main/java/io/cucumber/core/feature/GluePath.java b/core/src/main/java/io/cucumber/core/feature/GluePath.java index cd336566f3..4f36cc9c03 100644 --- a/core/src/main/java/io/cucumber/core/feature/GluePath.java +++ b/core/src/main/java/io/cucumber/core/feature/GluePath.java @@ -36,7 +36,7 @@ public class GluePath { private static final Logger log = LoggerFactory.getLogger(GluePath.class); - private static final Pattern FILESYSTEM_PATH_TO_PACKAGES = Pattern + private static final Pattern WELL_KNOWN_PROJECT_SOURCE_DIRECTORIES = Pattern .compile("src/(?:main|test)/(?:java|kotlin|scala|groovy)(|/|/.+)"); private GluePath() { @@ -80,7 +80,7 @@ private static String replaceNonStandardPathSeparator(String featureIdentifier) private static URI parseAssumeClasspathScheme(String gluePath) { URI uri = URI.create(gluePath); - warnWhenFileSystemPath(gluePath); + warnWhenWellKnownProjectSourceDirectory(gluePath); String schemeSpecificPart = uri.getSchemeSpecificPart(); if (!isValidIdentifier(schemeSpecificPart)) { @@ -104,8 +104,8 @@ private static URI parseAssumeClasspathScheme(String gluePath) { return uri; } - private static void warnWhenFileSystemPath(String gluePath) { - Matcher matcher = FILESYSTEM_PATH_TO_PACKAGES.matcher(gluePath); + private static void warnWhenWellKnownProjectSourceDirectory(String gluePath) { + Matcher matcher = WELL_KNOWN_PROJECT_SOURCE_DIRECTORIES.matcher(gluePath); if (!matcher.matches()) { return; } @@ -119,11 +119,11 @@ private static void warnWhenFileSystemPath(String gluePath) { } String packageName = classPathResource.replaceAll("/", "."); String message = "" + - "Cucumber was given the glue path '%s'. This path points to a source directory\n" + - "in your project. However cucumber looks for glue (i.e. step definitions) on the\n" + - "classpath.\n" + + "Consider changing the glue path from '%s' to '%s'.\n'" + "\n" + - "To avoid confusion consider using a package name instead for example: '%s'\n"; + "The current glue path points to a source directory in your project. However " + + "cucumber looks for glue (i.e. step definitions) on the classpath. By using a " + + "package name you can avoid this ambiguity."; return String.format(message, gluePath, packageName); }); } diff --git a/core/src/test/java/io/cucumber/core/feature/GluePathTest.java b/core/src/test/java/io/cucumber/core/feature/GluePathTest.java index acf3f2b9bb..1e6aeb4f86 100644 --- a/core/src/test/java/io/cucumber/core/feature/GluePathTest.java +++ b/core/src/test/java/io/cucumber/core/feature/GluePathTest.java @@ -2,9 +2,7 @@ import io.cucumber.core.logging.LogRecordListener; import io.cucumber.core.logging.LoggerFactory; -import org.hamcrest.CoreMatchers; import org.hamcrest.Matcher; -import org.hamcrest.text.IsEmptyString; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledOnOs; import org.junit.jupiter.api.condition.OS; @@ -20,11 +18,8 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.isEmptyString; -import static org.hamcrest.Matchers.matchesPattern; import static org.hamcrest.core.Is.is; import static org.hamcrest.core.IsEqual.equalTo; -import static org.hamcrest.text.IsEmptyString.emptyOrNullString; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.params.provider.Arguments.arguments; @@ -153,18 +148,26 @@ void warn_when_glue_as_filesystem_path(String gluePath, Matcher logPatte static Stream GluePathAndPatternProvider() { return Stream.of( - arguments("src/main/java", containsString("for example: ''")), - arguments("src/main/java/", containsString("for example: ''")), + arguments("src/main/java/com/example/package", + equalTo("" + + "Consider changing the glue path from " + + "'src/main/java/com/example/package' to " + + "'com.example.package'.\n" + + "'\n" + + "The current glue path points to a source " + + "directory in your project. However cucumber " + + "looks for glue (i.e. step definitions) on the " + + "classpath. By using a package name you can " + + "avoid this ambiguity.")), + arguments("src/main/java", containsString("to ''")), + arguments("src/main/java/", containsString("to ''")), arguments("src/main/java_other", nullValue()), arguments("src/main/other", nullValue()), - arguments("src/main/java/com", containsString("for example: 'com'")), - arguments("src/main/java/com/", containsString("for example: 'com'")), - arguments("src/main/groovy/com", containsString("for example: 'com'")), - arguments("src/main/java/com/example", containsString("for example: 'com.example'")), - arguments("src/main/java/com/example/", containsString("for example: 'com.example'")), - arguments("src/main/java/com/example/package", containsString("for example: 'com.example.package'")) - - ); + arguments("src/main/java/com", containsString("to 'com'")), + arguments("src/main/java/com/", containsString("to 'com'")), + arguments("src/main/groovy/com", containsString("to 'com'")), + arguments("src/main/java/com/example", containsString("to 'com.example'")), + arguments("src/main/java/com/example/", containsString("to 'com.example'"))); } } From 4df843ec82b73c30850c6ce912cf1d90675ce0a5 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Sun, 8 May 2022 23:08:47 +0200 Subject: [PATCH 09/13] Revert selected changes --- .../core/runtime/FeaturePathFeatureSupplierTest.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/core/src/test/java/io/cucumber/core/runtime/FeaturePathFeatureSupplierTest.java b/core/src/test/java/io/cucumber/core/runtime/FeaturePathFeatureSupplierTest.java index 75974df595..05faae1ebb 100644 --- a/core/src/test/java/io/cucumber/core/runtime/FeaturePathFeatureSupplierTest.java +++ b/core/src/test/java/io/cucumber/core/runtime/FeaturePathFeatureSupplierTest.java @@ -14,11 +14,11 @@ import java.util.function.Supplier; import static java.util.Collections.singletonList; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.matchesPattern; -import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertThrows; class FeaturePathFeatureSupplierTest { @@ -44,9 +44,8 @@ void logs_message_if_no_features_are_found() { FeaturePathFeatureSupplier supplier = new FeaturePathFeatureSupplier(classLoader, featureOptions, parser); supplier.get(); - assertThat(logRecordListener.getLogRecords().get(1).getMessage(), - matchesPattern(".*No features found at.*io/cucumber/core/options/?")); + equalTo("No features found at classpath:io/cucumber/core/options")); } @Test From b758e8cc5a868c67fe33917bf35934d6af45ac04 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Sun, 8 May 2022 23:15:39 +0200 Subject: [PATCH 10/13] Use original short wording, it was better --- .../io/cucumber/core/feature/GluePath.java | 2 +- .../io/cucumber/core/feature/GluePathTest.java | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/io/cucumber/core/feature/GluePath.java b/core/src/main/java/io/cucumber/core/feature/GluePath.java index 4f36cc9c03..601fd9c41d 100644 --- a/core/src/main/java/io/cucumber/core/feature/GluePath.java +++ b/core/src/main/java/io/cucumber/core/feature/GluePath.java @@ -119,7 +119,7 @@ private static void warnWhenWellKnownProjectSourceDirectory(String gluePath) { } String packageName = classPathResource.replaceAll("/", "."); String message = "" + - "Consider changing the glue path from '%s' to '%s'.\n'" + + "Consider replacing glue path '%s' with '%s'.\n'" + "\n" + "The current glue path points to a source directory in your project. However " + "cucumber looks for glue (i.e. step definitions) on the classpath. By using a " + diff --git a/core/src/test/java/io/cucumber/core/feature/GluePathTest.java b/core/src/test/java/io/cucumber/core/feature/GluePathTest.java index 1e6aeb4f86..72121d7d7f 100644 --- a/core/src/test/java/io/cucumber/core/feature/GluePathTest.java +++ b/core/src/test/java/io/cucumber/core/feature/GluePathTest.java @@ -150,8 +150,8 @@ static Stream GluePathAndPatternProvider() { return Stream.of( arguments("src/main/java/com/example/package", equalTo("" + - "Consider changing the glue path from " + - "'src/main/java/com/example/package' to " + + "Consider replacing glue path " + + "'src/main/java/com/example/package' with " + "'com.example.package'.\n" + "'\n" + "The current glue path points to a source " + @@ -159,15 +159,15 @@ static Stream GluePathAndPatternProvider() { "looks for glue (i.e. step definitions) on the " + "classpath. By using a package name you can " + "avoid this ambiguity.")), - arguments("src/main/java", containsString("to ''")), - arguments("src/main/java/", containsString("to ''")), + arguments("src/main/java", containsString("with ''")), + arguments("src/main/java/", containsString("with ''")), arguments("src/main/java_other", nullValue()), arguments("src/main/other", nullValue()), - arguments("src/main/java/com", containsString("to 'com'")), - arguments("src/main/java/com/", containsString("to 'com'")), - arguments("src/main/groovy/com", containsString("to 'com'")), - arguments("src/main/java/com/example", containsString("to 'com.example'")), - arguments("src/main/java/com/example/", containsString("to 'com.example'"))); + arguments("src/main/java/com", containsString("with 'com'")), + arguments("src/main/java/com/", containsString("with 'com'")), + arguments("src/main/groovy/com", containsString("with 'com'")), + arguments("src/main/java/com/example", containsString("with 'com.example'")), + arguments("src/main/java/com/example/", containsString("with 'com.example'"))); } } From 4d5498e0133570971a371dcdda075e0d99468693 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Sun, 8 May 2022 23:33:30 +0200 Subject: [PATCH 11/13] Revert feature path changes --- .../io/cucumber/core/feature/FeaturePath.java | 23 ------------ .../core/feature/FeaturePathTest.java | 37 ------------------- 2 files changed, 60 deletions(-) diff --git a/core/src/main/java/io/cucumber/core/feature/FeaturePath.java b/core/src/main/java/io/cucumber/core/feature/FeaturePath.java index 16d17ef16a..ed2f1ae301 100644 --- a/core/src/main/java/io/cucumber/core/feature/FeaturePath.java +++ b/core/src/main/java/io/cucumber/core/feature/FeaturePath.java @@ -1,13 +1,8 @@ package io.cucumber.core.feature; -import io.cucumber.core.logging.Logger; -import io.cucumber.core.logging.LoggerFactory; - import java.io.File; import java.net.URI; import java.util.Locale; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import static io.cucumber.core.resource.ClasspathSupport.CLASSPATH_SCHEME_PREFIX; import static io.cucumber.core.resource.ClasspathSupport.RESOURCE_SEPARATOR_CHAR; @@ -32,10 +27,6 @@ */ public class FeaturePath { - private static final Logger log = LoggerFactory.getLogger(FeaturePath.class); - - private static final Pattern FILESYSTEM_PATH_TO_RESOURCES = Pattern.compile("src/(?:main|test)/resources(/?)(.*)"); - private FeaturePath() { } @@ -78,7 +69,6 @@ private static String replaceNonStandardPathSeparator(String featureIdentifier) } private static URI parseAssumeFileScheme(String featureIdentifier) { - warnWhenFileSystemPathToResources(featureIdentifier); File featureFile = new File(featureIdentifier); return featureFile.toURI(); } @@ -111,17 +101,4 @@ private static String normalize(final String value) { return value.toLowerCase(Locale.US).replaceAll("[^a-z0-9]+", ""); } - private static void warnWhenFileSystemPathToResources(String featureIdentifier) { - Matcher matcher = FILESYSTEM_PATH_TO_RESOURCES.matcher(featureIdentifier); - if (matcher.matches()) { - if (!matcher.group(1).equals("/") && matcher.group(2).length() > 0) { - return; - } - log.warn( - () -> String.format("Consider replacing feature path '%s' with '%s'.", - featureIdentifier, - matcher.replaceAll("classpath:$2"))); - } - } - } diff --git a/core/src/test/java/io/cucumber/core/feature/FeaturePathTest.java b/core/src/test/java/io/cucumber/core/feature/FeaturePathTest.java index 3628a78b8f..64ea3375ee 100644 --- a/core/src/test/java/io/cucumber/core/feature/FeaturePathTest.java +++ b/core/src/test/java/io/cucumber/core/feature/FeaturePathTest.java @@ -1,26 +1,18 @@ package io.cucumber.core.feature; -import io.cucumber.core.logging.LogRecordListener; -import io.cucumber.core.logging.LoggerFactory; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledOnOs; import org.junit.jupiter.api.condition.EnabledOnOs; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; import java.io.File; import java.net.URI; -import java.util.stream.Stream; import static org.hamcrest.CoreMatchers.endsWith; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.Is.is; -import static org.hamcrest.text.MatchesPattern.matchesPattern; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.condition.OS.WINDOWS; -import static org.junit.jupiter.params.provider.Arguments.arguments; class FeaturePathTest { @@ -154,33 +146,4 @@ void can_parse_windows_file_path_with_standard_file_separator() { () -> assertThat(uri.getSchemeSpecificPart(), is("/C:/path/to/file.feature"))); } - @ParameterizedTest - @MethodSource("FeaturePathAndPatternProvider") - void warn_when_features_as_filesystem_path_to_resources(String featurePath, String logPattern) { - // warn when 'src/{test,main}/resources' is used - - LogRecordListener logRecordListener = new LogRecordListener(); - LoggerFactory.addListener(logRecordListener); - - FeaturePath.parse(featurePath); - - LoggerFactory.removeListener(logRecordListener); - - String logMessage = ""; - if (!logRecordListener.getLogRecords().isEmpty()) { - logMessage = logRecordListener.getLogRecords().get(0).getMessage(); - } - - assertThat(logMessage, matchesPattern(logPattern)); - } - - static Stream FeaturePathAndPatternProvider() { - return Stream.of( - arguments("src/test/resources_abcd", ""), - arguments("src/test/resources/", ".*replacing.*src/test/resources/.*'classpath:'.*"), - arguments("src/test/resources/features", - ".*replacing.*src/test/resources/features.*'classpath:features'.*"), - arguments("src/main/resources/features/feature1", - ".*replacing.*src/main/resources/features/feature1.*'classpath:features/feature1'.*")); - } } From a40005eade7d9ebb8354e9e8180f13a9974ab885 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Sun, 8 May 2022 23:35:34 +0200 Subject: [PATCH 12/13] Naming stuff --- .../test/java/io/cucumber/core/feature/GluePathTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/test/java/io/cucumber/core/feature/GluePathTest.java b/core/src/test/java/io/cucumber/core/feature/GluePathTest.java index 72121d7d7f..7eb2f747fc 100644 --- a/core/src/test/java/io/cucumber/core/feature/GluePathTest.java +++ b/core/src/test/java/io/cucumber/core/feature/GluePathTest.java @@ -126,8 +126,8 @@ void absolute_windows_path_form_is_not_valid() { } @ParameterizedTest - @MethodSource("GluePathAndPatternProvider") - void warn_when_glue_as_filesystem_path(String gluePath, Matcher logPattern) { + @MethodSource("warn_when_glue_as_filesystem_path_examples") + void when_when_glue_path_is_well_known_source_directory(String gluePath, Matcher logPattern) { // warn when 'src/{test,main}/{java,kotlin,scala,groovy}' is used LogRecordListener logRecordListener = new LogRecordListener(); @@ -146,7 +146,7 @@ void warn_when_glue_as_filesystem_path(String gluePath, Matcher logPatte assertThat(logMessage, logPattern); } - static Stream GluePathAndPatternProvider() { + static Stream warn_when_glue_as_filesystem_path_examples() { return Stream.of( arguments("src/main/java/com/example/package", equalTo("" + From 726d09a66fe7c81dda489533ab7475d3fe980e13 Mon Sep 17 00:00:00 2001 From: tal66 <77445020+tal66@users.noreply.github.com> Date: Tue, 10 May 2022 23:43:29 +0300 Subject: [PATCH 13/13] Update CHANGELOG.md --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8082e0d82..c8d24857d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +## [7.3.5] (2022-05-10) + +### Added +* [Core] Warn when glue path is passed as file scheme instead of classpath ([#2547](https://github.com/cucumber/cucumber-jvm/pull/2547) M.P. Korstanje) + ## [7.3.4] (2022-05-02) ### Fixed