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

improve line width estimate #3911

Merged
merged 5 commits into from Sep 1, 2018

Conversation

Projects
None yet
5 participants
@wimglenn
Contributor

wimglenn commented Aug 30, 2018

as discussed in pytest-dev/py#197

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Aug 30, 2018

Coverage Status

Coverage increased (+0.01%) to 94.061% when pulling 19fa01b on wimglenn:i18n_width into 9bd4b0a on pytest-dev:master.

coveralls commented Aug 30, 2018

Coverage Status

Coverage increased (+0.01%) to 94.061% when pulling 19fa01b on wimglenn:i18n_width into 9bd4b0a on pytest-dev:master.

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Aug 30, 2018

Codecov Report

Merging #3911 into master will decrease coverage by 0.01%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3911      +/-   ##
==========================================
- Coverage   96.24%   96.23%   -0.02%     
==========================================
  Files         108      108              
  Lines       23353    23360       +7     
==========================================
+ Hits        22476    22480       +4     
- Misses        877      880       +3
Flag Coverage Δ
#doctesting 31.94% <80%> (+0.07%) ⬆️
#nobyte 93.49% <80%> (-0.03%) ⬇️
#numpy 31.5% <80%> (+0.07%) ⬆️
#pexpect 53.58% <60%> (ø) ⬆️
#pluggymaster 95.08% <80%> (+0.01%) ⬆️
#py27 94.61% <80%> (-0.01%) ⬇️
#py34 93.89% <80%> (+0.01%) ⬆️
#py35 93.9% <80%> (+0.01%) ⬆️
#py36 94.81% <80%> (-0.02%) ⬇️
#py37 94.06% <80%> (+0.01%) ⬆️
#trial 34.17% <80%> (+0.07%) ⬆️
#xdist 94.63% <80%> (-0.02%) ⬇️
Impacted Files Coverage Δ
src/_pytest/terminal.py 95.59% <80%> (-0.32%) ⬇️
src/_pytest/capture.py 93.36% <0%> (-0.23%) ⬇️

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 9bd4b0a...19fa01b. Read the comment docs.

codecov bot commented Aug 30, 2018

Codecov Report

Merging #3911 into master will decrease coverage by 0.01%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3911      +/-   ##
==========================================
- Coverage   96.24%   96.23%   -0.02%     
==========================================
  Files         108      108              
  Lines       23353    23360       +7     
==========================================
+ Hits        22476    22480       +4     
- Misses        877      880       +3
Flag Coverage Δ
#doctesting 31.94% <80%> (+0.07%) ⬆️
#nobyte 93.49% <80%> (-0.03%) ⬇️
#numpy 31.5% <80%> (+0.07%) ⬆️
#pexpect 53.58% <60%> (ø) ⬆️
#pluggymaster 95.08% <80%> (+0.01%) ⬆️
#py27 94.61% <80%> (-0.01%) ⬇️
#py34 93.89% <80%> (+0.01%) ⬆️
#py35 93.9% <80%> (+0.01%) ⬆️
#py36 94.81% <80%> (-0.02%) ⬇️
#py37 94.06% <80%> (+0.01%) ⬆️
#trial 34.17% <80%> (+0.07%) ⬆️
#xdist 94.63% <80%> (-0.02%) ⬇️
Impacted Files Coverage Δ
src/_pytest/terminal.py 95.59% <80%> (-0.32%) ⬇️
src/_pytest/capture.py 93.36% <0%> (-0.23%) ⬇️

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 9bd4b0a...19fa01b. Read the comment docs.

Show outdated Hide outdated setup.py
@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Aug 30, 2018

Member

Thanks a lot @wimglenn! Please see my comments and let me know what you think.

Member

nicoddemus commented Aug 30, 2018

Thanks a lot @wimglenn! Please see my comments and let me know what you think.

wimglenn added some commits Aug 30, 2018

@wimglenn

This comment has been minimized.

Show comment
Hide comment
@wimglenn

wimglenn Aug 31, 2018

Contributor

@nicoddemus Oh, yep, good idea (also has the advantage for users to simply bump down py version if this change has teething problems on some platforms). Addressed in c18a5b5

Contributor

wimglenn commented Aug 31, 2018

@nicoddemus Oh, yep, good idea (also has the advantage for users to simply bump down py version if this change has teething problems on some platforms). Addressed in c18a5b5

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Aug 31, 2018

Member

Thanks @wimglenn! Took the liberty of moving the code to a function and adding a note to setup.py to remove that function once we require py>=1.6 (probably we should do so in features after this gets merged).

Member

nicoddemus commented Aug 31, 2018

Thanks @wimglenn! Took the liberty of moving the code to a function and adding a note to setup.py to remove that function once we require py>=1.6 (probably we should do so in features after this gets merged).

@wimglenn

This comment has been minimized.

Show comment
Hide comment
@wimglenn

wimglenn Aug 31, 2018

Contributor

@nicoddemus is that CI fail due the refactor? or is the codecov diff a new addition?

Contributor

wimglenn commented Aug 31, 2018

@nicoddemus is that CI fail due the refactor? or is the codecov diff a new addition?

@blueyed

This comment has been minimized.

Show comment
Hide comment
@blueyed

blueyed Aug 31, 2018

Contributor

or is the codecov diff a new addition?

This.
It seems the diff is not covered fully - but unfortunately the Details view does not load for me currently (https://codecov.io/gh/pytest-dev/pytest/compare/9bd4b0a05ee18e80e4835cd8d42cef1806b9f0f2...19fa01b91dc5289c82cc2c7dfd971eebc39cf28a).

Contributor

blueyed commented Aug 31, 2018

or is the codecov diff a new addition?

This.
It seems the diff is not covered fully - but unfortunately the Details view does not load for me currently (https://codecov.io/gh/pytest-dev/pytest/compare/9bd4b0a05ee18e80e4835cd8d42cef1806b9f0f2...19fa01b91dc5289c82cc2c7dfd971eebc39cf28a).

@blueyed

This comment has been minimized.

Show comment
Hide comment
@blueyed

blueyed Aug 31, 2018

Contributor

Maybe it is the code in setup.py, and we might want to either cover it, or ignore it if it is not possible easily.

Contributor

blueyed commented Aug 31, 2018

Maybe it is the code in setup.py, and we might want to either cover it, or ignore it if it is not possible easily.

@blueyed

This comment has been minimized.

Show comment
Hide comment
@blueyed

blueyed Aug 31, 2018

Contributor

Processing seems to hang on codecov, but the latest commit shows that it is due to not covering all py variants apparently: https://codecov.io/gh/pytest-dev/pytest/commit/c18a5b5179f724e7046742bae60da51dc74469d4

Contributor

blueyed commented Aug 31, 2018

Processing seems to hang on codecov, but the latest commit shows that it is due to not covering all py variants apparently: https://codecov.io/gh/pytest-dev/pytest/commit/c18a5b5179f724e7046742bae60da51dc74469d4

@wimglenn

This comment has been minimized.

Show comment
Hide comment
@wimglenn

wimglenn Aug 31, 2018

Contributor

Ugh. It will be expensive to test both those branches, because it requires both an old and a new version of py installed in test matrix (...or some pretty intrusive mocking)

Contributor

wimglenn commented Aug 31, 2018

Ugh. It will be expensive to test both those branches, because it requires both an old and a new version of py installed in test matrix (...or some pretty intrusive mocking)

@blueyed

This comment has been minimized.

Show comment
Hide comment
@blueyed

blueyed Aug 31, 2018

Contributor

@wimglenn
I think we can ignore this - it can be merged still/nonetheless.

Contributor

blueyed commented Aug 31, 2018

@wimglenn
I think we can ignore this - it can be merged still/nonetheless.

@nicoddemus nicoddemus merged commit 8d8e68c into pytest-dev:master Sep 1, 2018

2 of 4 checks passed

codecov/patch 80% of diff hit (target 96.24%)
Details
codecov/project 96.23% (-0.02%) compared to 9bd4b0a
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Sep 1, 2018

Member

Thanks again @wimglenn for great work on the PR!

Thanks @blueyed for chipping in as well.

Member

nicoddemus commented Sep 1, 2018

Thanks again @wimglenn for great work on the PR!

Thanks @blueyed for chipping in as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment