Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Automated / semi-automated python3 upgrades #5368

Merged
merged 12 commits into from
Jun 3, 2019
Merged

Automated / semi-automated python3 upgrades #5368

merged 12 commits into from
Jun 3, 2019

Conversation

asottile
Copy link
Member

@asottile asottile commented Jun 2, 2019

I split this into separate commits to make it easier to review, rebase, and replay.

@codecov
Copy link

codecov bot commented Jun 2, 2019 •
edited

Codecov Report

Merging #5368 into master will increase coverage by 4.11%.
The diff coverage is 92.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5368      +/-   ##
==========================================
+ Coverage   88.77%   92.88%   +4.11%     
==========================================
  Files         115      114       -1     
  Lines       25906    25528     -378     
  Branches     2496     2482      -14     
==========================================
+ Hits        22997    23711     +714     
+ Misses       2564     1487    -1077     
+ Partials      345      330      -15
Impacted Files Coverage Δ
testing/test_modimport.py 100% <ø> (ø) ⬆️
src/_pytest/setuponly.py 95.74% <ø> (-0.26%) ⬇️
testing/python/setup_only.py 100% <ø> (ø) ⬆️
testing/test_stepwise.py 100% <ø> (ø) ⬆️
src/_pytest/config/findpaths.py 95% <ø> (+2%) ⬆️
src/_pytest/stepwise.py 96.66% <ø> (ø) ⬆️
src/_pytest/setupplan.py 100% <ø> (ø) ⬆️
testing/freeze/tox_run.py 0% <ø> (-12.5%) ⬇️
src/_pytest/nose.py 97.05% <ø> (-0.38%) ⬇️
src/_pytest/warning_types.py 100% <ø> (ø) ⬆️
... and 138 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 b8781ff...ce78c9a. Read the comment docs.

@blueyed
Copy link
Contributor

blueyed commented Jun 2, 2019 •
edited

Why is "coding" removed again? (5e2e6b9)

@asottile
Copy link
Member Author

asottile commented Jun 2, 2019

Why is "coding" removed again? (5e2e6b9)

https://www.python.org/dev/peps/pep-3120/

nicoddemus
nicoddemus reviewed Jun 2, 2019
View reviewed changes
src/_pytest/assertion/rewrite.py Show resolved Hide resolved
setup.py
from setuptools import setup

# TODO: if py gets upgrade to >=1.6,
# remove _width_of_current_line in terminal.py
INSTALL_REQUIRES = [
"py>=1.5.0",
"six>=1.10.0",
Copy link
Member

Choose a reason for hiding this comment

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

Thanks six for your service. 🙇

Copy link
Member Author

Choose a reason for hiding this comment

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

o7

/me pours one 🍺 out

@nicoddemus
Copy link
Member

I don't understand why the coverage is reporting uncovered lines... for example, it says here:

https://codecov.io/gh/pytest-dev/pytest/compare/28ac469eaa7206a0bf1439b31ed187beb3598522...15669e81310e80018a86ed96678992e575f2f537/changes

That test_tmpdir_fixture was not executed at all, which doesn't really make sense... 🤔

@asottile
Copy link
Member Author

asottile commented Jun 3, 2019

I'm always suspicious of any data that codecov produces, in this case that test definitely ran: https://travis-ci.org/pytest-dev/pytest/jobs/540538554 🤔

@nicoddemus
Copy link
Member

Hmmm indeed, but I noticed that PYTEST_COVERAGE=1 was not set for that build. I will enable it for pure py37 runs, let's see.

@nicoddemus
Copy link
Member

Rebased on master (hope you don't mind @asottile) and fixed linting.

@asottile
Copy link
Member Author

asottile commented Jun 3, 2019

no problem, though I would have replayed the commits myself so that ended up in the right one 🤷‍♂️

@nicoddemus
Copy link
Member

OK I think we are good. Can you go ahead and merge @asottile if you are OK with the PR?

@asottile asottile merged commit 5976f36 into pytest-dev:master Jun 3, 2019
@asottile asottile deleted the pyupgrade branch June 3, 2019 15:41
blueyed
blueyed reviewed Jun 3, 2019
View reviewed changes
.travis.yml
@@ -34,7 +34,7 @@ jobs:
- test $(python -c 'import sys; print("%d%d" % sys.version_info[0:2])') = 37

# Full run of latest supported version, without xdist.
- env: TOXENV=py37
- env: TOXENV=py37 PYTEST_COVERAGE=1
Copy link
Contributor

Choose a reason for hiding this comment

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

We do not need this, do we?
We have py37-xdist with coverage already (at least).

Copy link
Member

Choose a reason for hiding this comment

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

Not sure, this fixed the coverage on the PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd guess it was the rebase that fixed it.

We can revert this and this - after things stabilized a bit.. :)

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

@blueyed blueyed blueyed left review comments

@nicoddemus nicoddemus nicoddemus approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@asottile @blueyed @nicoddemus