Skip tests instead of fail'ing if VCS <foo> command not installed #42

mgedmin · 2014-12-29T20:13:03Z

I have svn, hg and git installed, but not bzr. Any VCS tests will currently fail due to a missing command. Eg: If a user only uses git, svn, hg and bzr tests will fail.

They should instead be skipped with an appropriate message: "skipping . 'foo' command not installed" or equivalent.

======================================================================
ERROR: test_get_vcs_files (tests.TestBzr)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 770, in test_get_vcs_files
    self._init_vcs()
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 756, in _init_vcs
    self.vcs._init_vcs()
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 831, in _init_vcs
    self._run('bzr', 'init')
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 724, in _run
    stderr=subprocess.STDOUT)
  File "/usr/local/lib/python2.7/subprocess.py", line 710, in __init__
    errread, errwrite)
  File "/usr/local/lib/python2.7/subprocess.py", line 1335, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

======================================================================
ERROR: test_get_vcs_files_added_but_uncommitted (tests.TestBzr)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 781, in test_get_vcs_files_added_but_uncommitted
    self._init_vcs()
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 756, in _init_vcs
    self.vcs._init_vcs()
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 831, in _init_vcs
    self._run('bzr', 'init')
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 724, in _run
    stderr=subprocess.STDOUT)
  File "/usr/local/lib/python2.7/subprocess.py", line 710, in __init__
    errread, errwrite)
  File "/usr/local/lib/python2.7/subprocess.py", line 1335, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

======================================================================
ERROR: test_get_vcs_files_in_a_subdir (tests.TestBzr)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 791, in test_get_vcs_files_in_a_subdir
    self._init_vcs()
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 756, in _init_vcs
    self.vcs._init_vcs()
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 831, in _init_vcs
    self._run('bzr', 'init')
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 724, in _run
    stderr=subprocess.STDOUT)
  File "/usr/local/lib/python2.7/subprocess.py", line 710, in __init__
    errread, errwrite)
  File "/usr/local/lib/python2.7/subprocess.py", line 1335, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

======================================================================
ERROR: test_get_vcs_files_nonascii_filenames (tests.TestBzr)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 803, in test_get_vcs_files_nonascii_filenames
    self._init_vcs()
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 756, in _init_vcs
    self.vcs._init_vcs()
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 831, in _init_vcs
    self._run('bzr', 'init')
  File "/mnt/home/user/repos/freebsd/ports/devel/py-check-manifest/work/check-manifest-0.22/tests.py", line 724, in _run
    stderr=subprocess.STDOUT)
  File "/usr/local/lib/python2.7/subprocess.py", line 710, in __init__
    errread, errwrite)
  File "/usr/local/lib/python2.7/subprocess.py", line 1335, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

----------------------------------------------------------------------
Ran 87 tests in 8.683s

FAILED (errors=4)

mgedmin · 2014-12-29T08:17:43Z

I'm not sure about this: I don't want to accidentally break bzr support just because I forgot to install bzr on my dev or CI machine and the tests got skipped.

I wouldn't mind replacing the four obscure failures with one clear one ("cannot test bzr support: bzr is not installed").

I probably wouldn't even mind having that not be a failure (i.e. exit code 0) so end-users can check if check-manifest works on their machine, for the subset of version control systems they want to use; as long as there's a way to require all version control systems on a CI machine (e.g. by setting an environment variable).

Would you care to prepare a pull request?

mgedmin · 2014-12-29T08:18:41Z

I'm more concerned about the behavior of check-manifest itself: if you run it on a bzr checkout but don't have bzr installed, is there a clear error message or an obscure traceback? (I'm guessing the latter and I'd like to change it to the former.)

koobs · 2014-12-29T08:36:41Z

It's not just bzr support however, its any of the VCS commands, in any combination.

I was going to suggest requiring bzr, svn, hg and git as a past of tests_require, but I can't think of how since they're not Python packages.

The canonical example/comparison of a package that does this is SQLAlchemy, which supports many database backends. The test suite passes for any databases you have installed, and skips those you don't.

I note in unittest skipping, that "TestCase.setUp() can also skip the test. This is useful when a resource that needs to be set up is not available."

This might be the way to go to cover all vcs-foo unit tests with the same skip.

In terms of your desire to want all four installed for your own (as author) QA & pre-release testing, that's definitely important for your own development process. I would wrap that logic up in your Makefile, which is the most appropriate place for it. If you didn't have a Makefile, your test runner (tox, travis, etc) would be.

Re a pull request, I don't have the requisite Python skills to write up the method to portably check for the existence of a file using subprocess, but I'm sure someone has solved that already. Perhaps pip (which supports bzr, hg, svn and git) does already?

Edit:

Regarding the behaviour of check-manifest itself I agree, it should spit out a pretty error.

mgedmin · 2014-12-29T08:45:15Z

I'll see what I can do.

koobs · 2014-12-29T08:53:19Z

Champion :)

Fixes #42

mgedmin · 2014-12-29T20:17:26Z

The PR is preliminar: I checked that the tests get skipped if git isn't installed, but I haven't added the bit where I can ask Travis (or my Jenkins) to fail if any tests were skipped.

On Python 2.6 the test output is rather ugly:

......................./home/mg/src/check-manifest/.tox/py26/lib/python2.6/site-packages/unittest2/case.py:608: RuntimeWarning: TestResult has no addSkip method, skips not reported
  self._addSkip(result, test, reason)
................................................................
----------------------------------------------------------------------
Ran 87 tests in 8.813s

OK

I suppose I can live with that.

coveralls · 2014-12-29T20:18:44Z

Coverage remained the same when pulling 62b0159 on skip-tests into 32b6f67 on master.

koobs · 2014-12-30T03:09:17Z

Very nice 👍

This is so that I can detect problems with my CI server environment.

coveralls · 2015-01-30T10:19:25Z

Coverage remained the same at 100.0% when pulling d78a9a9 on skip-tests into 32b6f67 on master.

mgedmin · 2015-01-30T10:34:13Z

I'm down the rabbit hole.

TestCheckManifest relies on git being available. Options:

require git always, allow bzr/hg/svn to be uninstalled
require at least one of git/bzr/hg/svn, make TestCheckManifest pick any of them

I tried approach 2 because it seemed friendlier and discovered that

TestCheckManifest.test_missing_source_files fails with bzr/hg
TestCheckManifest.test_relative_pathname and test_relative_python fail with svn

So perhaps I should be running TestCheckManifest four times, with all four version control systems (skipping the ones that aren't installed)?

The tests will pick any of git/hg/bzr/svn, whichever is available. You can force the selection by setting FORCE_TEST_VCS. Slight downside to this: - TestCheckManifest.test_missing_source_files fails with bzr/hg - TestCheckManifest.test_relative_{pathname,python} fail with svn I'll now investigate if those are real bugs. I wonder if I should add a Travis matrix for forcing all the VCS systems?

Originally the idea was that I'd define an abstract VCS interface, test that, and then test the main logic using any of the VCSes. In practice I discovered that some main tests fail with some of the VCSes, which means my abstractions are leaking somewhere.

coveralls · 2015-01-30T10:42:49Z

Coverage remained the same at 100.0% when pulling 509727e on skip-tests into 32b6f67 on master.

coveralls · 2015-01-30T10:50:15Z

Coverage remained the same at 100.0% when pulling 4d36ff0 on skip-tests into 32b6f67 on master.

The bug was in the test: it assumed self._vcs._init_vcs() wouldn't change the current working directory.

This makes it behave the same way as with Git, and makes tests failed with FORCE_TEST_VCS=hg. AFAICT the original bug (#32) never affected Mercurial repositories, so the user-visible change is merely cosmetic.

coveralls · 2015-01-30T11:22:50Z

Coverage remained the same at 100.0% when pulling 03e660d on skip-tests into 32b6f67 on master.

koobs · 2015-01-30T11:35:36Z

"So perhaps I should be running TestCheckManifest four times, with all four version control systems (skipping the ones that aren't installed)?"

This ^^^ 👍

Unitize your tests :)

"Fix" here means "skip the unimportant part of the test that fails", since it's only a cosmetic feature.

coveralls · 2015-01-30T12:36:12Z

Coverage remained the same at 100.0% when pulling 12b4a23 on skip-tests into 32b6f67 on master.

Skip tests instead of failing if VCS <foo> command not installed

Skip VCS tests if the VCS isn't installed

62b0159

Fixes #42

Don't skip tests if SKIP_NO_TESTS=1

d78a9a9

This is so that I can detect problems with my CI server environment.

mgedmin added 2 commits January 30, 2015 12:38

mgedmin added 2 commits January 30, 2015 12:55

Fix tests when FORCE_TEST_VCS=svn

cf0ca84

The bug was in the test: it assumed self._vcs._init_vcs() wouldn't change the current working directory.

Warn about accidentally deleted files in Mercurial repositories

03e660d

This makes it behave the same way as with Git, and makes tests failed with FORCE_TEST_VCS=hg. AFAICT the original bug (#32) never affected Mercurial repositories, so the user-visible change is merely cosmetic.

Fix tests when FORCE_TEST_VCS=bzr

12b4a23

"Fix" here means "skip the unimportant part of the test that fails", since it's only a cosmetic feature.

mgedmin added a commit that referenced this pull request Jan 30, 2015

Merge pull request #42 from mgedmin/skip-tests

0893381

Skip tests instead of failing if VCS <foo> command not installed

mgedmin merged commit 0893381 into master Jan 30, 2015

mgedmin deleted the skip-tests branch January 30, 2015 22:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Skip tests instead of fail'ing if VCS <foo> command not installed #42

Skip tests instead of fail'ing if VCS <foo> command not installed #42

mgedmin commented Dec 29, 2014

mgedmin commented Dec 29, 2014

mgedmin commented Dec 29, 2014

koobs commented Dec 29, 2014

mgedmin commented Dec 29, 2014

koobs commented Dec 29, 2014

mgedmin commented Dec 29, 2014

coveralls commented Dec 29, 2014

koobs commented Dec 30, 2014

coveralls commented Jan 30, 2015

mgedmin commented Jan 30, 2015

coveralls commented Jan 30, 2015

coveralls commented Jan 30, 2015

coveralls commented Jan 30, 2015

koobs commented Jan 30, 2015

coveralls commented Jan 30, 2015

Skip tests instead of fail'ing if VCS <foo> command not installed #42

Skip tests instead of fail'ing if VCS <foo> command not installed #42

Conversation

mgedmin commented Dec 29, 2014

mgedmin commented Dec 29, 2014

mgedmin commented Dec 29, 2014

koobs commented Dec 29, 2014

mgedmin commented Dec 29, 2014

koobs commented Dec 29, 2014

mgedmin commented Dec 29, 2014

coveralls commented Dec 29, 2014

koobs commented Dec 30, 2014

coveralls commented Jan 30, 2015

mgedmin commented Jan 30, 2015

coveralls commented Jan 30, 2015

coveralls commented Jan 30, 2015

coveralls commented Jan 30, 2015

koobs commented Jan 30, 2015

coveralls commented Jan 30, 2015