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

Allow for scenario descriptions to be present #312

Closed

Conversation

rbcasperson
Copy link

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!

fixes pytest-dev#311

This treats scenario descriptions the same way it does
feature descriptions.
@coveralls
Copy link

coveralls commented Jul 25, 2019

Coverage Status

Coverage increased (+0.03%) to 96.147% when pulling a0e99e4 on rbcasperson:scenario-descriptions into 386ed90 on pytest-dev:master.

@rbcasperson rbcasperson force-pushed the scenario-descriptions branch from 9ca5b05 to 8fa23ca Compare August 13, 2019 15:19
@rbcasperson
Copy link
Author

@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.

@rbcasperson
Copy link
Author

@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-io
Copy link

codecov-io commented Dec 3, 2019

Codecov Report

Attention: Patch coverage is 96.55172% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.80%. Comparing base (34bc885) to head (ec43c05).
Report is 787 commits behind head on master.

Files with missing lines Patch % Lines
pytest_bdd/feature.py 93.75% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rcasperson-jc
Copy link

@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!

@rbcasperson rbcasperson requested a review from youtux December 10, 2019 16:08
@rbcasperson
Copy link
Author

@youtux or any contributor, definitely looking for next steps on this one. Thanks!

rbcasperson and others added 3 commits May 1, 2020 16:12
This leave description lines unchanged so that raw rST can be put in them.
It also prevent comments from being included in descriptions.
@jsa34
Copy link
Collaborator

jsa34 commented Dec 3, 2024

Superseded by #749

@jsa34 jsa34 closed this Dec 3, 2024
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.

Scenario descriptions cause odd results
6 participants