Fix `pytest.mark.parametrize` when the argvalue is an iterator by asottile · Pull Request #5356 · pytest-dev/pytest

asottile · 2019-06-01T20:51:42Z

Resolves #5354

Sup3rGeo · 2019-06-01T21:24:50Z

Thanks @asottile ! If I understand correctly, we were consuming the generator when checking for parametrization arguments to ignore at first so when the real parametrization kicked in there was nothing left for the parameters, right?

asottile · 2019-06-01T21:26:40Z

Thanks @asottile ! If I understand correctly, we were consuming the generator when checking for parametrization arguments to ignore at first so when the real parametrization kicked in there was nothing left for the parameters, right?

that is exactly right :D

codecov · 2019-06-01T21:39:51Z

Codecov Report

Merging #5356 into master will decrease coverage by 1.11%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #5356      +/-   ##
=========================================
- Coverage   95.32%   94.2%   -1.12%     
=========================================
  Files         115     115              
  Lines       26384   26391       +7     
  Branches     2607    2607              
=========================================
- Hits        25150   24862     -288     
- Misses        924    1219     +295     
  Partials      310     310

Impacted Files	Coverage Δ
testing/test_mark.py	`99.39% <100%> (ø)`	⬆️
src/_pytest/mark/structures.py	`92.73% <100%> (+0.08%)`	⬆️
testing/python/show_fixtures_per_test.py	`17.64% <0%> (-82.36%)`	⬇️
testing/python/setup_plan.py	`20% <0%> (-80%)`	⬇️
testing/test_warnings.py	`48.63% <0%> (-47%)`	⬇️
testing/test_pytester.py	`58.3% <0%> (-30.99%)`	⬇️
testing/test_tmpdir.py	`75.8% <0%> (-23.12%)`	⬇️
testing/python/setup_only.py	`80.35% <0%> (-19.65%)`	⬇️
testing/test_pluginmanager.py	`49.57% <0%> (-4.67%)`	⬇️
src/_pytest/python.py	`89.09% <0%> (-4.25%)`	⬇️
... and 7 more

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 917195e...0654c34. Read the comment docs.

nicoddemus

Thanks @asottile!

Please open the backport PR after this gets merged. 👍

asottile · 2019-06-01T22:12:19Z

#5357

The procedure I used here was:

git checkout origin/4.6-maintenance -b backport_5356
git cherry-pick ...

blueyed · 2019-06-05T09:57:00Z

@asottile @nicoddemus
Removed "needs backport" label - I think we should do this after the backport was done, to track closed PRs that still need one.
Agreed?

RonnyPfannschmidt · 2019-06-05T10:18:37Z

this pr doenst test critical cases in, and it should fail on iterators

RonnyPfannschmidt · 2019-06-05T10:19:23Z

+            yield 2
+            yield 3
+
+        @pytest.mark.parametrize('a', gen())


this breaks down on marker/generator reuse

@RonnyPfannschmidt please consider creating a new issue - or do you know what's up with the comment, @asottile ?

I think @RonnyPfannschmidt is suggesting this won't work (not at a computer)

m = pytest.mark.parametrize('a', gen()) @m def test1(a): pass @m def test2(a): pass

But if it doesn't, it never worked so 🤷‍♂️ probably?

we already had a issue about reuse of generators

yeah that example above is indeed broken, but wasn't the regression being targetted by this fix:

$ pytest t.py -vv ============================= test session starts ============================== platform linux -- Python 3.6.7, pytest-4.5.0, py-1.8.0, pluggy-0.12.0 -- /tmp/venv/bin/python3 cachedir: .pytest_cache rootdir: /tmp plugins: celery-4.3.0 collected 4 items t.py::test1[1] PASSED [ 25%] t.py::test1[2] PASSED [ 50%] t.py::test1[3] PASSED [ 75%] t.py::test2[a0] SKIPPED [100%] ===================== 3 passed, 1 skipped in 0.01 seconds ======================

nicoddemus · 2019-06-05T12:28:23Z

Removed "needs backport" label - I think we should do this after the backport was done, to track closed PRs that still need one.
Agreed?

Agreed, I had the same idea yesterday. 👍

asottile · 2019-06-05T13:42:22Z

Let's not lose track of "did we backport this?" let's replace the label when competed with a backported label

blueyed · 2019-06-05T13:49:34Z

Ok.
Although there is / should typically be a referencing comment anyway.

We should/could also do backports in batches then - resulting in a single CI run, instead of one for every backport.

nicoddemus · 2019-06-05T15:55:48Z

Agree with having a label: it is easy to see the labels when listing the PRs. 👍

asottile mentioned this pull request Jun 1, 2019

Version 4.6.0 skips tests without apparent reason #5354

Closed

3 tasks

nicoddemus mentioned this pull request Jun 1, 2019

Technical aspects related to Python 2.7 and 3.4 support (pytest 5.0) #5275

Closed

nicoddemus added the needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch label Jun 1, 2019

nicoddemus reviewed Jun 1, 2019

View reviewed changes

Comment thread testing/test_mark.py

nicoddemus approved these changes Jun 1, 2019

View reviewed changes

Fix pytest.mark.parametrize when the argvalue is an iterator

cafb13c

asottile merged commit e4fe41e into pytest-dev:master Jun 1, 2019

asottile deleted the fix_parametrize_iterator branch June 1, 2019 22:09

asottile mentioned this pull request Jun 1, 2019

[4.6] Fix pytest.mark.parametrize when the argvalue is an iterator #5357

Merged

blueyed removed the needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch label Jun 5, 2019

RonnyPfannschmidt reviewed Jun 5, 2019

View reviewed changes

asottile added the backported label Jun 5, 2019

Uh oh!

Conversation

asottile commented Jun 1, 2019

Uh oh!

Sup3rGeo commented Jun 1, 2019

Uh oh!

asottile commented Jun 1, 2019

Uh oh!

codecov Bot commented Jun 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

nicoddemus left a comment

Choose a reason for hiding this comment

Uh oh!

asottile commented Jun 1, 2019

Uh oh!

blueyed commented Jun 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RonnyPfannschmidt commented Jun 5, 2019

Uh oh!

RonnyPfannschmidt Jun 5, 2019

Choose a reason for hiding this comment

Uh oh!

blueyed Jun 5, 2019

Choose a reason for hiding this comment

Uh oh!

asottile Jun 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asottile Jun 5, 2019

Choose a reason for hiding this comment

Uh oh!

RonnyPfannschmidt Jun 5, 2019

Choose a reason for hiding this comment

Uh oh!

asottile Jun 5, 2019

Choose a reason for hiding this comment

Uh oh!

nicoddemus commented Jun 5, 2019

Uh oh!

asottile commented Jun 5, 2019

Uh oh!

blueyed commented Jun 5, 2019

Uh oh!

nicoddemus commented Jun 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented Jun 1, 2019 •

edited

Loading

blueyed commented Jun 5, 2019 •

edited

Loading

asottile Jun 5, 2019 •

edited

Loading