pytest-bdd
pytest-bdd copied to clipboard
Allow for scenario descriptions to be present
Fixes #311
In general, I tried to treat scenario descriptions exactly the same way as feature descriptions are currently treated. I'm not sure to what extent feature descriptions are used besides being an attribute on the Feature class, but the Scenario class now has a description attribute as well.
The biggest uncertainty I have with the code in this initial version of the PR is the testing. The current bug is a strange one which involves it creating extra scenarios that would be executed, and would pass with correctly implemented steps. Basically the same scenario runs as many times as there are lines in the scenario description.
The way I discovered the bug was by using the skip marker. The skip marker was only applied to the first, real scenario, so the subsequent "rogue" scenarios were not skipped, and thus gave me error for not implemented steps. In the first commit, I created tests that implement that. Without the fix in the second commit, only the first test would be skipped, and the rogue ones would fail. Certainly give it a try!
I also added a test in the already existing description.feature and test_description.py files. Together, I think they sufficiently confirm that scenario descriptions are all good, but I'm happy to head suggestions for additional tests or test changes.
Thanks, and hopefully this can get moving quickly!
Coverage increased (+0.03%) to 96.147% when pulling a0e99e4370370b09881166ed1c4df76c72dea3e5 on rbcasperson:scenario-descriptions into 386ed90cec68f4a9e000728928bbb4854f9a56d4 on pytest-dev:master.
@youtux it's been a little while, but I've made the little tweak to get CI passing for this PR, but it looks like there's a 405 response error in travis when making a coveralls API call. Not sure how to address that.
I've also followed along in #306, and perhaps that new work will help with some of the parsing issues. But I don't see why these changes would have to wait for something like that.
@youtux pinging here again to see if I can get anyone to look at this PR. There are other things that I would also like to fix, but I want to be sure if I spend the time, it will actually be looked at. Thanks!
Codecov Report
Merging #312 into master will increase coverage by
0.00%. The diff coverage is96.55%.
@@ Coverage Diff @@
## master #312 +/- ##
=======================================
Coverage 95.80% 95.80%
=======================================
Files 57 57
Lines 2217 2241 +24
Branches 185 188 +3
=======================================
+ Hits 2124 2147 +23
Misses 62 62
- Partials 31 32 +1
| Impacted Files | Coverage Δ | |
|---|---|---|
| pytest_bdd/feature.py | 98.92% <93.75%> (-0.33%) |
:arrow_down: |
| tests/feature/test_description.py | 100.00% <100.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 34bc885...ec43c05. Read the comment docs.
@youtux it has been a little while, but I've updated the tests based on your suggestions. Hopefully you or someone can get a chance to look at this again. Thanks!
@youtux or any contributor, definitely looking for next steps on this one. Thanks!