Use new raise syntax in one case #5481
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5481 +/- ##
=========================================
+ Coverage 95.86% 96.07% +0.2%
=========================================
Files 115 115
Lines 25458 25455 -3
Branches 2468 2467 -1
=========================================
+ Hits 24405 24455 +50
+ Misses 746 697 -49
+ Partials 307 303 -4
Continue to review full report at Codecov.
|
|
This PR has introduced regression in astropy testing, and now we see errors like: example file: https://github.com/astropy/astropy/blob/master/astropy/io/misc/asdf/tags/coordinates/tests/test_earthlocation.py I've double checked and reverting this change on top of current pytest master everything works as it used to. I would be also interested in suggested ways to go around this issue, but I got stack as it seems that the |
|
Does it work with |
|
Indeed, it does. Thanks for the quick diagnostic. |
|
@bsipocz just to check: reverting the change causes the whole module to be skipped, is that it? |
|
yes, indeed the module is not listed in the collections list, which is unfortunate. Though having the error means that nothing is collected either. |
|
I'm baffled, I get the correct behavior on Windows: 🤔 |
|
@bsipocz @nicoddemus |
|
I've tested with both 3.6 and 3.7 (based that @bsipocz is using 3.7 from the traceback) |
|
Here is a full header for the run with the patched pytest: |
|
And this is the failing one: |
|
@bsipocz Could you please also post the full output of your failed run? Thanks! My header: I will clock out now, I need to unwind a bit before bed. I will continue this tomorrow. 👍 Thanks for the report @bsipocz, and sorry for the trouble! |
|
Thanks for looking into this. And no trouble at all, we have other issues with the new release, too fixes are needed on our side, too. :) |
|
weird, works fine for me: |
|
The only way I can reproduce this is if I have a busted local copy of $ pytest astropy/io/misc/asdf/tags/coordinates/tests/test_earthlocation.py -rs
============================= test session starts ==============================
platform linux -- Python 3.6.8, pytest-5.0.0, py-1.8.0, pluggy-0.12.0
Running tests with Astropy version 4.0.dev1.
Running tests in astropy/io/misc/asdf/tags/coordinates/tests/test_earthlocation.py.
Date: 2019-07-02T22:57:07
Platform: Linux-4.15.0-54-generic-x86_64-with-Ubuntu-18.04-bionic
Executable: /tmp/astropy2/venv/bin/python3
Full Python Version:
3.6.8 (default, Jan 14 2019, 11:02:34)
[GCC 8.0.1 20180414 (experimental) [trunk revision 259383]]
encodings: sys: utf-8, locale: UTF-8, filesystem: utf-8
byteorder: little
float info: dig: 15, mant_dig: 15
Numpy: 1.16.4
Scipy: not available
Matplotlib: not available
h5py: not available
Pandas: not available
Cython: not available
Scikit-image: not available
astropy_helpers: 3.2.1
rootdir: /tmp/astropy2, inifile: setup.cfg
collected 0 items / 1 errors
==================================== ERRORS ====================================
_ ERROR collecting astropy/io/misc/asdf/tags/coordinates/tests/test_earthlocation.py _
ImportError while importing test module '/tmp/astropy2/astropy/io/misc/asdf/tags/coordinates/tests/test_earthlocation.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
astropy/io/misc/asdf/tags/coordinates/tests/test_earthlocation.py:7: in <module>
from asdf.tests.helpers import assert_roundtrip_tree
E ModuleNotFoundError: No module named 'asdf.tests.helpers'
=============================== warnings summary ===============================
venv/lib/python3.6/site-packages/_pytest/config/__init__.py:513
/tmp/astropy2/venv/lib/python3.6/site-packages/_pytest/config/__init__.py:513: PytestAssertRewriteWarning: Module already imported so cannot be rewritten: astropy.tests.plugins.display
self.import_plugin(import_spec)
-- Docs: https://docs.pytest.org/en/latest/warnings.html
!!!!!!!!!!!!!!!!!!! Interrupted: 1 errors during collection !!!!!!!!!!!!!!!!!!!!
===================== 1 warnings, 1 error in 0.18 seconds ======================@bsipocz could you also provide |
|
(and even then, changing the |
Yeah - I would not really have expected it to make a difference either. @bsipocz |
Yeah, looking at the code I can't figure out how that change could have affected |
|
This is now getting very weird as I cannot reproduce the original issue any more (with pytest v5.0.0) after making sure the repo is git cleaned. |
What do you mean? Can you also please post the full output you are getting? |
|
Let me open a PR in astropy to remove the version limitations, so we'll have clean VMs and multiple setups as now I don't trust my local any more. |
|
OK, so here is a run with It would be useful if the |
|
ok but that's not a regression, and certainly not caused by this 😆 I believe the reason they don't show up is they're excluded at discovery time instead of at runtime -- there's no "test" to skip (because the whole module is skipped) |
|
Yes, the regression I run into was something weird stuff locally and can't reproduce now (while it was totally consistent yesterday.). I'm sorry for the noise and false alarm. Switching to use pytest 5 at the end wasn't as painful as we first thought 😅 |
|
🎉 🎉 🎉 it might be possible to include skipped tests in the output one way or another -- perhaps open a separate issue for that? |
No problem at all, glad it was not a real problem in the end! 😁
Great to hear. Indeed this release should not cause big surprises other than dropping py27/py34 and deprecated features. 👍 |
|
Originally we had 90+ failures, but at the end all distilled down to the changes due to #5412. |
No description provided.