-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
WIP: Event stream formatter #962
Conversation
""" | ||
{"event":"TestRunStarted","protocol_version":"0.0.1"} | ||
{"event":"GherkinSourceRead","id":"features/simple.feature:1","source":"Feature:\n Scenario:\n Given this step passes"} | ||
{"event":"StepDefinitionFound","source_id":"features/simple.feature:3","definition_id": "features/step_definitions/steps.rb:1"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be named StepDefinitionMatched
instead? I think found can possibly be mistaken with loaded.
One more thing to point out, for pretty formatter. We probably need argument list to make them bold. Which should most probably go with StepDefinitionFound event: {
"event":"StepDefinitionFound",
"source_id":"features/simple.feature:3",
"definition_id": "features/step_definitions/steps.rb:1",
"arguments": [
[5, 10],
[16, 20]
]
} Which could mark positions from - to in step.Text. |
@l3pp4rd yes, agree regarding arguments. The offsets must be from the beginning of the line, since the formatter won't know at what column the step text starts (the text that was matched, following the step keyword). |
59bbfbf
to
1f0ba08
Compare
TODO: - Tests are failing because of extra "prepare world" in-built test step. What should we do with this? We could make the test more flexible, or try to disable the filter that adds this hook. - Tests are failing because of the duration added to the event. We could turn it off for now, but it's useful data. It would be better to introduce hack like we did for the test run time, and gsub it to something predictable.
Pairing with @everzet
1f0ba08
to
3ae61e3
Compare
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. |
@mattwynne do you want to keep this PR open? Should I try to move it forward or do you recommend starting over? |
@aslakhellesoy I think it's practically done. The only criticism I'd have is that it wasn't built test-first - I mostly manually tested it with the electron cucumber-gui spike, though it does (or did) work against the protocol described in cucumber/common#172 (comment) so that was part of testing it too. I'd have a crack and see if you can stabilise it. |
I think we can close this one now! 🎉 |
The goal of this ticket is to produce a stream of JSON events that conform to the standardised cucumber event protocol
This will allow cucumber to interoperate with other tools, and ultimately perhaps enable us to use a cross-platform formatter library.
The event protocol is new and emerging, and this PR aims to keep up with that.