Changeset 07ffde3 in trunk

Timestamp:

2021-11-05T15:13:23Z (3 years ago)

Author:

GitHub <noreply@…>

Branches:

master

Children:

68a27b1, 7faec6e5, bea4cf1, fe6a2f6f

Parents:

8383f6a (diff), b4bc95c (diff)
Note: this is a merge changeset, the changes displayed below correspond to the merge itself.
Use the (diff) links above to see all the changes relative to each parent.

git-author:

Jean-Paul Calderone <exarkun@…> (2021-11-05 15:13:23)

git-committer:

GitHub <noreply@…> (2021-11-05 15:13:23)

Message:

Merge pull request #1158 from tahoe-lafs/3838.more-main-tub-cleanup

More "main tub" initialization cleanup in test code

Fixes: ticket:3838

Files:

• newsfragments/3838.minor (added)
• src/allmydata/scripts/common.py (modified) (1 diff)
• src/allmydata/test/common.py (modified) (4 diffs)
• src/allmydata/test/common_system.py (modified) (2 diffs)
• src/allmydata/test/test_client.py (modified) (3 diffs)
• src/allmydata/test/test_node.py (modified) (7 diffs)
• src/allmydata/test/web/test_introducer.py (modified) (1 diff)

src/allmydata/scripts/common.py ¶

@@ -142 +142 @@
     if isinstance(furl, bytes):
         furl = furl.decode("utf-8")
-    basedir.child(b"private").child(b"introducers.yaml").setContent(
+    private = basedir.child(b"private")
+    private.makedirs(ignoreExistingDirectory=True)
+    private.child(b"introducers.yaml").setContent(
         safe_dump({
             "introducers": {

src/allmydata/test/common.py ¶

@@ -268 +268 @@
 
     config = attr.ib(default=None)
+    reactor = attr.ib(default=None)
 
     def setUp(self):
+        self.assigner = SameProcessStreamEndpointAssigner()
+        self.assigner.setUp()
+
         def format_config_items(config):
             return "\n".join(
@@ -293 +297 @@
             self.introducer_furl,
         )
+
+        node_config = self.node_config.copy()
+        if "tub.port" not in node_config:
+            if "tub.location" in node_config:
+                raise ValueError(
+                    "UseNode fixture does not support specifying tub.location "
+                    "without tub.port"
+                )
+
+            # Don't use the normal port auto-assignment logic.  It produces
+            # collisions and makes tests fail spuriously.
+            tub_location, tub_endpoint = self.assigner.assign(self.reactor)
+            node_config.update({
+                "tub.port": tub_endpoint,
+                "tub.location": tub_location,
+            })
+
         self.config = config_from_string(
             self.basedir.asTextMode().path,
@@ -305 +326 @@
 """.format(
     storage_plugin=self.storage_plugin,
-    node_config=format_config_items(self.node_config),
+    node_config=format_config_items(node_config),
     plugin_config_section=plugin_config_section,
 )
@@ -317 +338 @@
 
     def cleanUp(self):
-        pass
+        self.assigner.tearDown()
 
 

src/allmydata/test/common_system.py ¶

@@ -768 +768 @@
         config = {}
 
-        except1 = set(range(self.numclients)) - {1}
+        allclients = set(range(self.numclients))
+        except1 = allclients - {1}
         feature_matrix = {
             ("client", "nickname"): except1,
 
-            # client 1 has to auto-assign an address.
-            ("node", "tub.port"): except1,
-            ("node", "tub.location"): except1,
+            # Auto-assigning addresses is extremely failure prone and not
+            # amenable to automated testing in _this_ manner.
+            ("node", "tub.port"): allclients,
+            ("node", "tub.location"): allclients,
 
             # client 0 runs a webserver and a helper
@@ -856 +858 @@
         basedir = FilePath(self.getdir("client%d" % client_num))
         basedir.makedirs()
-        config = "[client]\n"
+        config = (
+            "[node]\n"
+            "tub.location = {}\n"
+            "tub.port = {}\n"
+            "[client]\n"
+        ).format(*self.port_assigner.assign(reactor))
+
         if helper_furl:
             config += "helper.furl = %s\n" % helper_furl

src/allmydata/test/test_client.py ¶

@@ -90 +90 @@
     MemoryIntroducerClient,
     get_published_announcements,
+    UseNode,
 )
 from .matchers import (
@@ -954 +955 @@
     @defer.inlineCallbacks
     def test_reloadable(self):
-        basedir = FilePath("test_client.Run.test_reloadable")
-        private = basedir.child("private")
-        private.makedirs()
+        from twisted.internet import reactor
+
         dummy = "pb://wl74cyahejagspqgy4x5ukrvfnevlknt@127.0.0.1:58889/bogus"
-        write_introducer(basedir, "someintroducer", dummy)
-        basedir.child("tahoe.cfg").setContent(BASECONFIG. encode("ascii"))
-        c1 = yield client.create_client(basedir.path)
+        fixture = UseNode(None, None, FilePath(self.mktemp()), dummy, reactor=reactor)
+        fixture.setUp()
+        self.addCleanup(fixture.cleanUp)
+
+        c1 = yield fixture.create_node()
         c1.setServiceParent(self.sparent)
 
@@ -982 +984 @@
         # reactor.stop, also instrument the shutdown event in an
         # attribute that we can check.)
-        c2 = yield client.create_client(basedir.path)
+        c2 = yield fixture.create_node()
         c2.setServiceParent(self.sparent)
         yield c2.disownServiceParent()

src/allmydata/test/test_node.py ¶

@@ -70 +70 @@
 from .common import (
     ConstantAddresses,
+    SameProcessStreamEndpointAssigner,
+    UseNode,
 )
 
@@ -81 +83 @@
 
 # see https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2946
-def testing_tub(config_data=''):
+def testing_tub(reactor, config_data=''):
     """
     Creates a 'main' Tub for testing purposes, from config data
     """
-    from twisted.internet import reactor
     basedir = 'dummy_basedir'
     config = config_from_string(basedir, 'DEFAULT_PORTNUMFILE_BLANK', config_data)
@@ -113 +114 @@
         # conflict with another service to prove it.
         self._available_port = 22
+        self.port_assigner = SameProcessStreamEndpointAssigner()
+        self.port_assigner.setUp()
+        self.addCleanup(self.port_assigner.tearDown)
 
     def _test_location(
@@ -138 +142 @@
             supply to the system under test as local addresses.
         """
+        from twisted.internet import reactor
+
         basedir = self.mktemp()
         create_node_dir(basedir, "testing")
+        if tub_port is None:
+            # Always configure a usable tub.port address instead of relying on
+            # the automatic port assignment.  The automatic port assignment is
+            # prone to collisions and spurious test failures.
+            _, tub_port = self.port_assigner.assign(reactor)
+
         config_data = "[node]\n"
-        if tub_port:
-            config_data += "tub.port = {}\n".format(tub_port)
+        config_data += "tub.port = {}\n".format(tub_port)
+
+        # If they wanted a certain location, go for it.  This probably won't
+        # agree with the tub.port value we set but that only matters if
+        # anything tries to use this to establish a connection ... which
+        # nothing in this test suite will.
         if tub_location is not None:
             config_data += "tub.location = {}\n".format(tub_location)
@@ -150 +166 @@
                        lambda: local_addresses)
 
-        tub = testing_tub(config_data)
+        tub = testing_tub(reactor, config_data)
 
         class Foo(object):
@@ -432 +448 @@
     @defer.inlineCallbacks
     def test_logdir_is_str(self):
-        basedir = "test_node/test_logdir_is_str"
+        from twisted.internet import reactor
+
+        basedir = FilePath(self.mktemp())
+        fixture = UseNode(None, None, basedir, "pb://introducer/furl", {}, reactor=reactor)
+        fixture.setUp()
+        self.addCleanup(fixture.cleanUp)
 
         ns = Namespace()
@@ -441 +462 @@
         self.patch(foolscap.logging.log, 'setLogDir', call_setLogDir)
 
-        create_node_dir(basedir, "nothing to see here")
-        yield client.create_client(basedir)
+        yield fixture.create_node()
         self.failUnless(ns.called)
 

src/allmydata/test/web/test_introducer.py ¶

@@ -84 +84 @@
     """
     node.create_node_dir(basedir, "testing")
-    _, port_endpoint = port_assigner.assign(reactor)
+    main_tub_location, main_tub_endpoint = port_assigner.assign(reactor)
+    _, web_port_endpoint = port_assigner.assign(reactor)
     with open(join(basedir, "tahoe.cfg"), "w") as f:
         f.write(
             "[node]\n"
-            "tub.location = 127.0.0.1:1\n" +
-            "web.port = {}\n".format(port_endpoint)
+            "tub.port = {main_tub_endpoint}\n"
+            "tub.location = {main_tub_location}\n"
+            "web.port = {web_port_endpoint}\n".format(
+                main_tub_endpoint=main_tub_endpoint,
+                main_tub_location=main_tub_location,
+                web_port_endpoint=web_port_endpoint,
+            )
         )