Skip to content

Conversation

grappler
Copy link

@grappler grappler commented Oct 3, 2025

🤔 What's changed?

Two new methods to be able to change the naming strategy and feature name. Moved the default values to the first method.

⚡️ What's your motivation?

To be able to set the naming strategy for the XML report.
cucumber/cucumber-jvm#3087

🏷️ What kind of change is this?

  • ⚡ New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

@grappler grappler changed the title Patch 1 Support configurable NamingStrategy.Strategy and NamingStrategy.FeatureName Oct 3, 2025
@grappler grappler changed the title Support configurable NamingStrategy.Strategy and NamingStrategy.FeatureName Support configurable NamingStrategy.Strategy and NamingStrategy.FeatureName in MessagesToJunitXmlWriter Oct 3, 2025
@vitalets
Copy link

vitalets commented Oct 4, 2025

Also relates to #74.

@grappler
Copy link
Author

grappler commented Oct 4, 2025

@vitalets Thank you for including that! I have added a JS solution, I hope you don't mind, as you were planning to work on a PR. Happy to get feedback.

@vitalets
Copy link

vitalets commented Oct 6, 2025

@vitalets Thank you for including that! I have added a JS solution, I hope you don't mind, as you were planning to work on a PR. Happy to get feedback.

Sure, thank you! I didn't get time for it(

return NamingStrategy.strategy(strategy).featureName(featureName).exampleName(exampleName).build();
}

private MessagesToJunitXmlWriter(NamingStrategy namingStrategy, OutputStream out) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vitalets The alternative would be to make this method public. This way the JS and Java solutions would be similar.

Copy link
Contributor

@mpkorstanje mpkorstanje Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That allows the whole naming strategy to be provided. Which isn't great if the test suite and test class names can't also be set because otherwise the FeatureName#EXCLUDE should always be used to avoid duplicate names feature names.

Also solved with the builder pattern.

}

export function makeReport(query: Query): ReportSuite {
export function makeReport(query: Query, customNamingStrategy: NamingStrategy = NAMING_STRATEGY): ReportSuite {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't compile.

- Makes the whole naming strategy configurable
- Make suite name configurable
- Make test class name configurable
@mpkorstanje
Copy link
Contributor

mpkorstanje commented Oct 6, 2025

Pushed a few changes.

  1. Replaced the long constructors with a builder pattern. This makes it easier to deal with optional values.
  2. Add 1 test case that uses the builder. This did mean all the test cases had to be renamed to avoid collisions.

@grappler would you like to finish the Javascript implementation? Javascript can use default values so the builder can be omitted, but adding the testSuiteName and testClassName parameters as well as the tests should be added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants