From 59d632e0bfeec326daa7feba9ea71903ddc6cb4a Mon Sep 17 00:00:00 2001 From: Christian Long Date: Mon, 16 Mar 2015 18:14:12 -0500 Subject: [PATCH 1/4] Rename file --- flake8/tests/{test_warnings.py => _test_warnings.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename flake8/tests/{test_warnings.py => _test_warnings.py} (100%) diff --git a/flake8/tests/test_warnings.py b/flake8/tests/_test_warnings.py similarity index 100% rename from flake8/tests/test_warnings.py rename to flake8/tests/_test_warnings.py From 7663dbd485b2796fedebf96d3e44875e908dc59b Mon Sep 17 00:00:00 2001 From: Christian Long Date: Mon, 16 Mar 2015 18:13:23 -0500 Subject: [PATCH 2/4] Run some tests separately --- flake8/tests/_test_warnings.py | 156 +++++++++++++++++++++++++++++++-- tox.ini | 2 + 2 files changed, 153 insertions(+), 5 deletions(-) diff --git a/flake8/tests/_test_warnings.py b/flake8/tests/_test_warnings.py index 9dca44b..b177a17 100644 --- a/flake8/tests/_test_warnings.py +++ b/flake8/tests/_test_warnings.py @@ -9,6 +9,7 @@ tests (nosetests --with-isolation doesn't help). In tox.ini, these tests are run separately. + """ from __future__ import with_statement @@ -24,6 +25,152 @@ except ImportError: from flake8 import engine from flake8.util import is_windows +# The Problem +# ------------ +# +# Some of the tests in this module pass when this module is run on its own, but +# they fail when this module is run as part of the whole test suite. These are +# the problematic tests: +# +# test_jobs_verbose +# test_stdin_jobs_warning +# +# On some platforms, the warnings.capture_warnings function doesn't work +# properly when run with the other flake8 tests. It drops some warnings, even +# though the warnings filter is set to 'always'. However, when run separately, +# these tests pass. +# +# This problem only occurs on Windows, with Python 3.3 and older. Maybe it's +# related to PEP 446 - Inheritable file descriptors? +# +# +# +# +# Things that didn't work +# ------------ +# +# Nose --attr +# I tried using the nosetests --attr feature to run the tests separately. I +# put the following in setup.cfg +# +# [nosetests] +# atttr=!run_alone +# +# Then I added a tox section thst did this +# +# nosetests --attr=run_alone +# +# However, the command line --attr would not override the config file --attr, +# so the special tox section wound up runing all the tests, and failing. +# +# +# +# Nose --with-isolation +# The nosetests --with-isolation flag did not help. +# +# +# +# unittest.skipIf +# I tried decorating the problematic tests with the unittest.skipIf +# decorator. +# +# @unittest.skipIf(is_windows() and sys.version_info < (3, 4), +# "Fails on Windows with Python < 3.4 when run with other" +# " tests.") +# +# The idea is, skip the tests in the main test run, on affected platforms. +# Then, only on those platforms, come back in later and run the tests +# separately. +# +# I added a new stanza to tox.ini, to run the tests separately on the +# affected platforms. +# +# nosetests --no-skip +# +# I ran in to a bug in the nosetests skip plugin. It would report the test as +# having been run, but it would not actually run the test. So, when run with +# --no-skip, the following test would be reported as having run and passed! +# +# @unittest.skip("This passes o_o") +# def test_should_fail(self): +# assert 0 +# +# This bug has been reported here: +# "--no-skip broken with Python 2.7" +# https://github.com/nose-devs/nose/issues/512 +# +# +# +# +# +# +# +# Possible solutions +# ------------ +# +# Solution 1 +# Move the problematic tests to _test_warnings.py, so nose.collector will not +# find them. Set up a separate section in tox.ini that runs this: +# +# nosetests flake8.tests._test_warnings +# +# This allows all tests to pass on all platforms, when run through tox. +# However, it means that, even on unaffected platforms, the problematic tests +# are not discovered and run outside of tox (if the user just runs nosetests +# manually, for example). +# +# +# +# Solution 2 +# Use py.test, and its @pytest.mark.xfail decorator. Add some separate stanzas +# in tox, and use the pytest --runxfail option to run the tests separately. +# This will allow us to run all the tests together, on platforms that allow it. +# On platforms that don't allow us to run the tests all together, this still +# runs all the tests, but in two separate steps. +# +# +# [tox] +# envlist = +# . . . +# py27-run-alone, +# py33-run-alone, +# +# [testenv:py27-run-alone] +# basepython = python2.7 +# usedevelop = True +# deps = +# mock +# pytest +# commands = +# py.test --runxfail --pyargs flake8.tests.test_warnings +# +# [testenv:py33-run-alone] +# basepython = python3.3 +# usedevelop = True +# deps = +# mock +# pytest +# commands = +# py.test --runxfail --pyargs flake8.tests.test_warnings +# +# +# This is the same solution as the nosetests --no-skip solution I described +# above, but --runxfail does not have the same bug as --no-skip. +# +# One small problem with solution 2 is that the extra stanzas (py27-run-alone +# and py33-run-alone) get run even on Linux, where they are not needed because +# these tests were already run as part of the main test run. As far as I can +# tell, there is no way to restrict tox stanzas so they only run on certain +# platforms. +# +# +# +# +# +# +# I have chosen to use solution 1 because it is simpler and does not introduce +# a dependency on py.test. + class IntegrationTestCaseWarnings(unittest.TestCase): """Integration style tests to check that warnings are issued properly for @@ -68,8 +215,7 @@ class IntegrationTestCaseWarnings(unittest.TestCase): style_guide = engine.get_style_guide(*args, **kwargs) # Now that the warnings have been collected, return the style guide and - # the - # warnings. + # the warnings. return (style_guide, collected_warnings) def verify_warnings(self, collected_warnings, expected_warnings): @@ -186,12 +332,12 @@ class IntegrationTestCaseWarnings(unittest.TestCase): self.verify_warnings(collected_warnings, expected_warings) self.assertEqual(self.count, 1) - def test_jobs_verbose(self): - self.test_jobs(verbose=True) - def test_no_args_no_warnings_verbose(self): self.test_no_args_no_warnings(verbose=True) + def test_jobs_verbose(self): + self.test_jobs(verbose=True) + def test_stdin_jobs_warning_verbose(self): self.test_stdin_jobs_warning(verbose=True) diff --git a/tox.ini b/tox.ini index d26d358..73bec1f 100644 --- a/tox.ini +++ b/tox.ini @@ -7,9 +7,11 @@ envlist = usedevelop = True deps = mock + nose commands = python setup.py test -q python setup.py flake8 + nosetests flake8.tests._test_warnings [testenv:py27-flake8] basepython = python2.7 From 63d8af6af50fb2a838c973c50c363ff68ee80951 Mon Sep 17 00:00:00 2001 From: Christian Long Date: Mon, 16 Mar 2015 19:12:46 -0500 Subject: [PATCH 3/4] Document the other things I tried --- flake8/tests/_test_warnings.py | 73 +++++++++------------------------- 1 file changed, 18 insertions(+), 55 deletions(-) diff --git a/flake8/tests/_test_warnings.py b/flake8/tests/_test_warnings.py index b177a17..71fada0 100644 --- a/flake8/tests/_test_warnings.py +++ b/flake8/tests/_test_warnings.py @@ -101,14 +101,29 @@ from flake8.util import is_windows # # # +# py.test +# +# I tried using py.test, and its @pytest.mark.xfail decorator. I added some +# separate stanzas in tox, and useing the pytest --runxfail option to run the +# tests separately. This allows us to run all the tests together, on +# platforms that allow it. On platforms that don't allow us to run the tests +# all together, this still runs all the tests, but in two separate steps. +# +# This is the same solution as the nosetests --no-skip solution I described +# above, but --runxfail does not have the same bug as --no-skip. +# +# This has the advantage that all tests are discoverable by default, outside +# of tox. However, nose does not recognize the pytest.mark.xfail decorator. +# So, if a user runs nosetests, it still tries to run the problematic tests +# together with the rest of the test suite, causing them to fail. # # # # -# Possible solutions +# +# +# Solution # ------------ -# -# Solution 1 # Move the problematic tests to _test_warnings.py, so nose.collector will not # find them. Set up a separate section in tox.ini that runs this: # @@ -118,58 +133,6 @@ from flake8.util import is_windows # However, it means that, even on unaffected platforms, the problematic tests # are not discovered and run outside of tox (if the user just runs nosetests # manually, for example). -# -# -# -# Solution 2 -# Use py.test, and its @pytest.mark.xfail decorator. Add some separate stanzas -# in tox, and use the pytest --runxfail option to run the tests separately. -# This will allow us to run all the tests together, on platforms that allow it. -# On platforms that don't allow us to run the tests all together, this still -# runs all the tests, but in two separate steps. -# -# -# [tox] -# envlist = -# . . . -# py27-run-alone, -# py33-run-alone, -# -# [testenv:py27-run-alone] -# basepython = python2.7 -# usedevelop = True -# deps = -# mock -# pytest -# commands = -# py.test --runxfail --pyargs flake8.tests.test_warnings -# -# [testenv:py33-run-alone] -# basepython = python3.3 -# usedevelop = True -# deps = -# mock -# pytest -# commands = -# py.test --runxfail --pyargs flake8.tests.test_warnings -# -# -# This is the same solution as the nosetests --no-skip solution I described -# above, but --runxfail does not have the same bug as --no-skip. -# -# One small problem with solution 2 is that the extra stanzas (py27-run-alone -# and py33-run-alone) get run even on Linux, where they are not needed because -# these tests were already run as part of the main test run. As far as I can -# tell, there is no way to restrict tox stanzas so they only run on certain -# platforms. -# -# -# -# -# -# -# I have chosen to use solution 1 because it is simpler and does not introduce -# a dependency on py.test. class IntegrationTestCaseWarnings(unittest.TestCase): From ef3a47a1551b0230a22f8c20d662ac7ea80d3841 Mon Sep 17 00:00:00 2001 From: Christian Long Date: Mon, 16 Mar 2015 19:27:39 -0500 Subject: [PATCH 4/4] Move test back where it was --- flake8/tests/_test_warnings.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flake8/tests/_test_warnings.py b/flake8/tests/_test_warnings.py index 71fada0..004a597 100644 --- a/flake8/tests/_test_warnings.py +++ b/flake8/tests/_test_warnings.py @@ -295,12 +295,12 @@ class IntegrationTestCaseWarnings(unittest.TestCase): self.verify_warnings(collected_warnings, expected_warings) self.assertEqual(self.count, 1) - def test_no_args_no_warnings_verbose(self): - self.test_no_args_no_warnings(verbose=True) - def test_jobs_verbose(self): self.test_jobs(verbose=True) + def test_no_args_no_warnings_verbose(self): + self.test_no_args_no_warnings(verbose=True) + def test_stdin_jobs_warning_verbose(self): self.test_stdin_jobs_warning(verbose=True)