Warn when feature / glue paths passed as file schemes

[tal66]

Hi, this closes #2543. changes:

• warn when 'src/{test,main}/resources' used in features path,
• warn when 'src/{test,main}/{java,kotlin,scala,groovy}' used in glue path
• tests for the above
• had to change core/runtime/FeaturePathFeatureSupplierTest as well, because it checked log messages at a specific index - which now changed. so also added a more convenient static method for this in LogRecordListener, used in my tests as well.

[codecov]

Codecov Report

Merging #2547 (726d09a) into main (94b54e2) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #2547      +/-   ##
============================================
+ Coverage     84.50%   84.53%   +0.02%     
- Complexity     2655     2661       +6     
============================================
  Files           310      310              
  Lines          9339     9356      +17     
  Branches        898      901       +3     
============================================
+ Hits           7892     7909      +17     
  Misses         1115     1115              
  Partials        332      332              

Impacted Files	Coverage Δ
...c/main/java/io/cucumber/core/feature/GluePath.java	81.13% <100.00%> (+8.90%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94b54e2...726d09a. Read the comment docs.

[mpkorstanje]

Thanks! That was quick!

Looks like we're going in the right direction over all but there are a few things to consider:

• It is usually a good idea to keep the tests as close to the code under test as possible. There are dedicated unit tests in FeaturePathTest and GluePathTest. Consider moving the tests there.

• I do think the anyRecordMatch method is pretty neat but I also think that after adding parameterized tests and fixing the root cause of the failure in the FeaturePathFeatureSupplierTest it won't be needed anymore. If done right there should only be one log record per test case.

[tal66]

thanks for the feedback, changed accordingly (except for adding explanation about the feature path, because it does work as file scheme, so not sure what more to say)

[mpkorstanje]

No problem. I'll think about the explanation a bit.

[mpkorstanje]

I think we're almost there now. Currently the regex patterns also match a few things they shouldn't match. Consider adding tests where nothing is logged.

Hint: You can pass matchers as an argument to the test method.

[tal66]

changed it.
but maybe reconsider whether the feature path warning is needed at all? if it's just a best practice, and has no noticeable effect on the execution, maybe a warning is not the place to mention that. especially if someone already put their feature files under resources (and those with src/main/resources-hello don't get a warning).

[mpkorstanje]

I've pushed a few improvements to the GluePath. Please let me know what you think about the explanation.

I still have to look at the FeaturePath. Hopefully that will come later.

[tal66]

let me know what you think about the explanation

I prefer shorter explanations, that start with a short action "change <x> to <y>" and might continue with a longer explanations for those interested. but it's a style thing, and your choice :)

[mpkorstanje]

That's a good idea.

• I have put the action first in the message. Then the explanation.
• I have removed the warning for FeaturePath. While it's not quite right, to reference files in src/{main,test}/resources because the build tool might still process them, it is a bit pedantic.

If you can add an entry in the changelog under "Added" I think we're done.

[tal66]

closing this?

[mpkorstanje]

Will be soon. It's good!

[aslakhellesoy]

Hi @tal66,

Thanks for your making your first contribution to Cucumber, and welcome to the Cucumber committers team! You can now push directly to this repo and all other repos under the cucumber organization! 🍾

In return for this generous offer we hope you will:

• ✅ Continue to use branches and pull requests. When someone on the core team approves a pull request (yours or someone else's), you're welcome to merge it yourself.
• 💚 Commit to setting a good example by following and upholding our code of conduct in your interactions with other collaborators and users.
• 💬 Join the community Slack channel to meet the rest of the team and make yourself at home.
• ℹ️ Don't feel obliged to help, just do what you can if you have the time and the energy.
• 🙋 Ask if you need anything. We're looking for feedback about how to make the project more welcoming, so please tell us!

On behalf of the Cucumber core team,
Aslak Hellesøy
Creator of Cucumber

[mpkorstanje]

Cheer @tal66! Sorry for the delay. Very happy with this. If you're looking for more to do, you may like #2546.