DM-11649: Enable flake8 and fix warnings from tests #40
Conversation
python/lsst/utils/tests.py
Outdated
| @@ -426,7 +426,7 @@ def inTestCase(func): | |||
| @inTestCase | |||
| def assertRaisesLsstCpp(testcase, excClass, callableObj, *args, **kwargs): | |||
| warnings.warn("assertRaisesLsstCpp is deprecated; please just use TestCase.assertRaises", | |||
| DeprecationWarning) | |||
| DeprecationWarning, 2) | |||
There was a problem hiding this comment.
Please use the named argument stacklevel for clarity., e.g. ...DeprecationWarning, stacklevel=2)
|
|
||
| [tool:pytest] | ||
| addopts = --flake8 | ||
| flake8-ignore = E133 E226 E228 N802 N803 |
There was a problem hiding this comment.
It's a pity this needs to be repeated, but at least it's nearby in the same file. I suggest adding a comment to the copy above saying "duplicated below", so changes are more likely to be propagated
There was a problem hiding this comment.
Yes, although in meas_base that helps a lot because you can get some errors masked per-file in pytest but have them still show up when you run flake8 manually.
tests/test_backtrace.py
Outdated
|
|
||
| class BacktraceTestCase(lsst.utils.tests.TestCase): | ||
| def setUp(self): | ||
| pass | ||
|
|
||
| def test_segfault(self): | ||
| if backtrace.isEnabled(): | ||
| pipe = subprocess.Popen((sys.executable, "tests/backtrace.py"), | ||
| output = subprocess.run([sys.executable, os.path.join(ROOT, "backtrace.py")], | ||
| stderr=subprocess.PIPE, |
There was a problem hiding this comment.
Thi is a nice change, but the Python 3 subprocess docs claim run was added in Python 3.5, and the 2.7 version of the docs doesn't list it. Unless future provides it, I don't think you can use it yet. If I'm right then consider a TODO note with the improved code for when we drop python 2.7 support.
There was a problem hiding this comment.
😭 That's really annoying. I'll see if I can get check_output to work.
* Determine backtrace.py location from location of test file so that the test can run from anywhere. * Ensure that the forked subprocess is closed properly by using check_output rather than native Popen. * Test that the subprocess does correctly fail.
|
Having been forced to look again at the |
|
@r-owen I believe I have made all the requested changes. |
No description provided.