Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn when feature / glue paths passed as file schemes #2547

Merged
merged 13 commits into from
May 17, 2022
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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/`.
Expand Down
36 changes: 36 additions & 0 deletions core/src/main/java/io/cucumber/core/feature/GluePath.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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)(|/|/.+)");
tal66 marked this conversation as resolved.
Show resolved Hide resolved

private GluePath() {

}
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
56 changes: 56 additions & 0 deletions core/src/test/java/io/cucumber/core/feature/GluePathTest.java
Original file line number Diff line number Diff line change
@@ -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 {

Expand Down Expand Up @@ -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'")));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down