Skip to content

Conversation

mpkorstanje
Copy link
Contributor

@mpkorstanje mpkorstanje commented Sep 4, 2025

⚡️ What's your motivation?

Cucumber implementations helpfully generate code to implement undefined steps. As we move to message based formatters, they should be able to provide these snippets to the user as well.

🏷️ What kind of change is this?

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

♻️ Anything particular you want feedback on?

I didn't quite follow the design in cucumber/common#2272 and I've aggregated the snippets for each pickle step into a single suggestion. This makes it easier to use the snippet type internally as the snippet generator can now generate snippets without knowing about the other parts of the event.

This does come with an implicit assumption that there is one suggestion per execution per undefined step. Does that assumption hold?

📋 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.

Cucumber implementations helpfully generate code to implement undefined
steps. As we move to message based formatters, they should be able to
provide these snippets to the user as well.
@mpkorstanje mpkorstanje force-pushed the add-suggested-code-snippets branch from 09028d2 to 432e2f6 Compare September 4, 2025 11:59
@mpkorstanje mpkorstanje marked this pull request as ready for review September 4, 2025 12:05
@clrudolphi
Copy link
Contributor

I feel I need a bit more context and explanation of the intent and presumed implementations.

Is a Suggestion message meant as a stand-alone message within a TestRun or is it meant to be embedded within a PickleStep, TestStep, or bounded by a TestStepStarted/Finished pair?

Is it expected that a Suggestion be produced for steps within Ignored/Skipped scenarios?

Is it expected that a Suggestion will be produced for each Pickle that results from an Scenario Outline Examples table? (this will be/might be repetitive)

@mpkorstanje
Copy link
Contributor Author

mpkorstanje commented Sep 4, 2025

The Suggestion is a standalone message. When it is emitted depends on the Cucumber implementation.

  • If an implementations step definition declarations are static then this message could theoretically be emitted before a test run even starts.
  • If an implementation interprets a pickle step-by-step then it would be emitted between a TestStepStarted/Finished.

So this message must be emitted after the Pickle it references, and before the TestStepFinished that tried to run the undefined step for which the snippets were generated (if any).

How the snippets are presented would depend on the tool consuming the messages. The suggested snippets might indeed get quite repetitive. And consumers should account for that.
We can't de-duplicate this earlier because the correct de-duplication strategy depends on the presentation. For example the summary printer might report on all undefined in a single single batch, while the JUnit XML reporter would de-duplicate on a per-pickle basis.

It would be sensible to only emit snippets for undefined steps as they are executed. But again this is implementation dependent. For example Cucumber JVM emits the snippets prior to the execution of any steps in a pickle, but that pickle is almost certainly going to be executed.

"description": "A unique id for this suggestion",
"type": "string"
},
"pickleId": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is pickleId strictly necessary given we have pickleStepId? Thinking about TestStep which has the latter and not the former.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mmh. Worth considering.

],
"properties": {
"language": {
"description": "The programming language of the code",
Copy link
Contributor

@davidjgoss davidjgoss Sep 4, 2025

Choose a reason for hiding this comment

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

Would it be worth saying this should be a lowercase identifier like java, typescript or ruby such that it would work with syntax highlighters like Prism without being remapped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. That's exactly what I was looking for.

Copy link
Contributor

@clrudolphi clrudolphi left a comment

Choose a reason for hiding this comment

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

LGTM

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