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 src/main or src/test is used in a glue or feature path #2543

Closed
mpkorstanje opened this issue Apr 30, 2022 · 4 comments · Fixed by #2547
Closed

Warn when src/main or src/test is used in a glue or feature path #2543

mpkorstanje opened this issue Apr 30, 2022 · 4 comments · Fixed by #2547
Assignees
Labels
good first issue Good for newcomers 🙏 help wanted Help wanted - not prioritized by core team ⚡ enhancement Request for new functionality

Comments

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Apr 30, 2022

👓 What did you see?

Novice users often do not understand the difference between the classpath and the filesystem path.

For example this mistake often shows up on stack overflow:

@RunWith(Cucumber.class)
@CucumberOptions(
        features = "src/test/resources/features",
        glue = "src/test/java/com/example"
)

This should be

@RunWith(Cucumber.class)
@CucumberOptions(
        features = "classpath:features",
        glue = "com.example"
)

Here scr/main/java/com/example is a filesystem path while classpath:com/example a classpath path.

Due to misunderstanding the difference between these paths novice users often specify paths that are positively not glue paths.

Feature paths often just happen to work because unlike glue features don't have to be on the classpath.

Additionally while classpath uris are supported for glue paths the package name format is preferred to avoid confusion.

✅ What did you expect to see?

When passing any feature path starting with src/{test,main}/resources emit a warning that suggests replacing it with classpath: prefix.

Whe passing a glue path starts with src/{test,main}/{java,kotlin,scala,groovy} emit a warning that advices the use of the package name.

The warning should include the original path used and the suggested replacement.

For the feature path it should also include an explanation that mentions the possible ambiguity and advice the user to avoid it. Either by using the suggested replacement or moving the feature files away from well known filesystem paths that are put on the classpath.

For glue paths it should inform users that files in these paths are typically compiled by their build tool before the compiled files are put on the classpath when executing. As such they probably meant to use the package name.

🛠️ Implementation hints

Feature path parsing is implemented in FeaturePath. Checking for the prefix in parseAssumeFileScheme seems like the right place.

Glue paths are parsed in GluePath. The parseAssumeClasspathScheme seems like the right place.

Logging warnings should be done using Cucumbers logging utility. For example see:

log.warn(() -> "You are using deprecated Main class. Please use io.cucumber.core.cli.Main");

🧪 Testing hints

You can test log messages by adding a LogRecordListener to the LoggerFactory. See the surrounding code for more context:

@BeforeEach
void setup() {
logRecordListener = new LogRecordListener();
LoggerFactory.addListener(logRecordListener);
}
@AfterEach
void tearDown() {
LoggerFactory.removeListener(logRecordListener);
}

@mpkorstanje mpkorstanje added ⚡ enhancement Request for new functionality 🙏 help wanted Help wanted - not prioritized by core team good first issue Good for newcomers labels Apr 30, 2022
@mpkorstanje mpkorstanje changed the title Warn when glue and feature paths are well known classpath paths Warn when src/main or src/test is used in a path Apr 30, 2022
@mpkorstanje mpkorstanje changed the title Warn when src/main or src/test is used in a path Warn when src/main or src/test is used in a glue or feature path Apr 30, 2022
@tal66
Copy link
Contributor

tal66 commented May 2, 2022

Hi, I started working on this issue :)

@mpkorstanje
Copy link
Contributor Author

Awesome!

I've added some hints for testing. If you have any problems you can ask for help in the #commiters channel in slack. Link is in the read me.

@AlexBravo95

This comment was marked as off-topic.

@mpkorstanje
Copy link
Contributor Author

@AlexBravo95 for questions please use stackoverflow. For bug reports, please create a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers 🙏 help wanted Help wanted - not prioritized by core team ⚡ enhancement Request for new functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants