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
19 changes: 19 additions & 0 deletions core/src/main/java/io/cucumber/core/feature/FeaturePath.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.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;
Expand All @@ -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/?(.*)");
mpkorstanje marked this conversation as resolved.
Show resolved Hide resolved

private FeaturePath() {

}
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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")));
mpkorstanje marked this conversation as resolved.
Show resolved Hide resolved
}
}

}
28 changes: 28 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 FILESYSTEM_PATH_TO_PACKAGES = 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);

warnWhenFileSystemPath(gluePath);

String schemeSpecificPart = uri.getSchemeSpecificPart();
if (!isValidIdentifier(schemeSpecificPart)) {
throw new IllegalArgumentException("The glue path contained invalid identifiers " + uri);
Expand All @@ -92,6 +104,22 @@ 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()) {
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));
}
}
}

private static boolean isProbablyPackage(String gluePath) {
return gluePath.contains(PACKAGE_SEPARATOR_STRING)
&& !gluePath.contains(RESOURCE_SEPARATOR_STRING);
Expand Down
31 changes: 31 additions & 0 deletions core/src/test/java/io/cucumber/core/feature/FeaturePathTest.java
Original file line number Diff line number Diff line change
@@ -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 {

Expand Down Expand Up @@ -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<Arguments> 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'.*"));
}
}
32 changes: 32 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,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 {

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

}
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 @@ -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.is;
import static org.hamcrest.CoreMatchers.startsWith;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertAll;
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 {
Expand All @@ -40,15 +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();
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(),
matchesPattern(".*No features found at.*io/cucumber/core/options/?"));
}

@Test
Expand Down