Skip to content

getfixturevalue does not correctly declare dependency with the calling fixture#4860

Merged
nicoddemus merged 5 commits into
pytest-dev:featuresfrom
nicoddemus:getfixturevalue-cleanup-1895
Mar 3, 2019
Merged

getfixturevalue does not correctly declare dependency with the calling fixture#4860
nicoddemus merged 5 commits into
pytest-dev:featuresfrom
nicoddemus:getfixturevalue-cleanup-1895

Conversation

@nicoddemus

@nicoddemus nicoddemus commented Mar 1, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

Fix #1895

@codecov

codecov Bot commented Mar 1, 2019 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #4860 into features will decrease coverage by 0.1%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##           features    #4860      +/-   ##
============================================
- Coverage     95.76%   95.66%   -0.11%     
============================================
  Files           113      113              
  Lines         25744    25257     -487     
  Branches       2508     2504       -4     
============================================
- Hits          24655    24162     -493     
- Misses          773      775       +2     
- Partials        316      320       +4
Flag Coverage Δ
#linux 95.48% <100%> (-0.11%) ⬇️
#osx 93.32% <100%> (ø) ⬆️
#windows 93.6% <100%> (ø) ⬆️
Impacted Files Coverage Δ
src/_pytest/fixtures.py 97.93% <100%> (-0.3%) ⬇️
testing/python/fixtures.py 99.08% <100%> (ø)
testing/test_modimport.py 81.81% <0%> (-1.52%) ⬇️
testing/code/test_source.py 95.53% <0%> (-1.07%) ⬇️
testing/test_argcomplete.py 68.11% <0%> (-0.9%) ⬇️
testing/test_compat.py 91.13% <0%> (-0.63%) ⬇️
testing/test_skipping.py 97.65% <0%> (-0.57%) ⬇️
src/_pytest/warnings.py 88.88% <0%> (-0.53%) ⬇️
src/_pytest/debugging.py 79.76% <0%> (-0.35%) ⬇️
... and 75 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 e1f97e4...c334adc. Read the comment docs.

@nicoddemus nicoddemus force-pushed the getfixturevalue-cleanup-1895 branch from ac85d46 to d97473e Compare March 2, 2019 12:54
@nicoddemus nicoddemus changed the title WIP: getfixturevalue does not correctly declare dependency with the calling fixture getfixturevalue does not correctly declare dependency with the calling fixture Mar 2, 2019
@nicoddemus

nicoddemus commented Mar 2, 2019

Copy link
Copy Markdown
Member Author

Ready for review

@nicoddemus

nicoddemus commented Mar 2, 2019

Copy link
Copy Markdown
Member Author

This is related to #4871. Any chance @blueyed and/or @asottile could take a look?

asottile
asottile approved these changes Mar 3, 2019
View reviewed changes

@asottile asottile left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

blueyed
blueyed suggested changes Mar 3, 2019
View reviewed changes

@blueyed blueyed left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Found some typos, and it should be against master, no?

Comment thread testing/python/fixtures.py Outdated
Comment thread testing/python/fixtures.py Outdated
Comment thread testing/python/fixtures.py Outdated
@blueyed @nicoddemus
Apply suggestions from code review
c334adc 
Co-Authored-By: nicoddemus <nicoddemus@gmail.com>
@nicoddemus

nicoddemus commented Mar 3, 2019

Copy link
Copy Markdown
Member Author

it should be against master, no?

While it is a bug fix, it does change the teardown order of fixtures, so I want to play safe here and release this on 4.4. 👍

@nicoddemus nicoddemus merged commit 23ea04f into pytest-dev:features Mar 3, 2019
@nicoddemus nicoddemus deleted the getfixturevalue-cleanup-1895 branch March 3, 2019 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asottile asottile asottile approved these changes

+1 more reviewer

@blueyed blueyed blueyed approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nicoddemus @blueyed @asottile