Warn when glue paths is a well known project source directory (#2547)

cucumber · May 17, 2022 · f175e4e · f175e4e
1 parent b106ab0
commit f175e4e
Show file tree

Hide file tree

Showing 6 changed files with 102 additions and 10 deletions.
diff --git 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

diff --git 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
@@ -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 WELL_KNOWN_PROJECT_SOURCE_DIRECTORIES = 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);
 
+        warnWhenWellKnownProjectSourceDirectory(gluePath);
+
         String schemeSpecificPart = uri.getSchemeSpecificPart();
         if (!isValidIdentifier(schemeSpecificPart)) {
             throw new IllegalArgumentException("The glue path contained invalid identifiers " + uri);
@@ -92,6 +104,30 @@ private static URI parseAssumeClasspathScheme(String gluePath) {
         return uri;
     }
 
+    private static void warnWhenWellKnownProjectSourceDirectory(String gluePath) {
+        Matcher matcher = WELL_KNOWN_PROJECT_SOURCE_DIRECTORIES.matcher(gluePath);
+        if (!matcher.matches()) {
+            return;
+        }
+        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 = "" +
+                    "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 " +
+                    "package name you can avoid this ambiguity.";
+            return String.format(message, gluePath, packageName);
+        });
+    }
+
     private static boolean isProbablyPackage(String gluePath) {
         return gluePath.contains(PACKAGE_SEPARATOR_STRING)
                 && !gluePath.contains(RESOURCE_SEPARATOR_STRING);

diff --git 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,28 @@
 package io.cucumber.core.feature;
 
+import io.cucumber.core.logging.LogRecordListener;
+import io.cucumber.core.logging.LoggerFactory;
+import org.hamcrest.Matcher;
 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.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.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 +125,49 @@ void absolute_windows_path_form_is_not_valid() {
             "The glue path must have a classpath scheme C:/com/example/app")));
     }
 
+    @ParameterizedTest
+    @MethodSource("warn_when_glue_as_filesystem_path_examples")
+    void when_when_glue_path_is_well_known_source_directory(String gluePath, Matcher<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);
+
+        String logMessage = logRecordListener.getLogRecords()
+                .stream()
+                .findFirst()
+                .map(LogRecord::getMessage)
+                .orElse(null);
+
+        assertThat(logMessage, logPattern);
+    }
+
+    static Stream<Arguments> warn_when_glue_as_filesystem_path_examples() {
+        return Stream.of(
+            arguments("src/main/java/com/example/package",
+                equalTo("" +
+                        "Consider replacing glue path " +
+                        "'src/main/java/com/example/package' with " +
+                        "'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("with ''")),
+            arguments("src/main/java/", containsString("with ''")),
+            arguments("src/main/java_other", nullValue()),
+            arguments("src/main/other", nullValue()),
+            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'")));
+    }
+
 }
diff --git 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,6 @@
 
 import io.cucumber.core.backend.ObjectFactory;
 import io.cucumber.core.exception.CucumberException;
-import io.cucumber.core.plugin.DefaultSummaryPrinter;
 import io.cucumber.core.plugin.HtmlFormatter;
 import io.cucumber.core.plugin.NoPublishFormatter;
 import io.cucumber.core.plugin.PluginFactory;
@@ -13,7 +12,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;
 

diff --git a/core/src/test/java/io/cucumber/core/runtime/FeaturePathFeatureSupplierTest.java b/core/src/test/java/io/cucumber/core/runtime/FeaturePathFeatureSupplierTest.java
@@ -15,10 +15,10 @@
 
 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.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 class FeaturePathFeatureSupplierTest {
@@ -40,15 +40,12 @@ 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();
-        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(logRecordListener.getLogRecords().get(1).getMessage(),
+            equalTo("No features found at classpath:io/cucumber/core/options"));
     }
 
     @Test