DONT MERGE YET - REVIEW COMMENTS PLEASE - Test renovation - in preparation for fixing the Hook handler bug and adding other features.

[Johnlon]

🤔 What's changed?

suite_context_test.go is now testing solely using the Public of godog api. To highlight it's new lease of life it is renamed to functional_tests.go

Previously this suite wasn't really testing godog at all, because it was testing a fake godog-like mock up.
Consequently a bunch of the tests weren't actually doing anything useful. The most useful tests therefore were the fmt_output_tests which DID go via the real product.

Much of the code changes are a consequence of fixing these issues.

• Fixed broken tests
• Removed duplicates tests
• Removed bogus / useless tests
• Relocated tests
• Added new tests

The public api has an additiive change TestSuite.RunWithResults() that returns the exit code plus a reference to the storage. This addiitonal public api is really useful for various reasons (eg postprocessing) but importantly its a hook needed to do some of the tests without totally hackery.

Formatter.Close()

This is conceivably a breaking if anyone has written their own formatter.

I added Close() to the Formatter api so that the framework will reliably close any files opened inside the formatters (eg cuke).

This gap was the cause of a testing problem that became apparent once using the proper cuke public api because the formatter output files were not getting flushed because there were never getting closed during the tests. The close is gives predictable (and correct) behaviour. This problem would only afflict godog where it's used in the "Library" mode because users of the CLI would have has the files closed as the CLI exited.

⚡️ What's your motivation?

Product is a bit too hard to work with and I want to add some features/fixes. This is only a partial renovation.

🏷️ What kind of change is this?

• :bank: Refactoring/debt/DX (improvement to code design, tooling, etc. without changing behaviour)
• :bug: Bug fix (non-breaking change which fixes a defect)
• :zap: New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

📋 Checklist:

• [x] I agree to respect and uphold the Cucumber Community Code of Conduct
• [ ] I've changed the behaviour of the code
  • [x] 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.

[github-actions[bot]]

Go API Changes

# github.com/cucumber/godog
## incompatible changes
NewBaseFmt: changed from func(string, io.Writer) *github.com/cucumber/godog/internal/formatters.Base to func(string, io.WriteCloser) *github.com/cucumber/godog/internal/formatters.Base
NewCukeFmt: changed from func(string, io.Writer) *github.com/cucumber/godog/internal/formatters.Cuke to func(string, io.WriteCloser) *github.com/cucumber/godog/internal/formatters.Cuke
NewEventsFmt: changed from func(string, io.Writer) *github.com/cucumber/godog/internal/formatters.Events to func(string, io.WriteCloser) *github.com/cucumber/godog/internal/formatters.Events
NewJUnitFmt: changed from func(string, io.Writer) *github.com/cucumber/godog/internal/formatters.JUnit to func(string, io.WriteCloser) *github.com/cucumber/godog/internal/formatters.JUnit
NewPrettyFmt: changed from func(string, io.Writer) *github.com/cucumber/godog/internal/formatters.Pretty to func(string, io.WriteCloser) *github.com/cucumber/godog/internal/formatters.Pretty
NewProgressFmt: changed from func(string, io.Writer) *github.com/cucumber/godog/internal/formatters.Progress to func(string, io.WriteCloser) *github.com/cucumber/godog/internal/formatters.Progress
## compatible changes
Attach: added
Attachment: added
Attachments: added
ErrAmbiguous: added
ExitFailure: added
ExitOptionError: added
ExitSuccess: added
NopCloser: added
RunResult: added
StepAmbiguous: added
TestSuite.RunWithResult: added

# github.com/cucumber/godog/colors
## incompatible changes
Colored: changed from func(io.Writer) io.Writer to func(io.WriteCloser) io.WriteCloser
Uncolored: changed from func(io.Writer) io.Writer to func(io.WriteCloser) io.WriteCloser

# github.com/cucumber/godog/formatters
## incompatible changes
Formatter.Ambiguous: added
Formatter.Close: added
FormatterFunc: changed from func(string, io.Writer) Formatter to func(string, io.WriteCloser) Formatter

# summary
Inferred base version: v0.14.1
Suggested version: v0.15.0

[Johnlon]

First, thanks for the effort to counter entropy - always a good & worthy battle if the heart is in the right place, and I "feel" it is here from what I've seen. 😊

I've sprinkled in a few "first impression" (mostly nit type) comments without having really grokked the changes, which were just too many to take in at once.

Consider breaking this up into (maybe prioritized) sub-concerns? I'd be more likely to be interested & able to give you better feedback & generate more excitement for pulling downstream. 🤓

I think this PR is already the atom of improvement. I'd prefer to get it merged and then add the fixes for the bugs that motivated this