Support rule keyword #441
Support rule keyword #441
Conversation
|
@michaelsauter - Thanks for running with this! |
|
Awesome @michaelsauter and thanks for doing your work in public! Let me know if you want to do some pairing ⚡ you can book a slot with me here. |
|
Thanks! Sorry I did not get to it again, but I should find some time in a couple of days. Will update here and might come back to your offer of pairing! |
michaelsauter
force-pushed
the
feature/rule
branch
from
ea16cbe
to
0bc77e2
Compare
Note that rules are not indented as that would complicate the logic considerably: every print directive would need to check if it is surrounded by a rule or not.
michaelsauter
force-pushed
the
feature/rule
branch
from
0bc77e2
to
e19f7af
Compare
|
@mxygem @mattwynne Alright, I finally had the time to work a bit on this. It's now at a state where at least the pretty format seems to work well. I have added some tests that demonstrate the behaviour. I would love to hear some feedback, and if this is still missing anything. One thing to call out maybe ... rules are not indented so far as that would complicate the logic considerably: every print directive would need to check if it is surrounded by a rule or not. What are your thoughts on this? |
Codecov Report
@@ Coverage Diff @@
## main #441 +/- ##
==========================================
- Coverage 81.47% 80.03% -1.44%
==========================================
Files 27 27
Lines 2186 2229 +43
==========================================
+ Hits 1781 1784 +3
- Misses 312 345 +33
- Partials 93 100 +7
Continue to review full report at Codecov.
|
|
I'll have another look into the code coverage when I get to it. |
| simple feature description | ||
|
|
||
| Background: simple background | ||
| Given passing step |
There was a problem hiding this comment.
I'm confused by this indentation - do we indent the steps by 4 characters within a Background?
There was a problem hiding this comment.
Or is this about having a universal fixed indentation for every step irrespective of whether it's inside a Rule, Background, Scenario etc?
There was a problem hiding this comment.
I think the indentation is actually ignored in the tests. I changed indentation and it did not caused the tests to fail.
|
Great work @michaelsauter. It looks like maybe we need some unit tests to cover the details of the |
|
One thing I wonder about might be missing, but we could do this in a separate PR, is to add support for tagging rules. |
|
Yeah, feature tests are included in code coverage, apparently current tests are not triggering particular code branches. |
|
I think there are a few more combinations that I can add to see if I can trigger those other branches. That said, I already added tests to cover some branches that were not tested before so I am a bit confused about the code coverage.
Good point, will check how hard this is and see if I can add it to this PR. Sorry it might be another couple of days before I get back to this ... |
|
@michaelsauter any updates on this one? Could really use it :) |
|
If @michaelsauter does not have capacity to finish with tests and code style, I think we can merge this PR to a temporary branch and I can then add missing parts. Let's wait couple of days for response. |
|
Sorry, I lost track of this and currently do not have a need for this. Would appreciate if you take this over. Thanks! |
|
Superseded by #480. Thanks @dumpsterfireproject! |
Related to #440.
After reading a bit more in the code I realized my original understanding of how it worked was wrong. Supporting
Rulemight not be that hard after all. I started implementing it a little. This is nowhere near finished though ...