Skip to content

Conversation

@Zac-HD
Copy link
Member

@Zac-HD Zac-HD commented Sep 26, 2018

Closes #4034.


#: user properties is a list of tuples (name, value) that holds user
#: defined properties of the test
if user_properties is None:
Copy link
Member

Choose a reason for hiding this comment

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

I think we can make this more succinct:

user_properties = [] if user_properties is None else list(user_properties) 

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Thanks @Zac-HD, good work! 👍

@Zac-HD
Copy link
Member Author

Zac-HD commented Sep 26, 2018

Happy to help! I use pytest all the time, so it's nice to give some code back too 😄

@codecov
Copy link

codecov bot commented Sep 26, 2018

Codecov Report

Merging #4041 into master will decrease coverage by 0.12%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4041      +/-   ##
==========================================
- Coverage   94.45%   94.33%   -0.13%     
==========================================
  Files         109      109              
  Lines       23788    23792       +4     
  Branches     2357     2359       +2     
==========================================
- Hits        22470    22445      -25     
- Misses       1006     1029      +23     
- Partials      312      318       +6
Flag Coverage Δ
#doctesting 28.58% <0%> (-0.72%) ⬇️
#linux 94.33% <0%> (+0.02%) ⬆️
#nobyte 0% <ø> (ø) ⬆️
#numpy 28.2% <0%> (-0.02%) ⬇️
#pexpect 0% <ø> (ø) ⬆️
#py27 92.5% <0%> (-0.09%) ⬇️
#py34 91.97% <0%> (-0.11%) ⬇️
#py35 91.98% <0%> (-0.11%) ⬇️
#py36 92.51% <0%> (-0.15%) ⬇️
#py37 92.12% <0%> (-0.17%) ⬇️
#trial 31.08% <0%> (-0.17%) ⬇️
#windows ?
#xdist 18.51% <0%> (-0.05%) ⬇️
Impacted Files Coverage Δ
src/_pytest/reports.py 82.95% <0%> (-3.96%) ⬇️
testing/test_paths.py 86.36% <0%> (-13.64%) ⬇️
src/_pytest/paths.py 91.3% <0%> (-8.7%) ⬇️
testing/test_tmpdir.py 95.06% <0%> (-4.94%) ⬇️
src/_pytest/capture.py 86.72% <0%> (-3.21%) ⬇️
src/_pytest/nodes.py 93.69% <0%> (-0.85%) ⬇️
testing/acceptance_test.py 97.14% <0%> (-0.66%) ⬇️
src/_pytest/pytester.py 85.56% <0%> (-0.46%) ⬇️
testing/test_capture.py 98.94% <0%> (-0.31%) ⬇️
src/_pytest/fixtures.py 96.99% <0%> (-0.28%) ⬇️
... and 1 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 b1fbb2a...a089a95. Read the comment docs.

@coveralls
Copy link

coveralls commented Sep 26, 2018

Coverage Status

Coverage increased (+0.02%) to 93.811% when pulling a089a95 on Zac-HD:user-properties-type into b1fbb2a on pytest-dev:master.

@Zac-HD
Copy link
Member Author

Zac-HD commented Sep 26, 2018

@nicoddemus - are these coverage tools ever useful? Because the diff clearly doesn't change coverage at all, but one tool has it going up and the other going down!

@nicoddemus
Copy link
Member

are these coverage tools ever useful?

We are still evaluating them... can't really answer that at the moment. 🤔

@Zac-HD
Copy link
Member Author

Zac-HD commented Sep 26, 2018

Fair enough! My thoughts:

  • Coverage tools that give inconsistent results are just as bad as any other flaky test.
  • The important thing about coverage measurement is not percentage, it's "which lines are not covered?"
  • 100% is therefore the only percentage target that makes sense; perhaps for a subset of files.

@Zac-HD
Copy link
Member Author

Zac-HD commented Sep 26, 2018

@nicoddemus - Appveyor failure looks like a job failed to upload coverage data, not failing tests 😌

@nicoddemus
Copy link
Member

Yeah unfortunately this happens somewhat frequently. 😞

@nicoddemus
Copy link
Member

Fair enough! My thoughts:

Definitely, agree with all your points. If codecov keeps behaving like that, I'm afraid we will have to revert back to coveralls (this was discussed briefly in tox-dev/tox#1019 (comment) also).

@nicoddemus nicoddemus merged commit d2fc7ca into pytest-dev:master Sep 27, 2018
@Zac-HD Zac-HD deleted the user-properties-type branch September 27, 2018 11:05
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