Skip to content

Conversation

@karlkovaciny
Copy link

I was playing with the tutorial at https://smarie.github.io/python-pytest-steps/#c-optional-steps-and-dependencies and got this error.

I had indented badly and my code looked like this:
with optional_step('step_b') as step_b:
assert True
yield step_b # should have been unindented, caused this error.

I just wanted to help any other newbie get past this in case it happened to them.

I was playing with the tutorial at https://smarie.github.io/python-pytest-steps/#c-optional-steps-and-dependencies and got this error.

I had indented badly and my code looked like this: 
    with optional_step('step_b') as step_b:
        assert True
        yield step_b # should have been unindented, caused this error.

I just wanted to help any other newbie get past this in case it happened to them.
@codecov-io
Copy link

codecov-io commented Nov 4, 2019

Codecov Report

Merging #35 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #35   +/-   ##
=======================================
  Coverage   83.52%   83.52%           
=======================================
  Files          27       27           
  Lines        1153     1153           
=======================================
  Hits          963      963           
  Misses        190      190
Impacted Files Coverage Δ
pytest_steps/steps_generator.py 79.55% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 575028b...00d708b. Read the comment docs.

if res.exec_result is None:
raise ValueError("Internal error: this should not happen")
raise ValueError("Internal error: this should not happen."
"Did you ``yield step_b`` inside the context manager instead of after it?")
Copy link
Owner

@smarie smarie Nov 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great suggestion. We just need to use the actual step name rather than "step_b", I guess this should do the trick, can you check ?

Suggested change
"Did you ``yield step_b`` inside the context manager instead of after it?")
"Did you ``yield %s`` inside the context manager instead of after it?" % optional_step.step_name)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this time optional_step is an instance of type, so you need to read res.step_name instead.

Suggested change
"Did you ``yield step_b`` inside the context manager instead of after it?")
raise ValueError("Internal error: this should not happen.\n"
"Did you ``yield %s`` inside the context manager instead of after it?" % res.step_name)

Here is a case that reproduces the error, do you want me to turn it into a test or something?

from pytest_steps import test_steps, optional_step

@test_steps('step_a')
def test_suite_opt():

    # Step A
    with optional_step('step_a') as step_a:
        assert True
        yield step_a   

By the way, your efforts at documentation, code clarity, and response to pull requests are all just exemplary. Thanks a lot for supporting this.

Copy link
Owner

@smarie smarie Nov 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops thanks for catching my mistake, I edited your PR too fast. Of course your suggestion is the correct one.

Concerning the test: I actually hesitated to ask you this ! but since you're proposing, yes do not hesitate to add such a test, in a separate test file under test/ (and not test_raw/) folder.

But I can also do it once merged based on your example, so do not feel forced, up to you.

Oh and thanks so much for the kind words ! I really appreciate :)

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.52%. Comparing base (575028b) to head (00d708b).
⚠️ Report is 38 commits behind head on main.

Files with missing lines Patch % Lines
pytest_steps/steps_generator.py 0.00% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@           Coverage Diff           @@
##             main      #35   +/-   ##
=======================================
  Coverage   83.52%   83.52%           
=======================================
  Files          27       27           
  Lines        1153     1153           
=======================================
  Hits          963      963           
  Misses        190      190           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

4 participants