diff --git a/.coveragerc b/.coveragerc index 4028f8ea0..eaac16d82 100644 --- a/.coveragerc +++ b/.coveragerc @@ -8,3 +8,5 @@ source = omit = */allmydata/test/* */allmydata/_version.py +parallel = True +branch = True diff --git a/integration/conftest.py b/integration/conftest.py index 24173a9ea..c59b4cf8d 100644 --- a/integration/conftest.py +++ b/integration/conftest.py @@ -31,6 +31,7 @@ from util import ( _create_node, _run_node, _cleanup_twistd_process, + _tahoe_runner_optional_coverage, ) @@ -41,6 +42,10 @@ def pytest_addoption(parser): "--keep-tempdir", action="store_true", dest="keep", help="Keep the tmpdir with the client directories (introducer, etc)", ) + parser.addoption( + "--coverage", action="store_true", dest="coverage", + help="Collect coverage statistics", + ) @pytest.fixture(autouse=True, scope='session') def eliot_logging(): @@ -174,11 +179,11 @@ log_gatherer.furl = {log_furl} if not exists(intro_dir): mkdir(intro_dir) done_proto = _ProcessExitedProtocol() - reactor.spawnProcess( + _tahoe_runner_optional_coverage( done_proto, - sys.executable, + reactor, + request, ( - sys.executable, '-m', 'allmydata.scripts.runner', 'create-introducer', '--listen=tcp', '--hostname=localhost', @@ -195,11 +200,11 @@ log_gatherer.furl = {log_furl} # but on linux it means daemonize. "tahoe run" is consistent # between platforms. protocol = _MagicTextProtocol('introducer running') - process = reactor.spawnProcess( + process = _tahoe_runner_optional_coverage( protocol, - sys.executable, + reactor, + request, ( - sys.executable, '-m', 'allmydata.scripts.runner', 'run', intro_dir, ), @@ -241,11 +246,11 @@ log_gatherer.furl = {log_furl} if not exists(intro_dir): mkdir(intro_dir) done_proto = _ProcessExitedProtocol() - reactor.spawnProcess( + _tahoe_runner_optional_coverage( done_proto, - sys.executable, + reactor, + request, ( - sys.executable, '-m', 'allmydata.scripts.runner', 'create-introducer', '--tor-control-port', 'tcp:localhost:8010', '--listen=tor', @@ -262,11 +267,11 @@ log_gatherer.furl = {log_furl} # but on linux it means daemonize. "tahoe run" is consistent # between platforms. protocol = _MagicTextProtocol('introducer running') - process = reactor.spawnProcess( + process = _tahoe_runner_optional_coverage( protocol, - sys.executable, + reactor, + request, ( - sys.executable, '-m', 'allmydata.scripts.runner', 'run', intro_dir, ), @@ -365,11 +370,11 @@ def alice_invite(reactor, alice, temp_dir, request): # consistently fail if we don't hack in this pause...) import time ; time.sleep(5) proto = _CollectOutputProtocol() - reactor.spawnProcess( + _tahoe_runner_optional_coverage( proto, - sys.executable, + reactor, + request, [ - sys.executable, '-m', 'allmydata.scripts.runner', 'magic-folder', 'create', '--poll-interval', '2', '--basedir', node_dir, 'magik:', 'alice', @@ -380,11 +385,11 @@ def alice_invite(reactor, alice, temp_dir, request): with start_action(action_type=u"integration:alice:magic_folder:invite") as a: proto = _CollectOutputProtocol() - reactor.spawnProcess( + _tahoe_runner_optional_coverage( proto, - sys.executable, + reactor, + request, [ - sys.executable, '-m', 'allmydata.scripts.runner', 'magic-folder', 'invite', '--basedir', node_dir, 'magik:', 'bob', ] @@ -416,13 +421,13 @@ def magic_folder(reactor, alice_invite, alice, bob, temp_dir, request): print("pairing magic-folder") bob_dir = join(temp_dir, 'bob') proto = _CollectOutputProtocol() - reactor.spawnProcess( + _tahoe_runner_optional_coverage( proto, - sys.executable, + reactor, + request, [ - sys.executable, '-m', 'allmydata.scripts.runner', 'magic-folder', 'join', - '--poll-interval', '2', + '--poll-interval', '1', '--basedir', bob_dir, alice_invite, join(temp_dir, 'magic-bob'), diff --git a/integration/test_magic_folder.py b/integration/test_magic_folder.py index 2691639e6..c0f630847 100644 --- a/integration/test_magic_folder.py +++ b/integration/test_magic_folder.py @@ -408,7 +408,7 @@ def test_alice_adds_files_while_bob_is_offline(reactor, request, temp_dir, magic bob_node_dir = join(temp_dir, "bob") # Take Bob offline. - yield util.cli(reactor, bob_node_dir, "stop") + yield util.cli(request, reactor, bob_node_dir, "stop") # Create a couple files in Alice's local directory. some_files = list( @@ -422,7 +422,7 @@ def test_alice_adds_files_while_bob_is_offline(reactor, request, temp_dir, magic good = False for i in range(15): - status = yield util.magic_folder_cli(reactor, alice_node_dir, "status") + status = yield util.magic_folder_cli(request, reactor, alice_node_dir, "status") good = status.count(".added-while-offline (36 B): good, version=0") == len(some_files) * 2 if good: # We saw each file as having a local good state and a remote good diff --git a/integration/util.py b/integration/util.py index 4dd20aa49..41327bead 100644 --- a/integration/util.py +++ b/integration/util.py @@ -117,8 +117,8 @@ def _cleanup_twistd_process(twistd_process, exited): :return: After the process has exited. """ try: - print("signaling {} with KILL".format(twistd_process.pid)) - twistd_process.signalProcess('KILL') + print("signaling {} with TERM".format(twistd_process.pid)) + twistd_process.signalProcess('TERM') print("signaled, blocking on exit") pytest_twisted.blockon(exited) print("exited, goodbye") @@ -126,6 +126,24 @@ def _cleanup_twistd_process(twistd_process, exited): pass +def _tahoe_runner_optional_coverage(proto, reactor, request, other_args): + """ + Internal helper. Calls spawnProcess with `-m + allmydata.scripts.runner` and `other_args`, optionally inserting a + `--coverage` option if the `request` indicates we should. + """ + if request.config.getoption('coverage'): + args = [sys.executable, '-m', 'coverage', 'run', '-m', 'allmydata.scripts.runner', '--coverage'] + else: + args = [sys.executable, '-m', 'allmydata.scripts.runner'] + args += other_args + return reactor.spawnProcess( + proto, + sys.executable, + args, + ) + + def _run_node(reactor, node_dir, request, magic_text): if magic_text is None: magic_text = "client running" @@ -134,15 +152,16 @@ def _run_node(reactor, node_dir, request, magic_text): # on windows, "tahoe start" means: run forever in the foreground, # but on linux it means daemonize. "tahoe run" is consistent # between platforms. - process = reactor.spawnProcess( + + process = _tahoe_runner_optional_coverage( protocol, - sys.executable, - ( - sys.executable, '-m', 'allmydata.scripts.runner', + reactor, + request, + [ '--eliot-destination', 'file:{}/logs/eliot.json'.format(node_dir), 'run', node_dir, - ), + ], ) process.exited = protocol.exited @@ -179,7 +198,6 @@ def _create_node(reactor, request, temp_dir, introducer_furl, flog_gatherer, nam mkdir(node_dir) done_proto = _ProcessExitedProtocol() args = [ - sys.executable, '-m', 'allmydata.scripts.runner', 'create-node', '--nickname', name, '--introducer', introducer_furl, @@ -194,11 +212,7 @@ def _create_node(reactor, request, temp_dir, introducer_furl, flog_gatherer, nam args.append('--no-storage') args.append(node_dir) - reactor.spawnProcess( - done_proto, - sys.executable, - args, - ) + _tahoe_runner_optional_coverage(done_proto, reactor, request, args) created_d = done_proto.done def created(_): @@ -331,17 +345,17 @@ def await_file_vanishes(path, timeout=10): raise FileShouldVanishException(path, timeout) -def cli(reactor, node_dir, *argv): +def cli(request, reactor, node_dir, *argv): + """ + Run a tahoe CLI subcommand for a given node, optionally running + under coverage if '--coverage' was supplied. + """ proto = _CollectOutputProtocol() - reactor.spawnProcess( - proto, - sys.executable, - [ - sys.executable, '-m', 'allmydata.scripts.runner', - '--node-directory', node_dir, - ] + list(argv), + _tahoe_runner_optional_coverage( + proto, reactor, request, + ['--node-directory', node_dir] + list(argv), ) return proto.done -def magic_folder_cli(reactor, node_dir, *argv): - return cli(reactor, node_dir, "magic-folder", *argv) +def magic_folder_cli(request, reactor, node_dir, *argv): + return cli(request, reactor, node_dir, "magic-folder", *argv) diff --git a/newsfragments/3234.other b/newsfragments/3234.other new file mode 100644 index 000000000..35dd44a1a --- /dev/null +++ b/newsfragments/3234.other @@ -0,0 +1 @@ +Collect coverage information from integration tests diff --git a/src/allmydata/scripts/runner.py b/src/allmydata/scripts/runner.py index 327325e95..751e27158 100644 --- a/src/allmydata/scripts/runner.py +++ b/src/allmydata/scripts/runner.py @@ -194,7 +194,51 @@ def run(): # doesn't return: calls sys.exit(rc) task.react(_run_with_reactor) + +def _setup_coverage(reactor): + """ + Arrange for coverage to be collected if the 'coverage' package is + installed + """ + # can we put this _setup_coverage call after we hit + # argument-parsing? + if '--coverage' not in sys.argv: + return + sys.argv.remove('--coverage') + + try: + import coverage + except ImportError: + raise RuntimeError( + "The 'coveage' package must be installed to use --coverage" + ) + + # this doesn't change the shell's notion of the environment, but + # it makes the test in process_startup() succeed, which is the + # goal here. + os.environ["COVERAGE_PROCESS_START"] = '.coveragerc' + + # maybe-start the global coverage, unless it already got started + cov = coverage.process_startup() + if cov is None: + cov = coverage.process_startup.coverage + + def write_coverage_data(): + """ + Make sure that coverage has stopped; internally, it depends on + ataxit handlers running which doesn't always happen (Twisted's + shutdown hook also won't run if os._exit() is called, but it + runs more-often than atexit handlers). + """ + cov.stop() + cov.save() + reactor.addSystemEventTrigger('after', 'shutdown', write_coverage_data) + + def _run_with_reactor(reactor): + + _setup_coverage(reactor) + d = defer.maybeDeferred(parse_or_exit_with_explanation, sys.argv[1:]) d.addCallback(_maybe_enable_eliot_logging, reactor) d.addCallback(dispatch) diff --git a/src/allmydata/test/cli/test_cli.py b/src/allmydata/test/cli/test_cli.py index 1f7f90c7f..8ea2966c6 100644 --- a/src/allmydata/test/cli/test_cli.py +++ b/src/allmydata/test/cli/test_cli.py @@ -2,7 +2,7 @@ import os.path from six.moves import cStringIO as StringIO import urllib, sys import re -from mock import patch +from mock import patch, Mock from twisted.trial import unittest from twisted.python.monkey import MonkeyPatcher @@ -525,7 +525,8 @@ class CLI(CLITestMixin, unittest.TestCase): self.failUnlessEqual(exitcode, 1) def fake_react(f): - d = f("reactor") + reactor = Mock() + d = f(reactor) # normally this Deferred would be errbacked with SystemExit, but # since we mocked out sys.exit, it will be fired with None. So # it's safe to drop it on the floor. diff --git a/tox.ini b/tox.ini index e165ed168..fa2e41fe5 100644 --- a/tox.ini +++ b/tox.ini @@ -49,9 +49,13 @@ commands = trial {env:TAHOE_LAFS_TRIAL_ARGS:--rterrors} {posargs:allmydata} [testenv:integration] +setenv = + COVERAGE_PROCESS_START=.coveragerc commands = # NOTE: 'run with "py.test --keep-tempdir -s -v integration/" to debug failures' - py.test -v integration/ + py.test --coverage -v integration/ + coverage combine + coverage report [testenv:coverage] # coverage (with --branch) takes about 65% longer to run @@ -64,6 +68,7 @@ commands = pip freeze tahoe --version coverage run --branch -m twisted.trial {env:TAHOE_LAFS_TRIAL_ARGS:--rterrors --reporter=timing} {posargs:allmydata} + coverage combine coverage xml [testenv:codechecks]