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
Allows use <params> in parsers defined steps #433
Allows use <params> in parsers defined steps #433
Conversation
Codecov Report
@@ Coverage Diff @@
## master #433 +/- ##
==========================================
+ Coverage 96.14% 96.20% +0.06%
==========================================
Files 50 50
Lines 1684 1715 +31
Branches 159 161 +2
==========================================
+ Hits 1619 1650 +31
Misses 38 38
Partials 27 27
Continue to review full report at Codecov.
|
c65efda
to
861a34d
Compare
2f83eed
to
2531ca0
Compare
@olegpidsadnyi , please review |
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.
Interesting. Can you share the use case where the examples are mixed with the step arguments?
d5a61fe
to
8cfd432
Compare
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.
This looks good to me. What do you think, @youtux?
@elchupanebrej Does it really need to be dependent on #391? I'm not sure yet about that one. Cucumber uses rule option no-unused-variables for raising errors on that. Is #391 related to your change or you can make it an independent one? |
8cfd432
to
fced7d7
Compare
@olegpidsadnyi I'm interested in 391 also. I'll prepare a new patch with adequate API for it and will try to not use it in this patch |
fced7d7
to
3e71c12
Compare
So parsers could parse values defined in Examples
3e71c12
to
d1ff34a
Compare
@olegpidsadnyi, there is no more dependence on #391 |
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.
This looks okay to me.
@elchupanebrej could you make cov happy again? |
b09f6ac
to
231b108
Compare
@olegpidsadnyi done |
Just one thing I don't like about this PR: values from Examples section are converted using converters and after that serialized again to str. I would like to have a possibility to substitute Step variables directly, but this would require huge refactoring |
Sorry, I was having the second thoughts and reverted the merge. I don't really understand why do we need to substitute anything at all in the name of the step. Previously it was just a total match. Otherwise a parser match. |
#293 (comment) For my project I've solved this by using regular expressions parser and special converter, but I really prefer DRY way. For this PR I check two approaches - full substitution of variables and approach presented in this PR. The second one is more flexible, but more dangerous |
This covers #409 #404 #293
Blocked by #391, it has to be merged first