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/(.*)");
tal66 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
}
}

}
23 changes: 23 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,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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,9 @@ public List<LogRecord> getLogRecords() {
return Arrays.asList(logRecords.toArray(new LogRecord[0]));
}

public static boolean anyRecordMessageMatch(LogRecordListener listener, String regex) {
tal66 marked this conversation as resolved.
Show resolved Hide resolved
return listener.getLogRecords().stream()
.anyMatch(r -> r.getMessage().matches(regex));
}

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

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

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'.*")));
tal66 marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
void create_with_extra_glue() {
RuntimeOptions runtimeOptions = parser().parse(ClassWithExtraGlue.class).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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/?"));
tal66 marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
Expand Down