Feature/given when then snippet

crosscode-nl commented 4 months ago

🤔 What's changed?

Support multiple snippet render functions for generating code for undefined steps.

⚡️ What's your motivation?

I created this to enhance issue cucumber/godog#545

I wanted to generate Given When Then functions instead of Step functions.

I wanted to generate steps with methods on a fixture instead of functions.

I wanted to refactor the code a bit to allow multiple snippet templates.

This all would allow to generate code that matches a teams WoW with Godog.

🏷️ What kind of change is this?

Added

Option SnippetFunc to select a snippet func for generating code for missing steps
Added SnippetFunc "gwt_func" to generate Given/When/Then step snippets
Added SnippetFunc "step_func" to generate Step snippets - this is the original and the default functionality

Changed

BREAKING CHANGE, changed formatters.FormatterFunc to take a snippetFunc string parameter

♻️ Anything particular you want feedback on?

I believe the already existing code could use some refactoring and tidying. For now I moved some code around to prevent circular dependencies. Mostly due to formatters.StepDefinition (now models.StepDefinitionBase) and the keywords in formatters.

📋 Checklist:

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

vearutop commented 4 months ago

I did not look deeply in the changes, but only checked gorelease report yet.

# github.com/cucumber/godog
## incompatible changes
StepDefinition: changed from github.com/cucumber/godog/formatters.StepDefinition to github.com/cucumber/godog/internal/models.StepDefinitionBase

# github.com/cucumber/godog/formatters
## incompatible changes
Formatter.Defined: changed from func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *StepDefinition) to func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *github.com/cucumber/godog/internal/models.StepDefinitionBase)
Formatter.Failed: changed from func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *StepDefinition, error) to func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *github.com/cucumber/godog/internal/models.StepDefinitionBase, error)
Formatter.Passed: changed from func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *StepDefinition) to func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *github.com/cucumber/godog/internal/models.StepDefinitionBase)
Formatter.Pending: changed from func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *StepDefinition) to func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *github.com/cucumber/godog/internal/models.StepDefinitionBase)
Formatter.Skipped: changed from func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *StepDefinition) to func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *github.com/cucumber/godog/internal/models.StepDefinitionBase)
Formatter.Summary: changed from func() to func(github.com/cucumber/godog/internal/snippets.Func)
Formatter.Undefined: changed from func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *StepDefinition) to func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *github.com/cucumber/godog/internal/models.StepDefinitionBase)
Given: removed
Keyword: removed
None: removed
StepDefinition: removed
Then: removed
When: removed

There are a lot of breaking changes in public API, if we merge this, code of many users might get broken which is bad. Can the change be implemented in a backwards compatible way?

crosscode-nl commented 4 months ago

I did not look deeply in the changes, but only checked gorelease report yet.

# github.com/cucumber/godog
## incompatible changes
StepDefinition: changed from github.com/cucumber/godog/formatters.StepDefinition to github.com/cucumber/godog/internal/models.StepDefinitionBase

# github.com/cucumber/godog/formatters
## incompatible changes
Formatter.Defined: changed from func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *StepDefinition) to func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *github.com/cucumber/godog/internal/models.StepDefinitionBase)
Formatter.Failed: changed from func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *StepDefinition, error) to func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *github.com/cucumber/godog/internal/models.StepDefinitionBase, error)
Formatter.Passed: changed from func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *StepDefinition) to func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *github.com/cucumber/godog/internal/models.StepDefinitionBase)
Formatter.Pending: changed from func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *StepDefinition) to func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *github.com/cucumber/godog/internal/models.StepDefinitionBase)
Formatter.Skipped: changed from func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *StepDefinition) to func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *github.com/cucumber/godog/internal/models.StepDefinitionBase)
Formatter.Summary: changed from func() to func(github.com/cucumber/godog/internal/snippets.Func)
Formatter.Undefined: changed from func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *StepDefinition) to func(*github.com/cucumber/messages/go/v21.Pickle, *github.com/cucumber/messages/go/v21.PickleStep, *github.com/cucumber/godog/internal/models.StepDefinitionBase)
Given: removed
Keyword: removed
None: removed
StepDefinition: removed
Then: removed
When: removed

There are a lot of breaking changes in public API, if we merge this, code of many users might get broken which is bad. Can the change be implemented in a backwards compatible way?

thanks for already taking a look at my draft a PR

I will give it a try later. I made the change to resolve a circular dependency. Package formatters depended on models and models depended on formatters.

Edit: I see I really did change the public API, confusing formatters with internal/formatters. I will change it back and investigate further what caused the circular dependency.

crosscode-nl commented 4 months ago

I modified the PR. It should now have less breaking changes to the public API. I don't think I can do completely without it to implement this feature since there is no path to the snippet code which allowed configuration passing. Besides a global variable - which is terrible to have imho - I could not think of a way that would not change the public API.

crosscode-nl commented 4 months ago

Fixed the issues, but I had to remove support for v1.16. If you really want to I can add that support back in but then I have to add a deprecated function back in: strings.Title. It is the external package that string.Title advices to use that is not compatible with Go v1.16.

Later I want to add expression support using Cucumbers expression implementation. They only support 1.20 and 1.21, since older versions are deprecated anyway.

codecov[bot] commented 4 months ago

Codecov Report

Attention: 17 lines in your changes are missing coverage. Please review.

Comparison is base (5a5631a) 83.22% compared to head (e81113e) 83.83%.

Files	Patch %	Lines
fmt.go	0.00%	12 Missing :warning:
flags.go	72.72%	2 Missing and 1 partial :warning:
internal/formatters/fmt_base.go	77.77%	2 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #596 +/- ## ========================================== + Coverage 83.22% 83.83% +0.60% ========================================== Files 28 27 -1 Lines 3416 3334 -82 ========================================== - Hits 2843 2795 -48 + Misses 458 429 -29 + Partials 115 110 -5 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

vearutop commented 4 months ago

I still haven't checked all the changes (hopefully will have time tomorrow), but at glance I think we don't need an option for snippet function.

I think it would be better to just switch unconditionally to new syntax. Anyway, these code snippets are just suggestions to developer, and if developer wants/needs Step, they can change it while pasting/implementing the step definition.

cucumber / godog