WIP: fix #4507 and ensure our modules have no warnings for general import #4510

RonnyPfannschmidt · 2018-12-05T14:06:53Z

…al import

codecov · 2018-12-05T15:01:28Z

Codecov Report

Merging #4510 into master will decrease coverage by 3.02%.
The diff coverage is 95%.

@@            Coverage Diff             @@
##           master    #4510      +/-   ##
==========================================
- Coverage   96.16%   93.14%   -3.03%     
==========================================
  Files         117      118       +1     
  Lines       25864    25906      +42     
  Branches     2498     2500       +2     
==========================================
- Hits        24873    24129     -744     
- Misses        690     1437     +747     
- Partials      301      340      +39

Impacted Files	Coverage Δ
src/_pytest/config/__init__.py	`94.13% <100%> (-0.49%)`	⬇️
src/_pytest/helpconfig.py	`97.58% <100%> (+0.05%)`	⬆️
src/_pytest/terminal.py	`90.39% <100%> (-2.64%)`	⬇️
src/_pytest/tmpdir.py	`100% <100%> (ø)`	⬆️
src/_pytest/setuponly.py	`95.74% <100%> (ø)`	⬆️
src/_pytest/doctest.py	`93.7% <100%> (-2.93%)`	⬇️
src/_pytest/pastebin.py	`87.71% <100%> (ø)`	⬆️
src/_pytest/setupplan.py	`100% <100%> (ø)`	⬆️
src/_pytest/junitxml.py	`95.82% <100%> (ø)`	⬆️
src/_pytest/stepwise.py	`96.55% <100%> (ø)`	⬆️
... and 46 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 5ecac3c...2d1d313. Read the comment docs.

johnthagen · 2018-12-05T15:02:47Z

@RonnyPfannschmidt Will this also solve #1403?

RonnyPfannschmidt · 2018-12-05T15:08:41Z

@johnthagen correct, its one of the warnings i sorted out

RonnyPfannschmidt · 2018-12-05T15:09:52Z

@johnthagen wait, actually it isnt, i only contained that warning to the assertion module

we will leave it in place until we drop python 2.7 support in the mainline as the implementation effort is a mess otherwise

asottile · 2018-12-05T16:42:27Z

testing/test_own_modules.py

+
+
+def _get_modules():
+    for module in vars(_pytest).values():


Could use pkgutil: https://github.com/asottile/git-code-debt/blob/v0.10.0/git_code_debt/util/discovery.py#L18

johnthagen · 2018-12-05T17:16:08Z

@RonnyPfannschmidt Do you think we could fix #1403 by using a warning filter?

with warnings.catch_warnings():
    warnings.filterwarnings('ignore', category=DeprecationWarning)
    import imp

asottile · 2018-12-05T17:33:07Z

I think the actual fix here needs to be to factor out usage of imp in python 3.x

Looks like we have a few usages, all in the assertion module:

$ grep 'imp\.' -- !$
grep 'imp\.' -- src/_pytest/assertion/rewrite.py
    PYTEST_TAG = imp.get_tag() + "-PYTEST"
        return imp.find_module(name, path)
            if tp == imp.PY_COMPILED:
                        fn = imp.source_from_cache(fn)
            elif tp != imp.PY_SOURCE:
        shown that the call to imp.find_module (inside of the find_module
            # I wish I could just call imp.load_compiled here, but __file__ has to
            mod = sys.modules[name] = imp.new_module(name)
        return tp == imp.PKG_DIRECTORY
    # sometime to be able to use imp.load_compiled to load them. (See
            fp.write(imp.get_magic())
            or data[:4] != imp.get_magic()

johnthagen · 2018-12-05T17:46:16Z

Python 2.7 has a small compatibility version of importlib. Perhaps that can help?

asottile · 2018-12-05T17:49:59Z

Python 2.7 has a small compatibility version of importlib. Perhaps that can help?

unfortunately it does not, pytest does code rewriting and builds modules + pyc files which is very much out of the scope of importlib.import_module (which, isn't actually a super helpful function given __import__ is a builtin!)

RonnyPfannschmidt · 2018-12-05T17:51:23Z

importlib2 might be applicable

asottile · 2018-12-05T18:14:59Z

Seems "use at your own risk beta" -- maybe not the best choice here?

johnthagen · 2018-12-05T21:51:33Z

Yeah, looks like importlib2 hasn't been updated since 2014, which probably isn't a good sign.

Zac-HD · 2018-12-07T06:20:48Z

@RonnyPfannschmidt - can you paste Closes #1403 and closes #4507 into the top comment, to automatically close those issues when this PR is merged?

It's also a machine-readable link between issues and related PRs that e.g. the refined github extension can pick up on, which means I can see whether a given issue has a currently-open PR that will resolve it 😄

Not a big deal, but I've found (and manually closed) a few orphan issues which stayed open long after the fixing PR was merged so we might as well use the tools to avoid that!

RonnyPfannschmidt · 2019-01-23T17:36:18Z

pushed a merge will have to amend it later on

RonnyPfannschmidt · 2019-01-24T09:01:59Z

@asottile the usage of distutils.spawn has to go - its importing imp

asottile · 2019-01-24T09:28:00Z

@asottile the usage of distutils.spawn has to go - its importing imp

you sure?

$ python3.7 -Werror -c 'import distutils.spawn'
$

asottile · 2019-01-24T09:30:17Z

yeah definitely not:

$ find /usr/lib/python3.6/distutils/ -name "*.py" | xargs grep -E '(from|import) imp'
/usr/lib/python3.6/distutils/util.py:import importlib.util
/usr/lib/python3.6/distutils/command/install_lib.py:import importlib.util
/usr/lib/python3.6/distutils/command/build_py.py:import importlib.util

RonnyPfannschmidt · 2019-01-24T10:02:01Z

@asottile https://travis-ci.org/pytest-dev/pytest/jobs/483565824#L313-L321 - definitively sure

asottile · 2019-01-24T10:17:25Z

that's virtualenv's fault -- not stdlib: pypa/virtualenv#955

asottile · 2019-01-24T10:18:29Z

err -- pypa/virtualenv#555

looks like it's been fixed, upgrade virtualenv and that should poof

blueyed · 2019-05-30T18:50:48Z

testing/test_own_modules.py

+        # virtualenv bug
+        "-W", "ignore:the imp:DeprecationWarning:distutils",
+        "-c", "import " + module_name,
+    ])


Can this be combinded with the (very slow) testing/test_modimport.py at least?
I would also like to run these kind of tests in a single CI job only.

Also needs # fmt: on again.

gaborbernat · 2019-06-19T14:38:50Z

@asottile @RonnyPfannschmidt any updates on this? does this requires a restart?

RonnyPfannschmidt · 2019-06-19T14:42:57Z

i lost track of it, im currently still on my hiatus, but i'l take a look at making this work quickly this week

…al import

RonnyPfannschmidt · 2019-06-20T21:47:15Z

i got started on resolving the rebase artifacts, but right now its unlikely to finish before sometime next week

gaborbernat · 2019-06-20T21:48:03Z

Thanks for the update!

RonnyPfannschmidt · 2019-06-20T21:54:16Z

main reason being that i currently do only public opensouce to do a bit of unpacking while the preps for the marriage on saturday are running ^^

nicoddemus · 2019-06-22T00:08:06Z

Gratz!

RonnyPfannschmidt · 2019-08-08T18:49:27Z

i started picking this one up again, i merged around a bit and will squash around soonish to deal with the fallout of this stuff

asottile · 2019-08-08T20:35:32Z

src/_pytest/capture.py

@@ -10,7 +10,10 @@
 from io import UnsupportedOperation
 from tempfile import TemporaryFile

-import pytest
+from .config import hookimpl


curious why the imports changed, maybe I missed this

Removes the import cycle to pytest

Else we can't import parts of pytest without breaking

can you show me what you mean -- like stacktrace or reproduction or whatnot, I'm not sure I understand (and I don't understand why it's necessary for this PR)

Pytest imports from all over the place this preveting isolation, if everything triggers every import then I can't let it effectively tell me what is going on

Last stack trace I had if this disappeared a few months ago in a terminal, perhaps I could recreate it when I'm back on that computer

johnthagen · 2019-10-01T16:06:47Z

I think a new warning has popped up today from attr (#5901):

  File "/home/gitlab-runner/builds/DwvLisya/0/.tox/py36/lib/python3.6/site-packages/_pytest/mark/structures.py", line 370, in <module>
    @attr.s(cmp=False, hash=False)
  File "/home/gitlab-runner/builds/DwvLisya/0/.tox/py36/lib/python3.6/site-packages/attr/_make.py", line 944, in attrs
    eq, order = _determine_eq_order(cmp, eq, order)
  File "/home/gitlab-runner/builds/DwvLisya/0/.tox/py36/lib/python3.6/site-packages/attr/_make.py", line 750, in _determine_eq_order
    warnings.warn(_CMP_DEPRECATION, DeprecationWarning, stacklevel=3)
DeprecationWarning: The usage of `cmp` is deprecated and will be removed on or after 2021-06-01.  Please use `eq` and `order` instead.

RonnyPfannschmidt · 2019-10-02T04:40:22Z

@asottile the communication around the super-seed leaves me personally with a bad after-taste, i'll reflect on that

asottile · 2019-10-02T04:49:46Z

@asottile the communication around the super-seed leaves me personally with a bad after-taste, i'll reflect on that

ah I mostly wanted to get this going again -- and wanted to see if I could find the import error -- no harm intended :(

testing/test_meta.py ensures this already as a side effect (+ tests a few more (`__init__.py` files) and should have been combined with it right away [1]. 1: pytest-dev#4510 (comment) Ref: pytest-dev@eaa05531e Ref: pytest-dev@4d31ea831

fix pytest-dev#4507 and ensure our modules have no warnigns for gener…

9342778

…al import

RonnyPfannschmidt requested review from nicoddemus and asottile December 5, 2018 14:06

RonnyPfannschmidt added 3 commits December 5, 2018 15:09

add missed file

ae3e529

CHANGELOG

4621e28

fix references used in pytester

892e10b

RonnyPfannschmidt mentioned this pull request Dec 5, 2018

DeprecationWarning triggered in _pytest.tmpdir use of attr #4507

Closed

add some specific warning filters for false positives

b17ad82

asottile reviewed Dec 5, 2018

View reviewed changes

merge master

9dd0cf6

fix merge misstake, return record_xml_attribute

1698166

blueyed reviewed May 30, 2019

View reviewed changes

RonnyPfannschmidt added 6 commits June 20, 2019 07:12

fix pytest-dev#4507 and ensure our modules have no warnigns for gener…

e1e3e35

…al import

fix references used in pytester

19fdbee

add some specific warning filters for false positives

0eb6b87

fix merge misstake, return record_xml_attribute

12f85a2

silence lsof and account for virtualenv bug

388357d

assertion only warns about imp on python3

61ef3e6

RonnyPfannschmidt force-pushed the fix-4507 branch from 51b40a3 to 61ef3e6 Compare June 20, 2019 05:14

RonnyPfannschmidt added 2 commits August 8, 2019 20:38

commit broken merge in prep for more

765a476

Merge branch 'master' into fix-4507

ea162a9

RonnyPfannschmidt added 4 commits August 8, 2019 21:02

more code updates

5113afb

sync junitxml with master

d96d573

assertion no longer uses imp

bb706aa

type annotate KNOWN BAD

2d1d313

asottile reviewed Aug 8, 2019

View reviewed changes

johnthagen mentioned this pull request Oct 1, 2019

Warning triggered from attr 19.2.0 cmp usage #5901

Closed

asottile mentioned this pull request Oct 1, 2019

Add test to ensure _pytest is warning-clean on import #5904

Merged

RonnyPfannschmidt closed this Oct 2, 2019

blueyed mentioned this pull request Feb 3, 2020

Remove testing/test_modimport.py #6666

Merged

WIP: fix #4507 and ensure our modules have no warnings for general import #4510

WIP: fix #4507 and ensure our modules have no warnings for general import #4510

Conversation

RonnyPfannschmidt commented Dec 5, 2018 • edited Loading

codecov bot commented Dec 5, 2018 • edited Loading

Codecov Report

johnthagen commented Dec 5, 2018

RonnyPfannschmidt commented Dec 5, 2018

RonnyPfannschmidt commented Dec 5, 2018

Choose a reason for hiding this comment

johnthagen commented Dec 5, 2018 • edited Loading

asottile commented Dec 5, 2018

johnthagen commented Dec 5, 2018

asottile commented Dec 5, 2018

RonnyPfannschmidt commented Dec 5, 2018

asottile commented Dec 5, 2018

johnthagen commented Dec 5, 2018

Zac-HD commented Dec 7, 2018

RonnyPfannschmidt commented Jan 23, 2019

RonnyPfannschmidt commented Jan 24, 2019

asottile commented Jan 24, 2019

asottile commented Jan 24, 2019

RonnyPfannschmidt commented Jan 24, 2019

asottile commented Jan 24, 2019

asottile commented Jan 24, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaborbernat commented Jun 19, 2019

RonnyPfannschmidt commented Jun 19, 2019

RonnyPfannschmidt commented Jun 20, 2019

gaborbernat commented Jun 20, 2019

RonnyPfannschmidt commented Jun 20, 2019

nicoddemus commented Jun 22, 2019

RonnyPfannschmidt commented Aug 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnthagen commented Oct 1, 2019 • edited Loading

RonnyPfannschmidt commented Oct 2, 2019

asottile commented Oct 2, 2019

RonnyPfannschmidt commented Dec 5, 2018 •

edited

Loading

codecov bot commented Dec 5, 2018 •

edited

Loading

johnthagen commented Dec 5, 2018 •

edited

Loading

johnthagen commented Oct 1, 2019 •

edited

Loading