#12018 Fix flaky twisted.test.test_twistd tests. (#12019)

twisted · Oct 31, 2023 · ca3f79e · ca3f79e
2 parents 95c6776 + 607b6e7
commit ca3f79e
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 13 deletions.
diff --git a/src/twisted/newsfragments/12018.misc b/src/twisted/newsfragments/12018.misc
diff --git a/src/twisted/test/test_twistd.py b/src/twisted/test/test_twistd.py
@@ -972,7 +972,7 @@ def switchUIDFail(uid, gid, euid):
         exc = self.assertRaises(SystemExit, runner.shedPrivileges, 35, 200, None)
         self.assertEqual(exc.code, 1)
 
-    def _setUID(self, wantedUser, wantedUid, wantedGroup, wantedGid):
+    def _setUID(self, wantedUser, wantedUid, wantedGroup, wantedGid, pidFile):
         """
         Common code for tests which try to pass the the UID to
         L{UnixApplicationRunner}.
@@ -994,7 +994,9 @@ def setgid(gid):
         self.patch(os, "setgid", setgid)
 
         options = twistd.ServerOptions()
-        options.parseOptions(["--nodaemon", "--uid", str(wantedUid)])
+        options.parseOptions(
+            ["--nodaemon", "--uid", str(wantedUid), "--pidfile", pidFile]
+        )
         application = service.Application("test_setupEnvironment")
         self.runner = UnixApplicationRunner(options)
         runner = UnixApplicationRunner(options)
@@ -1006,29 +1008,27 @@ def test_setUidWithoutGid(self):
         with a UID and no GUID will result in the GUID being
         set to the default GUID for that UID.
         """
-        self._setUID("foo", 5151, "bar", 4242)
+        self._setUID(
+            "foo", 5151, "bar", 4242, self.mktemp() + "_test_setUidWithoutGid.pid"
+        )
 
     def test_setUidSameAsCurrentUid(self):
         """
         If the specified UID is the same as the current UID of the process,
         then a warning is displayed.
         """
-
-        # FIXME:https://github.com/twisted/twisted/issues/10332
-        # Assert that there were no existing warnings.
-        existing_warnings = self.flushWarnings()
-        self.assertEqual([], existing_warnings)
-
         currentUid = os.getuid()
-        self._setUID("morefoo", currentUid, "morebar", 4343)
+        self._setUID(
+            "morefoo", currentUid, "morebar", 4343, "test_setUidSameAsCurrentUid.pid"
+        )
 
         warningsShown = self.flushWarnings()
-        self.assertEqual(1, len(warningsShown))
         expectedWarning = (
             "tried to drop privileges and setuid {} but uid is already {}; "
             "should we be root? Continuing.".format(currentUid, currentUid)
         )
         self.assertEqual(expectedWarning, warningsShown[0]["message"])
+        self.assertEqual(1, len(warningsShown), warningsShown)
 
 
 @skipIf(not _twistd_unix, "twistd unix not available")
@@ -1618,7 +1618,6 @@ def test_unmarkedObserversDeprecated(self):
         self.assertIn("starting up", textFromEventDict(logs[0]))
 
         warnings = self.flushWarnings([self.test_unmarkedObserversDeprecated])
-        self.assertEqual(len(warnings), 1, warnings)
         self.assertEqual(
             warnings[0]["message"],
             (
@@ -1632,6 +1631,7 @@ def test_unmarkedObserversDeprecated(self):
                 "implementing objects instead."
             ),
         )
+        self.assertEqual(len(warnings), 1, warnings)
 
 
 @skipIf(not _twistd_unix, "twistd unix not available")
@@ -1739,7 +1739,8 @@ def test_getLogObserverDefaultFile(self):
         """
         logFiles = _patchTextFileLogObserver(self.patch)
         logger = UnixAppLogger({"logfile": "", "nodaemon": False})
-        logger._getLogObserver()
+        observer = logger._getLogObserver()
+        self.addCleanup(observer._outFile.close)
 
         self.assertEqual(len(logFiles), 1)
         self.assertEqual(logFiles[0].path, os.path.abspath("twistd.log"))