Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print step declaration line instead of handler declaration line #668

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

SpencerC
Copy link

@SpencerC SpencerC commented Nov 27, 2024

🤔 What's changed?

Pretty printing results now prints the line where the step is declared instead of the line where the handler is declared.

⚡️ What's your motivation?

Printing the file and line number of handlers often produces meaningless results. For example:

  • Anonymous functions: ctx.Step("...", func () {...}) always point to line 0: some_file_test.go:0
  • Instance receiver methods: ctx.Step("...", instance.Method) always point to: <autogenerated>:1.

Pointing to the step declaration does require an additional click to get to the handler code, so an alternative approach would be to improve the reflection process to do a better job locating the handler code.

🏷️ What kind of change is this?

  • 🏦 Refactoring/debt/DX (improvement to code design, tooling, etc. without changing behaviour)

This might be considered breaking if anyone is relying on the previous specific log format in CI tasks.

♻️ 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.

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.

1 participant