From bc00513d336d44a6560a8c9b7059f04135215c20 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 2 May 2019 12:51:06 -0400 Subject: [PATCH 01/25] news fragment --- newsfragments/3025.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3025.minor diff --git a/newsfragments/3025.minor b/newsfragments/3025.minor new file mode 100644 index 000000000..e69de29bb From e6da5e6a82a99a50296edf0a5a25ba28b171f2c3 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 9 Apr 2019 08:51:51 -0400 Subject: [PATCH 02/25] Switch to simpler, declarative skip style --- src/allmydata/test/test_runner.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 2ce989a6d..d6b1a625c 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -5,6 +5,10 @@ from __future__ import ( import os.path, re, sys +from unittest import ( + skipIf, +) + from twisted.trial import unittest from twisted.python import usage, runtime @@ -41,13 +45,12 @@ def get_root_from_file(src): srcfile = allmydata.__file__ rootdir = get_root_from_file(srcfile) +cannot_daemonize = False +if runtime.platformType == "win32": + # twistd on windows doesn't daemonize. cygwin should work normally. + cannot_daemonize = "twistd does not fork under windows" class RunBinTahoeMixin: - def skip_if_cannot_daemonize(self): - if runtime.platformType == "win32": - # twistd on windows doesn't daemonize. cygwin should work normally. - raise unittest.SkipTest("twistd does not fork under windows") - @inlineCallbacks def find_import_location(self): res = yield self.run_bintahoe(["--version-and-path"]) @@ -383,9 +386,8 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, fileutil.make_dirs(basedir) return basedir + @skipIf(cannot_daemonize, cannot_daemonize) def test_introducer(self): - self.skip_if_cannot_daemonize() - basedir = self.workdir("test_introducer") c1 = os.path.join(basedir, "c1") exit_trigger_file = os.path.join(c1, _Client.EXIT_TRIGGER_FILE) @@ -503,9 +505,8 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, # on Francois's Lenny-armv5tel buildslave. test_introducer.timeout = 960 + @skipIf(cannot_daemonize, cannot_daemonize) def test_client_no_noise(self): - self.skip_if_cannot_daemonize() - basedir = self.workdir("test_client_no_noise") c1 = os.path.join(basedir, "c1") exit_trigger_file = os.path.join(c1, _Client.EXIT_TRIGGER_FILE) @@ -567,9 +568,8 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, d.addBoth(self._remove, exit_trigger_file) return d + @skipIf(cannot_daemonize, cannot_daemonize) def test_client(self): - self.skip_if_cannot_daemonize() - basedir = self.workdir("test_client") c1 = os.path.join(basedir, "c1") exit_trigger_file = os.path.join(c1, _Client.EXIT_TRIGGER_FILE) @@ -685,8 +685,8 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, fileutil.remove(file) return res + @skipIf(cannot_daemonize, cannot_daemonize) def test_baddir(self): - self.skip_if_cannot_daemonize() basedir = self.workdir("test_baddir") fileutil.make_dirs(basedir) From 5a1183500e6cb1e439756fdae08c36ff2333cbc2 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 2 May 2019 14:21:35 -0400 Subject: [PATCH 03/25] rewrite RunNode.test_client to use "tahoe run" --- src/allmydata/scripts/tahoe_stop.py | 4 +- src/allmydata/test/cli_node_api.py | 152 ++++++++++++++++++++ src/allmydata/test/test_runner.py | 215 ++++++++++++++-------------- 3 files changed, 262 insertions(+), 109 deletions(-) create mode 100644 src/allmydata/test/cli_node_api.py diff --git a/src/allmydata/scripts/tahoe_stop.py b/src/allmydata/scripts/tahoe_stop.py index 9918f7e0d..2b7b98faf 100644 --- a/src/allmydata/scripts/tahoe_stop.py +++ b/src/allmydata/scripts/tahoe_stop.py @@ -32,12 +32,12 @@ def stop(config): print("%s does not look like a running node directory (no twistd.pid)" % quoted_basedir, file=err) # we define rc=2 to mean "nothing is running, but it wasn't me who # stopped it" - return 2 + return COULD_NOT_STOP elif pid == -1: print("%s contains an invalid PID file" % basedir, file=err) # we define rc=2 to mean "nothing is running, but it wasn't me who # stopped it" - return 2 + return COULD_NOT_STOP # kill it hard (SIGKILL), delete the twistd.pid file, then wait for the # process itself to go away. If it hasn't gone away after 20 seconds, warn diff --git a/src/allmydata/test/cli_node_api.py b/src/allmydata/test/cli_node_api.py new file mode 100644 index 000000000..903148131 --- /dev/null +++ b/src/allmydata/test/cli_node_api.py @@ -0,0 +1,152 @@ + +__all__ = [ + "CLINodeAPI", + "Expect", + "on_stdout", + "wait_for_exit", +] + +import os +import sys + +import attr + +from twisted.internet.error import ( + ProcessDone, +) +from twisted.python.filepath import ( + FilePath, +) +from twisted.internet.protocol import ( + Protocol, + ProcessProtocol, +) +from twisted.internet.defer import ( + Deferred, + succeed, +) + +from allmydata.client import _Client + +class Expect(Protocol): + def __init__(self): + self._expectations = [] + + def expect(self, expectation): + if expectation in self._buffer: + return succeed(None) + d = Deferred() + self._expectations.append((expectation, d)) + return d + + def connectionMade(self): + self._buffer = b"" + + def dataReceived(self, data): + self._buffer += data + for i in range(len(self._expectations) - 1, -1, -1): + expectation, d = self._expectations[i] + if expectation in self._buffer: + del self._expectations[i] + d.callback(None) + + +class _Stdout(ProcessProtocol): + def __init__(self, stdout_protocol): + self._stdout_protocol = stdout_protocol + + def connectionMade(self): + self._stdout_protocol.makeConnection(self.transport) + + def outReceived(self, data): + self._stdout_protocol.dataReceived(data) + + def processEnded(self, reason): + self._stdout_protocol.connectionLost(reason) + + +def on_stdout(protocol): + return _Stdout(protocol) + + +@attr.s +class CLINodeAPI(object): + reactor = attr.ib() + basedir = attr.ib(type=FilePath) + + @property + def twistd_pid_file(self): + return self.basedir.child(u"twistd.pid") + + @property + def node_url_file(self): + return self.basedir.child(u"node.url") + + @property + def storage_furl_file(self): + return self.basedir.child(u"private").child(u"storage.furl") + + @property + def config_file(self): + return self.basedir.child(u"tahoe.cfg") + + @property + def exit_trigger_file(self): + return self.basedir.child(_Client.EXIT_TRIGGER_FILE) + + def _execute(self, process_protocol, argv): + exe = sys.executable + argv = [ + exe, + u"-m", + u"allmydata.scripts.runner", + ] + argv + return self.reactor.spawnProcess( + processProtocol=process_protocol, + executable=exe, + args=argv, + env=os.environ, + ) + + def run(self, protocol): + """ + Start the node running. + + :param ProcessProtocol protocol: This protocol will be hooked up to + the node process and can handle output or generate input. + """ + self.process = self._execute( + protocol, + [u"run", self.basedir.asTextMode().path], + ) + # Don't let the process run away forever. + self.active() + + def stop(self, protocol): + self._execute( + protocol, + [u"stop", self.basedir.asTextMode().path], + ) + + def active(self): + # By writing this file, we get two minutes before the client will + # exit. This ensures that even if the 'stop' command doesn't work (and + # the test fails), the client should still terminate. + self.exit_trigger_file.touch() + + +class _WaitForEnd(ProcessProtocol): + def __init__(self, ended): + self._ended = ended + + def processEnded(self, reason): + if reason.check(ProcessDone): + self._ended.callback(None) + else: + self._ended.errback(reason) + + +def wait_for_exit(): + ended = Deferred() + protocol = _WaitForEnd(ended) + return protocol, ended diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index d6b1a625c..99ae78e95 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -9,10 +9,17 @@ from unittest import ( skipIf, ) +import attr + from twisted.trial import unittest +from twisted.internet.error import ( + ProcessTerminated, +) +from twisted.internet import reactor from twisted.python import usage, runtime from twisted.internet.defer import inlineCallbacks, returnValue +from twisted.python.filepath import FilePath from allmydata.util import fileutil, pollmixin from allmydata.util.encodingutil import unicode_to_argv, unicode_to_output, \ @@ -22,10 +29,21 @@ from allmydata.test import common_util import allmydata from allmydata import __appname__ from .common_util import parse_cli, run_cli - +from .cli_node_api import ( + CLINodeAPI, + Expect, + on_stdout, + wait_for_exit, +) from ._twisted_9607 import ( getProcessOutputAndValue, ) +from ..util.eliotutil import ( + inline_callbacks, +) +from ..scripts.tahoe_stop import ( + COULD_NOT_STOP, +) timeout = 240 @@ -367,19 +385,22 @@ class CreateNode(unittest.TestCase): # can't provide all three _test("create-stats-gatherer --hostname=foo --location=foo --port=foo D") + class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, RunBinTahoeMixin): - # exercise "tahoe start", for both introducer, client node, and - # key-generator, by spawning "tahoe start" as a subprocess. This doesn't - # get us figleaf-based line-level coverage, but it does a better job of - # confirming that the user can actually run "./bin/tahoe start" and - # expect it to work. This verifies that bin/tahoe sets up PYTHONPATH and - # the like correctly. + """ + exercise "tahoe run" (or "tahoe start", for a while), for both + introducer, client node, and key-generator, by spawning "tahoe run" (or + "tahoe start") as a subprocess. This doesn't get us line-level coverage, + but it does a better job of confirming that the user can actually run + "./bin/tahoe run" and expect it to work. This verifies that bin/tahoe + sets up PYTHONPATH and the like correctly. - # This doesn't work on cygwin (it hangs forever), so we skip this test - # when we're on cygwin. It is likely that "tahoe start" itself doesn't - # work on cygwin: twisted seems unable to provide a version of - # spawnProcess which really works there. + This doesn't work on cygwin (it hangs forever), so we skip this test + when we're on cygwin. It is likely that "tahoe start" itself doesn't + work on cygwin: twisted seems unable to provide a version of + spawnProcess which really works there. + """ def workdir(self, name): basedir = os.path.join("test_runner", "RunNode", name) @@ -568,118 +589,98 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, d.addBoth(self._remove, exit_trigger_file) return d - @skipIf(cannot_daemonize, cannot_daemonize) + @inline_callbacks def test_client(self): + """ + Test many things. + + 0) Verify that "tahoe create-node" takes a --webport option and writes + the value to the configuration file. + + 1) Verify that "tahoe run" writes a pid file and a node url file. + + 2) Verify that the storage furl file has a stable value across a + "tahoe run" / "tahoe stop" / "tahoe run" sequence. + + 3) Verify that the pid file is removed after "tahoe stop" succeeds. + """ basedir = self.workdir("test_client") c1 = os.path.join(basedir, "c1") - exit_trigger_file = os.path.join(c1, _Client.EXIT_TRIGGER_FILE) - twistd_pid_file = os.path.join(c1, "twistd.pid") - node_url_file = os.path.join(c1, "node.url") - storage_furl_file = os.path.join(c1, "private", "storage.furl") - config_file = os.path.join(c1, "tahoe.cfg") - d = self.run_bintahoe(["--quiet", "create-node", "--basedir", c1, - "--webport", "0", - "--hostname", "localhost"]) - def _cb(res): - out, err, rc_or_sig = res - self.failUnlessEqual(rc_or_sig, 0) + def stop_and_wait(tahoe): + p, d = wait_for_exit() + tahoe.stop(p) + return d - # Check that the --webport option worked. - config = fileutil.read(config_file) - self.failUnlessIn('\nweb.port = 0\n', config) + tahoe = CLINodeAPI(reactor, FilePath(c1)) + # Set this up right now so we don't forget later. + self.addCleanup( + lambda: stop_and_wait(tahoe).addErrback( + # Let it fail because the process has already exited. + lambda err: ( + err.trap(ProcessTerminated) + and self.assertEqual( + err.value.exitCode, + COULD_NOT_STOP, + ) + ) + ) + ) - # By writing this file, we get two minutes before the client will - # exit. This ensures that even if the 'stop' command doesn't work - # (and the test fails), the client should still terminate. - fileutil.write(exit_trigger_file, "") - # now it's safe to start the node - d.addCallback(_cb) + out, err, rc_or_sig = yield self.run_bintahoe([ + "--quiet", "create-node", "--basedir", c1, + "--webport", "0", + "--hostname", "localhost", + ]) + self.failUnlessEqual(rc_or_sig, 0) - def _start(res): - return self.run_bintahoe(["--quiet", "start", c1]) - d.addCallback(_start) + # Check that the --webport option worked. + config = fileutil.read(tahoe.config_file.path) + self.assertIn('\nweb.port = 0\n', config) - def _cb2(res): - out, err, rc_or_sig = res - fileutil.write(exit_trigger_file, "") - errstr = "rc=%d, OUT: '%s', ERR: '%s'" % (rc_or_sig, out, err) - self.failUnlessEqual(rc_or_sig, 0, errstr) - self.failUnlessEqual(out, "", errstr) - # self.failUnlessEqual(err, "", errstr) # See test_client_no_noise -- for now we ignore noise. + # After this it's safe to start the node + tahoe.active() - # the parent (twistd) has exited. However, twistd writes the pid - # from the child, not the parent, so we can't expect twistd.pid - # to exist quite yet. + p = Expect() + # This will run until we stop it. + tahoe.run(on_stdout(p)) + # Wait for startup to have proceeded to a reasonable point. + yield p.expect("client running") + tahoe.active() - # the node is running, but it might not have made it past the - # first reactor turn yet, and if we kill it too early, it won't - # remove the twistd.pid file. So wait until it does something - # that we know it won't do until after the first turn. - d.addCallback(_cb2) + # read the storage.furl file so we can check that its contents don't + # change on restart + storage_furl = fileutil.read(tahoe.storage_furl_file.path) + self.assertTrue(tahoe.twistd_pid_file.exists()) - def _node_has_started(): - return os.path.exists(node_url_file) - d.addCallback(lambda res: self.poll(_node_has_started)) + # rm this so we can detect when the second incarnation is ready + tahoe.node_url_file.remove() + yield stop_and_wait(tahoe) - def _started(res): - # read the storage.furl file so we can check that its contents - # don't change on restart - self.storage_furl = fileutil.read(storage_furl_file) + p = Expect() + # We don't have to add another cleanup for this one, the one from + # above is still registered. + tahoe.run(on_stdout(p)) + yield p.expect("client running") + tahoe.active() - fileutil.write(exit_trigger_file, "") - self.failUnless(os.path.exists(twistd_pid_file)) + self.assertEqual( + storage_furl, + fileutil.read(tahoe.storage_furl_file.path), + ) - # rm this so we can detect when the second incarnation is ready - os.unlink(node_url_file) - return self.run_bintahoe(["--quiet", "restart", c1]) - d.addCallback(_started) + self.assertTrue( + tahoe.twistd_pid_file.exists(), + "PID file ({}) didn't exist when we expected it to. These exist: {}".format( + tahoe.twistd_pid_file, + tahoe.twistd_pid_file.parent().listdir(), + ), + ) + yield stop_and_wait(tahoe) - def _cb3(res): - out, err, rc_or_sig = res + # twistd.pid should be gone by now. + self.assertFalse(tahoe.twistd_pid_file.exists()) - fileutil.write(exit_trigger_file, "") - errstr = "rc=%d, OUT: '%s', ERR: '%s'" % (rc_or_sig, out, err) - self.failUnlessEqual(rc_or_sig, 0, errstr) - self.failUnlessEqual(out, "", errstr) - # self.failUnlessEqual(err, "", errstr) # See test_client_no_noise -- for now we ignore noise. - d.addCallback(_cb3) - - # again, the second incarnation of the node might not be ready yet, - # so poll until it is - d.addCallback(lambda res: self.poll(_node_has_started)) - - def _check_same_furl(res): - self.failUnlessEqual(self.storage_furl, - fileutil.read(storage_furl_file)) - d.addCallback(_check_same_furl) - - # now we can kill it. TODO: On a slow machine, the node might kill - # itself before we get a chance to, especially if spawning the - # 'tahoe stop' command takes a while. - def _stop(res): - fileutil.write(exit_trigger_file, "") - self.failUnless(os.path.exists(twistd_pid_file), - (twistd_pid_file, os.listdir(os.path.dirname(twistd_pid_file)))) - return self.run_bintahoe(["--quiet", "stop", c1]) - d.addCallback(_stop) - - def _cb4(res): - out, err, rc_or_sig = res - - fileutil.write(exit_trigger_file, "") - # the parent has exited by now - errstr = "rc=%d, OUT: '%s', ERR: '%s'" % (rc_or_sig, out, err) - self.failUnlessEqual(rc_or_sig, 0, errstr) - self.failUnlessEqual(out, "", errstr) - # self.failUnlessEqual(err, "", errstr) # See test_client_no_noise -- for now we ignore noise. - # the parent was supposed to poll and wait until it sees - # twistd.pid go away before it exits, so twistd.pid should be - # gone by now. - self.failIf(os.path.exists(twistd_pid_file)) - d.addCallback(_cb4) - d.addBoth(self._remove, exit_trigger_file) - return d def _remove(self, res, file): fileutil.remove(file) From 57fc078383699c5a93f8e96a8af0275b0ce23a97 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 3 May 2019 07:27:58 -0400 Subject: [PATCH 04/25] factor cleanup into api class --- src/allmydata/test/cli_node_api.py | 21 ++++++++++++++++++++- src/allmydata/test/test_runner.py | 16 +--------------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/allmydata/test/cli_node_api.py b/src/allmydata/test/cli_node_api.py index 903148131..e4a96537f 100644 --- a/src/allmydata/test/cli_node_api.py +++ b/src/allmydata/test/cli_node_api.py @@ -26,7 +26,12 @@ from twisted.internet.defer import ( succeed, ) -from allmydata.client import _Client +from ..client import ( + _Client, +) +from ..scripts.tahoe_stop import ( + COULD_NOT_STOP, +) class Expect(Protocol): def __init__(self): @@ -134,6 +139,20 @@ class CLINodeAPI(object): # the test fails), the client should still terminate. self.exit_trigger_file.touch() + def cleanup(self): + stopping = stop_and_wait(tahoe) + stopping.addErrback( + # Let it fail because the process has already exited. + lambda err: ( + err.trap(ProcessTerminated) + and self.assertEqual( + err.value.exitCode, + COULD_NOT_STOP, + ) + ) + ) + return stopping + class _WaitForEnd(ProcessProtocol): def __init__(self, ended): diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 99ae78e95..0166eb0bb 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -41,9 +41,6 @@ from ._twisted_9607 import ( from ..util.eliotutil import ( inline_callbacks, ) -from ..scripts.tahoe_stop import ( - COULD_NOT_STOP, -) timeout = 240 @@ -614,18 +611,7 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, tahoe = CLINodeAPI(reactor, FilePath(c1)) # Set this up right now so we don't forget later. - self.addCleanup( - lambda: stop_and_wait(tahoe).addErrback( - # Let it fail because the process has already exited. - lambda err: ( - err.trap(ProcessTerminated) - and self.assertEqual( - err.value.exitCode, - COULD_NOT_STOP, - ) - ) - ) - ) + self.addCleanup(tahoe.cleanup) out, err, rc_or_sig = yield self.run_bintahoe([ "--quiet", "create-node", "--basedir", c1, From 0e8472c017231f9387f07611d36701ada6b22493 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 3 May 2019 08:55:35 -0400 Subject: [PATCH 05/25] rewrite test_baddir as several `tahoe run`-using tests --- src/allmydata/test/cli_node_api.py | 56 ++++++++---- src/allmydata/test/test_runner.py | 139 +++++++++++++++++++++-------- 2 files changed, 140 insertions(+), 55 deletions(-) diff --git a/src/allmydata/test/cli_node_api.py b/src/allmydata/test/cli_node_api.py index e4a96537f..cf6050cac 100644 --- a/src/allmydata/test/cli_node_api.py +++ b/src/allmydata/test/cli_node_api.py @@ -3,16 +3,19 @@ __all__ = [ "CLINodeAPI", "Expect", "on_stdout", + "on_stdout_and_stderr", "wait_for_exit", ] import os import sys +from errno import ENOENT import attr from twisted.internet.error import ( ProcessDone, + ProcessTerminated, ) from twisted.python.filepath import ( FilePath, @@ -37,6 +40,9 @@ class Expect(Protocol): def __init__(self): self._expectations = [] + def get_buffered_output(self): + return self._buffer + def expect(self, expectation): if expectation in self._buffer: return succeed(None) @@ -55,23 +61,32 @@ class Expect(Protocol): del self._expectations[i] d.callback(None) + def connectionLost(self, reason): + for ignored, d in self._expectations: + d.errback(reason) -class _Stdout(ProcessProtocol): - def __init__(self, stdout_protocol): + +class _ProcessProtocolAdapter(ProcessProtocol): + def __init__(self, stdout_protocol, fds): self._stdout_protocol = stdout_protocol + self._fds = fds def connectionMade(self): self._stdout_protocol.makeConnection(self.transport) - def outReceived(self, data): - self._stdout_protocol.dataReceived(data) + def childDataReceived(self, childFD, data): + if childFD in self._fds: + self._stdout_protocol.dataReceived(data) def processEnded(self, reason): self._stdout_protocol.connectionLost(reason) def on_stdout(protocol): - return _Stdout(protocol) + return _ProcessProtocolAdapter(protocol, {1}) + +def on_stdout_and_stderr(protocol): + return _ProcessProtocolAdapter(protocol, {1, 2}) @attr.s @@ -125,7 +140,11 @@ class CLINodeAPI(object): [u"run", self.basedir.asTextMode().path], ) # Don't let the process run away forever. - self.active() + try: + self.active() + except OSError as e: + if ENOENT != e.errno: + raise def stop(self, protocol): self._execute( @@ -133,24 +152,27 @@ class CLINodeAPI(object): [u"stop", self.basedir.asTextMode().path], ) + def stop_and_wait(self): + protocol, ended = wait_for_exit() + self.stop(protocol) + return ended + def active(self): # By writing this file, we get two minutes before the client will # exit. This ensures that even if the 'stop' command doesn't work (and # the test fails), the client should still terminate. self.exit_trigger_file.touch() + def _check_cleanup_reason(self, reason): + # Let it fail because the process has already exited. + reason.trap(ProcessTerminated) + if reason.value.exitCode != COULD_NOT_STOP: + return reason + return None + def cleanup(self): - stopping = stop_and_wait(tahoe) - stopping.addErrback( - # Let it fail because the process has already exited. - lambda err: ( - err.trap(ProcessTerminated) - and self.assertEqual( - err.value.exitCode, - COULD_NOT_STOP, - ) - ) - ) + stopping = self.stop_and_wait() + stopping.addErrback(self._check_cleanup_reason) return stopping diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 0166eb0bb..6edd49ef4 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -13,12 +13,13 @@ import attr from twisted.trial import unittest -from twisted.internet.error import ( - ProcessTerminated, -) from twisted.internet import reactor from twisted.python import usage, runtime -from twisted.internet.defer import inlineCallbacks, returnValue +from twisted.internet.defer import ( + inlineCallbacks, + returnValue, + DeferredList, +) from twisted.python.filepath import FilePath from allmydata.util import fileutil, pollmixin @@ -33,6 +34,7 @@ from .cli_node_api import ( CLINodeAPI, Expect, on_stdout, + on_stdout_and_stderr, wait_for_exit, ) from ._twisted_9607 import ( @@ -604,11 +606,6 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, basedir = self.workdir("test_client") c1 = os.path.join(basedir, "c1") - def stop_and_wait(tahoe): - p, d = wait_for_exit() - tahoe.stop(p) - return d - tahoe = CLINodeAPI(reactor, FilePath(c1)) # Set this up right now so we don't forget later. self.addCleanup(tahoe.cleanup) @@ -641,7 +638,7 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, # rm this so we can detect when the second incarnation is ready tahoe.node_url_file.remove() - yield stop_and_wait(tahoe) + yield tahoe.stop_and_wait() p = Expect() # We don't have to add another cleanup for this one, the one from @@ -662,7 +659,7 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, tahoe.twistd_pid_file.parent().listdir(), ), ) - yield stop_and_wait(tahoe) + yield tahoe.stop_and_wait() # twistd.pid should be gone by now. self.assertFalse(tahoe.twistd_pid_file.exists()) @@ -672,36 +669,102 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, fileutil.remove(file) return res - @skipIf(cannot_daemonize, cannot_daemonize) - def test_baddir(self): - basedir = self.workdir("test_baddir") + def test_run_bad_directory(self): + """ + If ``tahoe run`` is pointed at a non-node directory, it reports an error + and exits. + """ + return self._bad_directory_test( + u"test_run_bad_directory", + "tahoe run", + lambda tahoe, p: tahoe.run(p), + "is not a recognizable node directory", + ) + + def test_run_bogus_directory(self): + """ + If ``tahoe run`` is pointed at a non-directory, it reports an error and + exits. + """ + return self._bad_directory_test( + u"test_run_bogus_directory", + "tahoe run", + lambda tahoe, p: CLINodeAPI( + tahoe.reactor, + tahoe.basedir.sibling(u"bogus"), + ).run(p), + "does not look like a directory at all" + ) + + def test_stop_bad_directory(self): + """ + If ``tahoe run`` is pointed at a directory where no node is running, it + reports an error and exits. + """ + return self._bad_directory_test( + u"test_stop_bad_directory", + "tahoe stop", + lambda tahoe, p: tahoe.stop(p), + "does not look like a running node directory", + ) + + @inline_callbacks + def _bad_directory_test(self, workdir, description, operation, expected_message): + """ + Verify that a certain ``tahoe`` CLI operation produces a certain expected + message and then exits. + + :param unicode workdir: A distinct path name for this test to operate + on. + + :param unicode description: A description of the operation being + performed. + + :param operation: A two-argument callable implementing the operation. + The first argument is a ``CLINodeAPI`` instance to use to perform + the operation. The second argument is an ``IProcessProtocol`` to + which the operations output must be delivered. + + :param unicode expected_message: Some text that is expected in the + stdout or stderr of the operation in the successful case. + + :return: A ``Deferred`` that fires when the assertions have been made. + """ + basedir = self.workdir(workdir) fileutil.make_dirs(basedir) - d = self.run_bintahoe(["--quiet", "start", "--basedir", basedir]) - def _cb(res): - out, err, rc_or_sig = res - self.failUnlessEqual(rc_or_sig, 1) - self.failUnless("is not a recognizable node directory" in err, err) - d.addCallback(_cb) + tahoe = CLINodeAPI(reactor, FilePath(basedir)) + # If tahoe ends up thinking it should keep running, make sure it stops + # promptly when the test is done. + self.addCleanup(tahoe.cleanup) - def _then_stop_it(res): - return self.run_bintahoe(["--quiet", "stop", "--basedir", basedir]) - d.addCallback(_then_stop_it) + p = Expect() + operation(tahoe, on_stdout_and_stderr(p)) - def _cb2(res): - out, err, rc_or_sig = res - self.failUnlessEqual(rc_or_sig, 2) - self.failUnless("does not look like a running node directory" in err) - d.addCallback(_cb2) + client_running = p.expect(b"client running") - def _then_start_in_bogus_basedir(res): - not_a_dir = os.path.join(basedir, "bogus") - return self.run_bintahoe(["--quiet", "start", "--basedir", not_a_dir]) - d.addCallback(_then_start_in_bogus_basedir) + result, index = yield DeferredList([ + p.expect(expected_message), + client_running, + ], fireOnOneCallback=True, consumeErrors=True, + ) - def _cb3(res): - out, err, rc_or_sig = res - self.failUnlessEqual(rc_or_sig, 1) - self.failUnlessIn("does not look like a directory at all", err) - d.addCallback(_cb3) - return d + self.assertEqual( + index, + 0, + "Expected error message from '{}', got something else: {}".format( + description, + p.get_buffered_output(), + ), + ) + + # It should not be running. + self.assertFalse(tahoe.twistd_pid_file.exists()) + + # Wait for the operation to *complete*. If we got this far it's + # because we got the expected message so we can expect the "tahoe ..." + # child process to exit very soon. This other Deferred will fail when + # it eventually does but DeferredList above will consume the error. + # What's left is a perfect indicator that the process has exited and + # we won't get blamed for leaving the reactor dirty. + yield client_running From 97e8ba830184c05df2cfd16c8e67db606864b8b6 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 3 May 2019 11:36:11 -0400 Subject: [PATCH 06/25] Remove test_client_no_noise and rewrite test_introducer "tahoe run" has no quiet option so `test_client_no_noise` is not applicable. This is a loss of the coverage of the quiet option for `tahoe start`. That is unfortunate but fixing any `tahoe start`-using test is really hard and the functionality that is no longer covered is so trivial it hardly seems like it made sense to test it by running multiple tahoe child processes anyway. --- src/allmydata/test/cli_node_api.py | 42 ++++-- src/allmydata/test/test_runner.py | 213 +++++++---------------------- 2 files changed, 79 insertions(+), 176 deletions(-) diff --git a/src/allmydata/test/cli_node_api.py b/src/allmydata/test/cli_node_api.py index cf6050cac..607d4e3c5 100644 --- a/src/allmydata/test/cli_node_api.py +++ b/src/allmydata/test/cli_node_api.py @@ -4,6 +4,7 @@ __all__ = [ "Expect", "on_stdout", "on_stdout_and_stderr", + "on_different", "wait_for_exit", ] @@ -17,6 +18,9 @@ from twisted.internet.error import ( ProcessDone, ProcessTerminated, ) +from twisted.internet.interfaces import ( + IProcessProtocol, +) from twisted.python.filepath import ( FilePath, ) @@ -67,27 +71,37 @@ class Expect(Protocol): class _ProcessProtocolAdapter(ProcessProtocol): - def __init__(self, stdout_protocol, fds): - self._stdout_protocol = stdout_protocol + def __init__(self, fds): self._fds = fds def connectionMade(self): - self._stdout_protocol.makeConnection(self.transport) + for proto in self._fds.values(): + proto.makeConnection(self.transport) def childDataReceived(self, childFD, data): - if childFD in self._fds: - self._stdout_protocol.dataReceived(data) + try: + proto = self._fds[childFD] + except KeyError: + pass + else: + proto.dataReceived(data) def processEnded(self, reason): - self._stdout_protocol.connectionLost(reason) + notified = set() + for proto in self._fds.values(): + if proto not in notified: + proto.connectionLost(reason) + notified.add(proto) def on_stdout(protocol): - return _ProcessProtocolAdapter(protocol, {1}) + return _ProcessProtocolAdapter({1: protocol}) def on_stdout_and_stderr(protocol): - return _ProcessProtocolAdapter(protocol, {1, 2}) + return _ProcessProtocolAdapter({1: protocol, 2: protocol}) +def on_different(fd_mapping): + return _ProcessProtocolAdapter(fd_mapping) @attr.s class CLINodeAPI(object): @@ -106,6 +120,10 @@ class CLINodeAPI(object): def storage_furl_file(self): return self.basedir.child(u"private").child(u"storage.furl") + @property + def introducer_furl_file(self): + return self.basedir.child(u"private").child(u"introducer.furl") + @property def config_file(self): return self.basedir.child(u"tahoe.cfg") @@ -128,16 +146,18 @@ class CLINodeAPI(object): env=os.environ, ) - def run(self, protocol): + def run(self, protocol, extra_tahoe_args=()): """ Start the node running. - :param ProcessProtocol protocol: This protocol will be hooked up to + :param IProcessProtocol protocol: This protocol will be hooked up to the node process and can handle output or generate input. """ + if not IProcessProtocol.providedBy(protocol): + raise TypeError("run requires process protocol, got {}".format(protocol)) self.process = self._execute( protocol, - [u"run", self.basedir.asTextMode().path], + list(extra_tahoe_args) + [u"run", self.basedir.asTextMode().path], ) # Don't let the process run away forever. try: diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 6edd49ef4..cbf595699 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -35,6 +35,7 @@ from .cli_node_api import ( Expect, on_stdout, on_stdout_and_stderr, + on_different, wait_for_exit, ) from ._twisted_9607 import ( @@ -406,187 +407,69 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, fileutil.make_dirs(basedir) return basedir - @skipIf(cannot_daemonize, cannot_daemonize) + @inline_callbacks def test_introducer(self): + """ + The introducer furl is stable across restarts. + """ basedir = self.workdir("test_introducer") c1 = os.path.join(basedir, "c1") - exit_trigger_file = os.path.join(c1, _Client.EXIT_TRIGGER_FILE) - twistd_pid_file = os.path.join(c1, "twistd.pid") - introducer_furl_file = os.path.join(c1, "private", "introducer.furl") - node_url_file = os.path.join(c1, "node.url") - config_file = os.path.join(c1, "tahoe.cfg") + tahoe = CLINodeAPI(reactor, FilePath(c1)) + self.addCleanup(tahoe.stop_and_wait) - d = self.run_bintahoe(["--quiet", "create-introducer", "--basedir", c1, "--hostname", "localhost"]) - def _cb(res): - out, err, rc_or_sig = res - self.failUnlessEqual(rc_or_sig, 0) + out, err, rc_or_sig = yield self.run_bintahoe([ + "--quiet", + "create-introducer", + "--basedir", c1, + "--hostname", "127.0.0.1", + ]) - # This makes sure that node.url is written, which allows us to - # detect when the introducer restarts in _node_has_restarted below. - config = fileutil.read(config_file) - self.failUnlessIn('\nweb.port = \n', config) - fileutil.write(config_file, config.replace('\nweb.port = \n', '\nweb.port = 0\n')) + self.assertEqual(rc_or_sig, 0) - # by writing this file, we get ten seconds before the node will - # exit. This insures that even if the test fails (and the 'stop' - # command doesn't work), the client should still terminate. - fileutil.write(exit_trigger_file, "") - # now it's safe to start the node - d.addCallback(_cb) + # This makes sure that node.url is written, which allows us to + # detect when the introducer restarts in _node_has_restarted below. + config = fileutil.read(tahoe.config_file.path) + self.assertIn('\nweb.port = \n', config) + fileutil.write( + tahoe.config_file.path, + config.replace('\nweb.port = \n', '\nweb.port = 0\n'), + ) - def _then_start_the_node(res): - return self.run_bintahoe(["--quiet", "start", c1]) - d.addCallback(_then_start_the_node) + p = Expect() + tahoe.run(on_stdout(p)) + yield p.expect("introducer running") + tahoe.active() - def _cb2(res): - out, err, rc_or_sig = res + yield self.poll(tahoe.introducer_furl_file.exists) - fileutil.write(exit_trigger_file, "") - errstr = "rc=%d, OUT: '%s', ERR: '%s'" % (rc_or_sig, out, err) - self.failUnlessEqual(rc_or_sig, 0, errstr) - self.failUnlessEqual(out, "", errstr) - # self.failUnlessEqual(err, "", errstr) # See test_client_no_noise -- for now we ignore noise. + # read the introducer.furl file so we can check that the contents + # don't change on restart + furl = fileutil.read(tahoe.introducer_furl_file.path) - # the parent (twistd) has exited. However, twistd writes the pid - # from the child, not the parent, so we can't expect twistd.pid - # to exist quite yet. + tahoe.active() - # the node is running, but it might not have made it past the - # first reactor turn yet, and if we kill it too early, it won't - # remove the twistd.pid file. So wait until it does something - # that we know it won't do until after the first turn. - d.addCallback(_cb2) + self.assertTrue(tahoe.twistd_pid_file.exists()) + self.assertTrue(tahoe.node_url_file.exists()) - def _node_has_started(): - return os.path.exists(introducer_furl_file) - d.addCallback(lambda res: self.poll(_node_has_started)) + # rm this so we can detect when the second incarnation is ready + tahoe.node_url_file.remove() - def _started(res): - # read the introducer.furl file so we can check that the contents - # don't change on restart - self.furl = fileutil.read(introducer_furl_file) + yield tahoe.stop_and_wait() - fileutil.write(exit_trigger_file, "") - self.failUnless(os.path.exists(twistd_pid_file)) - self.failUnless(os.path.exists(node_url_file)) + p = Expect() + tahoe.run(on_stdout(p)) + yield p.expect("introducer running") - # rm this so we can detect when the second incarnation is ready - os.unlink(node_url_file) - return self.run_bintahoe(["--quiet", "restart", c1]) - d.addCallback(_started) + # Again, the second incarnation of the node might not be ready yet, so + # poll until it is. This time introducer_furl_file already exists, so + # we check for the existence of node_url_file instead. + yield self.poll(tahoe.node_url_file.exists) - def _then(res): - out, err, rc_or_sig = res - fileutil.write(exit_trigger_file, "") - errstr = "rc=%d, OUT: '%s', ERR: '%s'" % (rc_or_sig, out, err) - self.failUnlessEqual(rc_or_sig, 0, errstr) - self.failUnlessEqual(out, "", errstr) - # self.failUnlessEqual(err, "", errstr) # See test_client_no_noise -- for now we ignore noise. - d.addCallback(_then) - - # Again, the second incarnation of the node might not be ready yet, - # so poll until it is. This time introducer_furl_file already - # exists, so we check for the existence of node_url_file instead. - def _node_has_restarted(): - return os.path.exists(node_url_file) - d.addCallback(lambda res: self.poll(_node_has_restarted)) - - def _check_same_furl(res): - self.failUnless(os.path.exists(introducer_furl_file)) - self.failUnlessEqual(self.furl, fileutil.read(introducer_furl_file)) - d.addCallback(_check_same_furl) - - # Now we can kill it. TODO: On a slow machine, the node might kill - # itself before we get a chance to, especially if spawning the - # 'tahoe stop' command takes a while. - def _stop(res): - fileutil.write(exit_trigger_file, "") - self.failUnless(os.path.exists(twistd_pid_file)) - - return self.run_bintahoe(["--quiet", "stop", c1]) - d.addCallback(_stop) - - def _after_stopping(res): - out, err, rc_or_sig = res - fileutil.write(exit_trigger_file, "") - # the parent has exited by now - errstr = "rc=%d, OUT: '%s', ERR: '%s'" % (rc_or_sig, out, err) - self.failUnlessEqual(rc_or_sig, 0, errstr) - self.failUnlessEqual(out, "", errstr) - # self.failUnlessEqual(err, "", errstr) # See test_client_no_noise -- for now we ignore noise. - # the parent was supposed to poll and wait until it sees - # twistd.pid go away before it exits, so twistd.pid should be - # gone by now. - self.failIf(os.path.exists(twistd_pid_file)) - d.addCallback(_after_stopping) - d.addBoth(self._remove, exit_trigger_file) - return d - # This test has hit a 240-second timeout on our feisty2.5 buildslave, and a 480-second timeout - # on Francois's Lenny-armv5tel buildslave. - test_introducer.timeout = 960 - - @skipIf(cannot_daemonize, cannot_daemonize) - def test_client_no_noise(self): - basedir = self.workdir("test_client_no_noise") - c1 = os.path.join(basedir, "c1") - exit_trigger_file = os.path.join(c1, _Client.EXIT_TRIGGER_FILE) - twistd_pid_file = os.path.join(c1, "twistd.pid") - node_url_file = os.path.join(c1, "node.url") - - d = self.run_bintahoe(["--quiet", "create-client", "--basedir", c1, "--webport", "0"]) - def _cb(res): - out, err, rc_or_sig = res - errstr = "cc=%d, OUT: '%s', ERR: '%s'" % (rc_or_sig, out, err) - assert rc_or_sig == 0, errstr - self.failUnlessEqual(rc_or_sig, 0) - - # By writing this file, we get two minutes before the client will exit. This ensures - # that even if the 'stop' command doesn't work (and the test fails), the client should - # still terminate. - fileutil.write(exit_trigger_file, "") - # now it's safe to start the node - d.addCallback(_cb) - - def _start(res): - return self.run_bintahoe(["--quiet", "start", c1]) - d.addCallback(_start) - - def _cb2(res): - out, err, rc_or_sig = res - errstr = "cc=%d, OUT: '%s', ERR: '%s'" % (rc_or_sig, out, err) - fileutil.write(exit_trigger_file, "") - self.failUnlessEqual(rc_or_sig, 0, errstr) - self.failUnlessEqual(out, "", errstr) # If you emit noise, you fail this test. - errlines = err.split("\n") - self.failIf([True for line in errlines if (line != "" and "UserWarning: Unbuilt egg for setuptools" not in line - and "from pkg_resources import load_entry_point" not in line)], errstr) - if err != "": - raise unittest.SkipTest("This test is known not to pass on Ubuntu Lucid; see #1235.") - - # the parent (twistd) has exited. However, twistd writes the pid - # from the child, not the parent, so we can't expect twistd.pid - # to exist quite yet. - - # the node is running, but it might not have made it past the - # first reactor turn yet, and if we kill it too early, it won't - # remove the twistd.pid file. So wait until it does something - # that we know it won't do until after the first turn. - d.addCallback(_cb2) - - def _node_has_started(): - return os.path.exists(node_url_file) - d.addCallback(lambda res: self.poll(_node_has_started)) - - # now we can kill it. TODO: On a slow machine, the node might kill - # itself before we get a chance to, especially if spawning the - # 'tahoe stop' command takes a while. - def _stop(res): - self.failUnless(os.path.exists(twistd_pid_file), - (twistd_pid_file, os.listdir(os.path.dirname(twistd_pid_file)))) - return self.run_bintahoe(["--quiet", "stop", c1]) - d.addCallback(_stop) - d.addBoth(self._remove, exit_trigger_file) - return d + # The point of this test! After starting the second time the + # introducer furl file must exist and contain the same contents as it + # did before. + self.assertTrue(tahoe.introducer_furl_file.exists()) + self.assertEqual(furl, fileutil.read(tahoe.introducer_furl_file.path)) @inline_callbacks def test_client(self): From aac36fb30ac4a9dbdc8673b2f4549cfa913072b5 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 3 May 2019 12:09:03 -0400 Subject: [PATCH 07/25] fix line separator --- src/allmydata/test/test_runner.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index cbf595699..3fd1f1f54 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -10,6 +10,7 @@ from unittest import ( ) import attr +from os import linesep from twisted.trial import unittest @@ -429,10 +430,13 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, # This makes sure that node.url is written, which allows us to # detect when the introducer restarts in _node_has_restarted below. config = fileutil.read(tahoe.config_file.path) - self.assertIn('\nweb.port = \n', config) + self.assertIn('{}web.port = {}'.format(linesep, linesep), config) fileutil.write( tahoe.config_file.path, - config.replace('\nweb.port = \n', '\nweb.port = 0\n'), + config.replace( + '{}web.port = {}'.format(linesep, linesep), + '{}web.port = 0{}'.format(linesep, linesep), + ) ) p = Expect() @@ -502,7 +506,10 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, # Check that the --webport option worked. config = fileutil.read(tahoe.config_file.path) - self.assertIn('\nweb.port = 0\n', config) + self.assertIn( + '{}web.port = 0{}'.format(linesep, linesep), + config, + ) # After this it's safe to start the node tahoe.active() From 86d33e19c5c3201f339852f473674d84170a5497 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 3 May 2019 12:09:10 -0400 Subject: [PATCH 08/25] no more "tahoe start" here at all --- src/allmydata/test/test_runner.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 3fd1f1f54..dfef1bef1 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -390,12 +390,11 @@ class CreateNode(unittest.TestCase): class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, RunBinTahoeMixin): """ - exercise "tahoe run" (or "tahoe start", for a while), for both - introducer, client node, and key-generator, by spawning "tahoe run" (or - "tahoe start") as a subprocess. This doesn't get us line-level coverage, - but it does a better job of confirming that the user can actually run - "./bin/tahoe run" and expect it to work. This verifies that bin/tahoe - sets up PYTHONPATH and the like correctly. + exercise "tahoe run" for both introducer, client node, and key-generator, + by spawning "tahoe run" (or "tahoe start") as a subprocess. This doesn't + get us line-level coverage, but it does a better job of confirming that + the user can actually run "./bin/tahoe run" and expect it to work. This + verifies that bin/tahoe sets up PYTHONPATH and the like correctly. This doesn't work on cygwin (it hangs forever), so we skip this test when we're on cygwin. It is likely that "tahoe start" itself doesn't From b38a724d3d41bace21f307aad276fd8306cf220c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 3 May 2019 12:09:21 -0400 Subject: [PATCH 09/25] remove unused things --- src/allmydata/test/test_runner.py | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index dfef1bef1..8b52f3312 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -4,18 +4,12 @@ from __future__ import ( ) import os.path, re, sys - -from unittest import ( - skipIf, -) - -import attr from os import linesep from twisted.trial import unittest from twisted.internet import reactor -from twisted.python import usage, runtime +from twisted.python import usage from twisted.internet.defer import ( inlineCallbacks, returnValue, @@ -26,7 +20,6 @@ from twisted.python.filepath import FilePath from allmydata.util import fileutil, pollmixin from allmydata.util.encodingutil import unicode_to_argv, unicode_to_output, \ get_filesystem_encoding -from allmydata.client import _Client from allmydata.test import common_util import allmydata from allmydata import __appname__ @@ -36,8 +29,6 @@ from .cli_node_api import ( Expect, on_stdout, on_stdout_and_stderr, - on_different, - wait_for_exit, ) from ._twisted_9607 import ( getProcessOutputAndValue, @@ -64,11 +55,6 @@ def get_root_from_file(src): srcfile = allmydata.__file__ rootdir = get_root_from_file(srcfile) -cannot_daemonize = False -if runtime.platformType == "win32": - # twistd on windows doesn't daemonize. cygwin should work normally. - cannot_daemonize = "twistd does not fork under windows" - class RunBinTahoeMixin: @inlineCallbacks def find_import_location(self): From 6110fb0b9c134b482c8539cfda4b17b5a1ef4a82 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 13 May 2019 06:28:57 -0400 Subject: [PATCH 10/25] Skip the PID file checks on Windows --- src/allmydata/test/test_runner.py | 43 +++++++++++++++++++------------ 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 8b52f3312..0bc0a9e4d 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -16,7 +16,9 @@ from twisted.internet.defer import ( DeferredList, ) from twisted.python.filepath import FilePath - +from twisted.python.runtime import ( + platform, +) from allmydata.util import fileutil, pollmixin from allmydata.util.encodingutil import unicode_to_argv, unicode_to_output, \ get_filesystem_encoding @@ -437,7 +439,9 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, tahoe.active() - self.assertTrue(tahoe.twistd_pid_file.exists()) + # We don't keep track of PIDs in files on Windows. + if not platform.isWindows(): + self.assertTrue(tahoe.twistd_pid_file.exists()) self.assertTrue(tahoe.node_url_file.exists()) # rm this so we can detect when the second incarnation is ready @@ -468,12 +472,12 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, 0) Verify that "tahoe create-node" takes a --webport option and writes the value to the configuration file. - 1) Verify that "tahoe run" writes a pid file and a node url file. + 1) Verify that "tahoe run" writes a pid file and a node url file (on POSIX). 2) Verify that the storage furl file has a stable value across a "tahoe run" / "tahoe stop" / "tahoe run" sequence. - 3) Verify that the pid file is removed after "tahoe stop" succeeds. + 3) Verify that the pid file is removed after "tahoe stop" succeeds (on POSIX). """ basedir = self.workdir("test_client") c1 = os.path.join(basedir, "c1") @@ -509,7 +513,10 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, # read the storage.furl file so we can check that its contents don't # change on restart storage_furl = fileutil.read(tahoe.storage_furl_file.path) - self.assertTrue(tahoe.twistd_pid_file.exists()) + + # We don't keep track of PIDs in files on Windows. + if not platform.isWindows(): + self.assertTrue(tahoe.twistd_pid_file.exists()) # rm this so we can detect when the second incarnation is ready tahoe.node_url_file.remove() @@ -527,17 +534,20 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, fileutil.read(tahoe.storage_furl_file.path), ) - self.assertTrue( - tahoe.twistd_pid_file.exists(), - "PID file ({}) didn't exist when we expected it to. These exist: {}".format( - tahoe.twistd_pid_file, - tahoe.twistd_pid_file.parent().listdir(), - ), - ) + if not platform.isWindows(): + self.assertTrue( + tahoe.twistd_pid_file.exists(), + "PID file ({}) didn't exist when we expected it to. " + "These exist: {}".format( + tahoe.twistd_pid_file, + tahoe.twistd_pid_file.parent().listdir(), + ), + ) yield tahoe.stop_and_wait() - # twistd.pid should be gone by now. - self.assertFalse(tahoe.twistd_pid_file.exists()) + if not platform.isWindows(): + # twistd.pid should be gone by now. + self.assertFalse(tahoe.twistd_pid_file.exists()) def _remove(self, res, file): @@ -633,8 +643,9 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, ), ) - # It should not be running. - self.assertFalse(tahoe.twistd_pid_file.exists()) + if not platform.isWindows(): + # It should not be running. + self.assertFalse(tahoe.twistd_pid_file.exists()) # Wait for the operation to *complete*. If we got this far it's # because we got the expected message so we can expect the "tahoe ..." From 79a230cce5324359f969e3437364cce39776b08f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 13 May 2019 09:31:46 -0400 Subject: [PATCH 11/25] Fix stopping on Windows --- src/allmydata/test/cli_node_api.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/allmydata/test/cli_node_api.py b/src/allmydata/test/cli_node_api.py index 607d4e3c5..526b333b7 100644 --- a/src/allmydata/test/cli_node_api.py +++ b/src/allmydata/test/cli_node_api.py @@ -17,6 +17,7 @@ import attr from twisted.internet.error import ( ProcessDone, ProcessTerminated, + ProcessExitedAlready, ) from twisted.internet.interfaces import ( IProcessProtocol, @@ -32,13 +33,18 @@ from twisted.internet.defer import ( Deferred, succeed, ) - +from twisted.internet.task import ( + deferLater, +) from ..client import ( _Client, ) from ..scripts.tahoe_stop import ( COULD_NOT_STOP, ) +from ..util.eliotutil import ( + inline_callbacks, +) class Expect(Protocol): def __init__(self): @@ -107,6 +113,7 @@ def on_different(fd_mapping): class CLINodeAPI(object): reactor = attr.ib() basedir = attr.ib(type=FilePath) + process = attr.ib(default=None) @property def twistd_pid_file(self): @@ -172,10 +179,22 @@ class CLINodeAPI(object): [u"stop", self.basedir.asTextMode().path], ) + @inline_callbacks def stop_and_wait(self): - protocol, ended = wait_for_exit() - self.stop(protocol) - return ended + if platform.isWindows(): + # On Windows there is no PID file and no "tahoe stop". + if self.process is not None: + while True: + try: + self.process.signalProcess("TERM") + except ProcessExitedAlready: + break + else: + yield deferLater(self.reactor, 0.1, lambda: None) + else: + protocol, ended = wait_for_exit() + self.stop(protocol) + yield ended def active(self): # By writing this file, we get two minutes before the client will From 463f6ae63cb75d6636d993b4819eb179d0ae2f1e Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 13 May 2019 09:35:31 -0400 Subject: [PATCH 12/25] sigh, I don't know --- src/allmydata/test/cli_node_api.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/allmydata/test/cli_node_api.py b/src/allmydata/test/cli_node_api.py index 526b333b7..f6ee6015e 100644 --- a/src/allmydata/test/cli_node_api.py +++ b/src/allmydata/test/cli_node_api.py @@ -25,6 +25,9 @@ from twisted.internet.interfaces import ( from twisted.python.filepath import ( FilePath, ) +from twisted.python.runtime import ( + platform, +) from twisted.internet.protocol import ( Protocol, ProcessProtocol, From ec304a8c2ae576e37e47c5a672e20ce696e854ae Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 13 May 2019 10:09:01 -0400 Subject: [PATCH 13/25] Constrain to older pip that might be less broken Trying to fix appveyor failure: ``` Obtaining file:///C:/projects/tahoe-lafs Complete output from command python setup.py egg_info: warning: no previously-included files found matching 'pyproject.toml' Installed c:\projects\tahoe-lafs\.eggs\setuptools-41.0.1-py2.7.egg Traceback (most recent call last): File "", line 1, in File "C:\projects\tahoe-lafs\setup.py", line 297, in **setup_args File "c:\python27\Lib\distutils\core.py", line 111, in setup _setup_distribution = dist = klass(attrs) File "C:\projects\tahoe-lafs\.tox\coverage\lib\site-packages\setuptools\dist.py", line 272, in __init__ _Distribution.__init__(self,attrs) File "c:\python27\Lib\distutils\dist.py", line 287, in __init__ self.finalize_options() File "C:\projects\tahoe-lafs\.tox\coverage\lib\site-packages\setuptools\dist.py", line 327, in finalize_options ep.load()(self, ep.name, value) File "C:\projects\tahoe-lafs\.tox\coverage\lib\site-packages\pkg_resources\__init__.py", line 2202, in load return self.resolve() File "C:\projects\tahoe-lafs\.tox\coverage\lib\site-packages\pkg_resources\__init__.py", line 2212, in resolve raise ImportError(str(exc)) ImportError: 'module' object has no attribute 'check_specifier' ``` --- .appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.appveyor.yml b/.appveyor.yml index bc1353b3c..cb240addf 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -13,7 +13,7 @@ environment: install: - | - %PYTHON%\python.exe -m pip install -U pip + %PYTHON%\python.exe -m pip install -U 'pip<19' %PYTHON%\python.exe -m pip install wheel tox virtualenv # note: From f5d5a1311ff557760443257e713fa5df4c4acf97 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 13 May 2019 10:18:11 -0400 Subject: [PATCH 14/25] Maybe this is how you do quotes on Windows --- .appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.appveyor.yml b/.appveyor.yml index cb240addf..8f1f3e0db 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -13,7 +13,7 @@ environment: install: - | - %PYTHON%\python.exe -m pip install -U 'pip<19' + %PYTHON%\python.exe -m pip install -U "pip<19" %PYTHON%\python.exe -m pip install wheel tox virtualenv # note: From f17647f29cf4aeb361d3d70b4f5da2a9faa6cccd Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 13 May 2019 10:23:18 -0400 Subject: [PATCH 15/25] Find out what versions are in the tox environment --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index 2271bd2ac..848454333 100644 --- a/tox.ini +++ b/tox.ini @@ -38,6 +38,7 @@ commands = [testenv:coverage] # coverage (with --branch) takes about 65% longer to run commands = + pip freeze tahoe --version coverage run --branch -m twisted.trial {env:TAHOE_LAFS_TRIAL_ARGS:--rterrors --reporter=timing} {posargs:allmydata} coverage xml From abf319888d69b412032422a0a2893a457eb7d2ac Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 13 May 2019 10:27:41 -0400 Subject: [PATCH 16/25] Try pinning pip another way Previous commit didn't yield any information because the error happens before the commands are run. --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index 848454333..62547b3df 100644 --- a/tox.ini +++ b/tox.ini @@ -20,6 +20,7 @@ passenv = TAHOE_LAFS_* PIP_* SUBUNITREPORTER_* USERPROFILE HOMEDRIVE HOMEPATH # available to those systems. Installing it ahead of time (with pip) avoids # this problem. deps = + pip<19 certifi subunitreporter # We add usedevelop=True for speed, and extras=test to get things like "mock" From 33bd9bf503ecb1a3f2f204f05c5e5848031e33dc Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 13 May 2019 10:31:46 -0400 Subject: [PATCH 17/25] Get a not-*quite*-so-old version of pip --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 62547b3df..6c7150d9f 100644 --- a/tox.ini +++ b/tox.ini @@ -20,7 +20,7 @@ passenv = TAHOE_LAFS_* PIP_* SUBUNITREPORTER_* USERPROFILE HOMEDRIVE HOMEPATH # available to those systems. Installing it ahead of time (with pip) avoids # this problem. deps = - pip<19 + pip>8.1.1,<19 certifi subunitreporter # We add usedevelop=True for speed, and extras=test to get things like "mock" From f82d74f3274aad521ec2e4f994146156fefe60ea Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 13 May 2019 10:36:35 -0400 Subject: [PATCH 18/25] Dump more tox logs Because we still don't know what versions of python packages are installed --- .appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.appveyor.yml b/.appveyor.yml index 8f1f3e0db..3306d2826 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -50,7 +50,7 @@ test_script: # the interpreter you're using - Appveyor does not do anything special # to put the Python version you want to use on PATH. - | - %PYTHON%\Scripts\tox.exe -e coverage + %PYTHON%\Scripts\tox.exe -vvv -e coverage after_test: # This builds the main tahoe wheel, and wheels for all dependencies. From 27962dcd2f26a1d6628af2e7355c810c95c56c4b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 13 May 2019 11:13:42 -0400 Subject: [PATCH 19/25] Try pinning *new* versions Tox released 4 hours ago ******BROKE****** everything by removing the upgrade-these-packages-automatically feature. Not that I'm bitter. --- tox.ini | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index 6c7150d9f..be50eefcb 100644 --- a/tox.ini +++ b/tox.ini @@ -20,9 +20,11 @@ passenv = TAHOE_LAFS_* PIP_* SUBUNITREPORTER_* USERPROFILE HOMEDRIVE HOMEPATH # available to those systems. Installing it ahead of time (with pip) avoids # this problem. deps = - pip>8.1.1,<19 + pip==19.1.1 + setuptools==41.0.1 + wheel==0.33.4 + subunitreporter==19.3.2 certifi - subunitreporter # We add usedevelop=True for speed, and extras=test to get things like "mock" # that are required for our unit tests. usedevelop = True From 7c71dd6e1412328beaf9f2471ebc35eb94d89762 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 13 May 2019 11:18:39 -0400 Subject: [PATCH 20/25] going to try another fix --- tox.ini | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tox.ini b/tox.ini index be50eefcb..6154c2dbd 100644 --- a/tox.ini +++ b/tox.ini @@ -20,10 +20,7 @@ passenv = TAHOE_LAFS_* PIP_* SUBUNITREPORTER_* USERPROFILE HOMEDRIVE HOMEPATH # available to those systems. Installing it ahead of time (with pip) avoids # this problem. deps = - pip==19.1.1 - setuptools==41.0.1 - wheel==0.33.4 - subunitreporter==19.3.2 + subunitreporter certifi # We add usedevelop=True for speed, and extras=test to get things like "mock" # that are required for our unit tests. From 26dad41c824392eadb4422bb3fb416edd5f1b7e4 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 13 May 2019 11:19:15 -0400 Subject: [PATCH 21/25] Pin the older tox to see if that helps --- .appveyor.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.appveyor.yml b/.appveyor.yml index 3306d2826..a705f4d71 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -13,8 +13,8 @@ environment: install: - | - %PYTHON%\python.exe -m pip install -U "pip<19" - %PYTHON%\python.exe -m pip install wheel tox virtualenv + %PYTHON%\python.exe -m pip install -U pip + %PYTHON%\python.exe -m pip install wheel tox==3.9.0 virtualenv # note: # %PYTHON% has: python.exe From 9bbdbf3fe72f825aee3f26e96b90d2fda232dac8 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 13 May 2019 11:23:04 -0400 Subject: [PATCH 22/25] get rid of this I guess --- .appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.appveyor.yml b/.appveyor.yml index a705f4d71..640701b77 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -50,7 +50,7 @@ test_script: # the interpreter you're using - Appveyor does not do anything special # to put the Python version you want to use on PATH. - | - %PYTHON%\Scripts\tox.exe -vvv -e coverage + %PYTHON%\Scripts\tox.exe -e coverage after_test: # This builds the main tahoe wheel, and wheels for all dependencies. From 1717243212a53707baf10b79714b1c363ae47bfc Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 13 May 2019 11:32:02 -0400 Subject: [PATCH 23/25] Re-pin these because it's nice to know what versions we're using --- tox.ini | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 6154c2dbd..be50eefcb 100644 --- a/tox.ini +++ b/tox.ini @@ -20,7 +20,10 @@ passenv = TAHOE_LAFS_* PIP_* SUBUNITREPORTER_* USERPROFILE HOMEDRIVE HOMEPATH # available to those systems. Installing it ahead of time (with pip) avoids # this problem. deps = - subunitreporter + pip==19.1.1 + setuptools==41.0.1 + wheel==0.33.4 + subunitreporter==19.3.2 certifi # We add usedevelop=True for speed, and extras=test to get things like "mock" # that are required for our unit tests. From aec341eea8ea5cc9b7ab2098334c8a6876fbdb9c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 14 May 2019 03:36:34 -0400 Subject: [PATCH 24/25] explain the pinning in tox deps config --- tox.ini | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tox.ini b/tox.ini index be50eefcb..bab1ac71f 100644 --- a/tox.ini +++ b/tox.ini @@ -20,10 +20,23 @@ passenv = TAHOE_LAFS_* PIP_* SUBUNITREPORTER_* USERPROFILE HOMEDRIVE HOMEPATH # available to those systems. Installing it ahead of time (with pip) avoids # this problem. deps = + # Pin all of these versions for the same reason you ever want to pin + # anything: to prevent new releases with regressions from introducing + # spurious failures into CI runs for whatever development work is + # happening at the time. The versions selected here are just the current + # versions at the time. Bumping them to keep up with future releases is + # fine as long as those releases are known to actually work. pip==19.1.1 setuptools==41.0.1 wheel==0.33.4 subunitreporter==19.3.2 + # As an exception, we don't pin certifi because it contains CA + # certificates which necessarily change over time. Pinning this is + # guaranteed to cause things to break eventually as old certificates + # expire and as new ones are used in the wild that aren't present in + # whatever version we pin. Hopefully there won't be functionality + # regressions in new releases of this package that cause us the kind of + # suffering we're trying to avoid with the above pins. certifi # We add usedevelop=True for speed, and extras=test to get things like "mock" # that are required for our unit tests. From 0294d1440130c70906663e24585b238a9731f25f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 14 May 2019 03:37:35 -0400 Subject: [PATCH 25/25] explain the new `pip freeze` in the coverage commands --- tox.ini | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tox.ini b/tox.ini index bab1ac71f..659574e15 100644 --- a/tox.ini +++ b/tox.ini @@ -54,6 +54,11 @@ commands = [testenv:coverage] # coverage (with --branch) takes about 65% longer to run commands = + # As an aid to debugging, dump all of the Python packages and their + # versions that are installed in the test environment. This is + # particularly useful to get from CI runs - though hopefully the + # version pinning we do limits the variability of this output + # somewhat. pip freeze tahoe --version coverage run --branch -m twisted.trial {env:TAHOE_LAFS_TRIAL_ARGS:--rterrors --reporter=timing} {posargs:allmydata}