pytest: close file handles after use (cont.), and tidy-ups

- dante.py, dnsd.py, sshd.py: drop redundant conditions.
  Spotted in sshd by GitHub Code Quality.
- curl.py: comment out `if` to silence CodeQL warning.

Reported by GitHub CodeQL

Follow-up to 8145476d5dd97d0ec704e9ea65b2f2028b8a945c #21916

Closes #21917

diff --git a/tests/http/testenv/caddy.py b/tests/http/testenv/caddy.py
index eece1a5a394af772cf8e4cd446a2fadbbd7a1ce4..1d554b1906f43ebc53b293b11adce2b2f185b6cd 100644 (file)
--- a/tests/http/testenv/caddy.py
+++ b/tests/http/testenv/caddy.py
@@ -55,6 +55,7 @@ class Caddy:
         self._conf_file = os.path.join(self._caddy_dir, 'Caddyfile')
         self._error_log = os.path.join(self._caddy_dir, 'caddy.log')
         self._tmp_dir = os.path.join(self._caddy_dir, 'tmp')
+        self._error_fd = None
         self._process = None
         self._http_port = 0
         self._https_port = 0
@@ -68,6 +69,11 @@ class Caddy:
     def port(self) -> int:
         return self._https_port
 
+    def close_log(self):
+        if self._error_fd:
+            self._error_fd.close()
+            self._error_fd = None
+
     def clear_logs(self):
         self._rmf(self._error_log)
 
@@ -107,8 +113,8 @@ class Caddy:
         args = [
             self._caddy, 'run'
         ]
-        caddyerr = open(self._error_log, 'a')
-        self._process = subprocess.Popen(args=args, cwd=self._caddy_dir, stderr=caddyerr)
+        self._error_fd = open(self._error_log, 'a')
+        self._process = subprocess.Popen(args=args, cwd=self._caddy_dir, stderr=self._error_fd)
         if self._process.returncode is not None:
             return False
         return not wait_live or self.wait_live(timeout=timedelta(seconds=Env.SERVER_TIMEOUT))
@@ -122,7 +128,9 @@ class Caddy:
             except Exception:
                 self._process.kill()
             self._process = None
+            self.close_log()
             return not wait_dead or self.wait_dead(timeout=timedelta(seconds=5))
+        self.close_log()
         return True
 
     def restart(self):

diff --git a/tests/http/testenv/client.py b/tests/http/testenv/client.py
index 1b4e303ac6095a430df5088fbb005a6ccde9668d..76c4822cbc8d186cec2fc76aed51a95d1d8fea98 100644 (file)
--- a/tests/http/testenv/client.py
+++ b/tests/http/testenv/client.py
@@ -114,10 +114,10 @@ class LocalClient:
     def dump_logs(self):
         lines = []
         lines.append('>>--stdout ----------------------------------------------\n')
-        with open(self._stdoutfile) as cstdout:
-            lines.extend(cstdout.readlines())
+        with open(self._stdoutfile) as fd:
+            lines.extend(fd.readlines())
         lines.append('>>--stderr ----------------------------------------------\n')
-        with open(self._stderrfile) as cstderr:
-            lines.extend(cstderr.readlines())
+        with open(self._stderrfile) as fd:
+            lines.extend(fd.readlines())
         lines.append('<<-------------------------------------------------------\n')
         return ''.join(lines)

diff --git a/tests/http/testenv/curl.py b/tests/http/testenv/curl.py
index 0864e2899eb5007ef2dd7c20372c58b1af4f6715..308e29f110671a6ce23b32002a74c4324116ae33 100644 (file)
--- a/tests/http/testenv/curl.py
+++ b/tests/http/testenv/curl.py
@@ -191,13 +191,14 @@ class RunTcpDump:
         if self._proc:
             raise Exception('tcpdump still running')
         lines = []
-        for line in open(self._stdoutfile):
-            m = re.match(r'.* IP 127\.0\.0\.1\.(\d+) [<>] 127\.0\.0\.1\.(\d+):.*', line)
-            if m:
-                sport = int(m.group(1))
-                dport = int(m.group(2))
-                if ports is None or sport in ports or dport in ports:
-                    lines.append(line)
+        with open(self._stdoutfile) as fd:
+            for line in fd:
+                m = re.match(r'.* IP 127\.0\.0\.1\.(\d+) [<>] 127\.0\.0\.1\.(\d+):.*', line)
+                if m:
+                    sport = int(m.group(1))
+                    dport = int(m.group(2))
+                    if ports is None or sport in ports or dport in ports:
+                        lines.append(line)
         return lines
 
     @property
@@ -208,7 +209,8 @@ class RunTcpDump:
     def stderr(self) -> List[str]:
         if self._proc:
             raise Exception('tcpdump still running')
-        return open(self._stderrfile).readlines()
+        with open(self._stderrfile) as fd:
+            return fd.readlines()
 
     def sample(self):
         # not sure how to make that detection reliable for all platforms
@@ -1027,8 +1029,8 @@ class CurlClient:
                                          cwd=self._run_dir, shell=False,
                                          env=self._run_env)
                     profile = RunProfile(p.pid, started_at, self._run_dir)
-                    if intext is not None and False:
-                        p.communicate(input=intext.encode(), timeout=1)
+                    #if intext is not None and False:
+                    #    p.communicate(input=intext.encode(), timeout=1)
                     if self._with_perf:
                         perf = PerfProfile(p.pid, self._run_dir)
                         perf.start()

diff --git a/tests/http/testenv/dante.py b/tests/http/testenv/dante.py
index 2feb42eb7ccaa0866be7d0aece87ea6a089c5b9c..b2ea4dd0cfe4733b254be1423892e009e493881d 100644 (file)
--- a/tests/http/testenv/dante.py
+++ b/tests/http/testenv/dante.py
@@ -57,6 +57,7 @@ class Dante:
         self._dante_log = os.path.join(self._dante_dir, 'dante.log')
         self._error_log = os.path.join(self._dante_dir, 'error.log')
         self._pid_file = os.path.join(self._dante_dir, 'dante.pid')
+        self._error_fd = None
         self._process = None
 
         self.clear_logs()
@@ -65,6 +66,11 @@ class Dante:
     def port(self) -> int:
         return self._port
 
+    def close_log(self):
+        if self._error_fd:
+            self._error_fd.close()
+            self._error_fd = None
+
     def clear_logs(self):
         self._rmf(self._error_log)
         self._rmf(self._dante_log)
@@ -89,7 +95,7 @@ class Dante:
             self._process.terminate()
             self._process.wait(timeout=2)
             self._process = None
-            return not wait_dead or True
+        self.close_log()
         return True
 
     def restart(self):
@@ -122,8 +128,8 @@ class Dante:
             '-p', f'{self._pid_file}',
             '-d', '0',
         ]
-        procerr = open(self._error_log, 'a')
-        self._process = subprocess.Popen(args=args, stderr=procerr)
+        self._error_fd = open(self._error_log, 'a')
+        self._process = subprocess.Popen(args=args, stderr=self._error_fd)
         if self._process.returncode is not None:
             return False
         return self.wait_live(timeout=timedelta(seconds=Env.SERVER_TIMEOUT))

diff --git a/tests/http/testenv/dnsd.py b/tests/http/testenv/dnsd.py
index b8ff097280a33ca184035444d33da6a558bfb27b..a7dc0885571fea6e8b94a436891f9998beb08661 100644 (file)
--- a/tests/http/testenv/dnsd.py
+++ b/tests/http/testenv/dnsd.py
@@ -56,6 +56,7 @@ class Dnsd:
         self._conf_file = os.path.join(self._log_dir, 'dnsd.cmd')
         self._pid_file = os.path.join(self._log_dir, 'dnsd.pid')
         self._error_log = os.path.join(self._log_dir, 'dnsd.err.log')
+        self._error_fd = None
         self._process = None
 
         self.clear_logs()
@@ -64,6 +65,11 @@ class Dnsd:
     def port(self) -> int:
         return self._port
 
+    def close_log(self):
+        if self._error_fd:
+            self._error_fd.close()
+            self._error_fd = None
+
     def clear_logs(self):
         self._rmf(self._log_file)
         self._rmf(self._error_log)
@@ -87,7 +93,7 @@ class Dnsd:
             self._process.terminate()
             self._process.wait(timeout=2)
             self._process = None
-            return not wait_dead or True
+        self.close_log()
         return True
 
     def restart(self):
@@ -122,8 +128,8 @@ class Dnsd:
             '--logfile', f'{self._log_file}',
             '--pidfile', f'{self._pid_file}',
         ]
-        procerr = open(self._error_log, 'a')
-        self._process = subprocess.Popen(args=args, stderr=procerr)
+        self._error_fd = open(self._error_log, 'a')
+        self._process = subprocess.Popen(args=args, stderr=self._error_fd)
         if self._process.returncode is not None:
             return False
         return self.wait_live(timeout=timedelta(seconds=Env.SERVER_TIMEOUT))

diff --git a/tests/http/testenv/h2o.py b/tests/http/testenv/h2o.py
index 279cff8b90c3b2cb0dbde2ac9ef81ec56973c40e..deb3608d3a8cccd9cde962cac74c9100960dd359 100644 (file)
--- a/tests/http/testenv/h2o.py
+++ b/tests/http/testenv/h2o.py
@@ -48,6 +48,7 @@ class H2o:
         self._port = 0  # defaults to h3_port
         self._cred_name = cred_name
         self._loaded_cred_name = None
+        self._error_fd = None
         self._process = None
         self._tmp_dir = os.path.join(self.env.gen_dir, self._name)
         self._run_dir = os.path.join(self._tmp_dir, "run")
@@ -72,6 +73,11 @@ class H2o:
     def h2_port(self) -> Optional[int]:
         return getattr(self, "_h2_port", None)
 
+    def close_log(self):
+        if self._error_fd:
+            self._error_fd.close()
+            self._error_fd = None
+
     def clear_logs(self):
         self._rmf(self._error_log)
         self._rmf(self._stderr)
@@ -127,8 +133,8 @@ class H2o:
         self._loaded_cred_name = self._cred_name
         self.write_config()
         args = [self._cmd, "-c", self._conf_file]
-        ngerr = open(self._stderr, "a")
-        self._process = subprocess.Popen(args=args, stderr=ngerr)
+        self._error_fd = open(self._stderr, "a")
+        self._process = subprocess.Popen(args=args, stderr=self._error_fd)
         if self._process.returncode is not None:
             return False
         if wait_live:
@@ -155,15 +161,19 @@ class H2o:
                 self._process.kill()
                 self._process.wait(timeout=2)
             self._process = None
+            self.close_log()
             return not wait_dead or self.wait_for_state(
                 live=False, timeout=timedelta(seconds=5)
             )
+        self.close_log()
         return True
 
     def kill(self, wait_dead=True):
         if self._process:
             self._process.kill()
+            self.close_log()
             return True
+        self.close_log()
         return False
 
     def restart(self):

diff --git a/tests/http/testenv/nghttpx.py b/tests/http/testenv/nghttpx.py
index c72a7f7f6de302eef7c8f063ddc360b3e3be64b0..37c72ad9451fa349f56dd4f77d8cd4f60671f408 100644 (file)
--- a/tests/http/testenv/nghttpx.py
+++ b/tests/http/testenv/nghttpx.py
@@ -55,6 +55,7 @@ class Nghttpx:
         self._error_log = os.path.join(self._run_dir, 'nghttpx.log')
         self._stderr = os.path.join(self._run_dir, 'nghttpx.stderr')
         self._tmp_dir = os.path.join(self._run_dir, 'tmp')
+        self._error_fd = None
         self._process: Optional[subprocess.Popen] = None
         self._cred_name = self._def_cred_name = cred_name
         self._loaded_cred_name = ''
@@ -86,6 +87,11 @@ class Nghttpx:
     def exists(self):
         return self._cmd and os.path.exists(self._cmd)
 
+    def close_log(self):
+        if self._error_fd:
+            self._error_fd.close()
+            self._error_fd = None
+
     def clear_logs(self):
         self._rmf(self._error_log)
         self._rmf(self._stderr)
@@ -116,7 +122,9 @@ class Nghttpx:
             self._process.terminate()
             self._process.wait(timeout=2)
             self._process = None
+            self.close_log()
             return not wait_dead or self.wait_dead(timeout=timedelta(seconds=5))
+        self.close_log()
         return True
 
     def restart(self):
@@ -262,8 +270,8 @@ class NghttpxQuic(Nghttpx):
             '--frontend-http3-max-connection-window-size=100M',
             # f'--frontend-quic-debug-log',
         ])
-        ngerr = open(self._stderr, 'a')
-        self._process = subprocess.Popen(args=args, stderr=ngerr)
+        self._error_fd = open(self._stderr, 'a')
+        self._process = subprocess.Popen(args=args, stderr=self._error_fd)
         if self._process.returncode is not None:
             return False
         return not wait_live or self.wait_live(timeout=timedelta(seconds=Env.SERVER_TIMEOUT))
@@ -312,8 +320,8 @@ class NghttpxFwd(Nghttpx):
             creds.pkey_file,
             creds.cert_file,
         ]
-        ngerr = open(self._stderr, 'a')
-        self._process = subprocess.Popen(args=args, stderr=ngerr)
+        self._error_fd = open(self._stderr, 'a')
+        self._process = subprocess.Popen(args=args, stderr=self._error_fd)
         if self._process.returncode is not None:
             return False
         return not wait_live or self.wait_live(timeout=timedelta(seconds=Env.SERVER_TIMEOUT))

diff --git a/tests/http/testenv/sshd.py b/tests/http/testenv/sshd.py
index 83accd74920ba2dbb13aae8751664ff2480346f0..de8078dd265c5a1b1e674fe075cc4213cfc5fb52 100644 (file)
--- a/tests/http/testenv/sshd.py
+++ b/tests/http/testenv/sshd.py
@@ -74,6 +74,7 @@ class Sshd:
         ]
         self._user_key_files = []
         self._user_pub_files = []
+        self._error_fd = None
         self._process = None
 
         self.clear_logs()
@@ -164,14 +165,21 @@ class Sshd:
                 pubkey = fp.read()
             fd.write(pubkey)
 
+    def close_log(self):
+        if self._error_fd:
+            self._error_fd.close()
+            self._error_fd = None
+
     def clear_logs(self):
         self._rmf(self._sshd_log)
 
     def dump_log(self):
         lines = ['>>--sshd log ----------------------------------------------\n']
-        lines.extend(open(self._sshd_log))
+        with open(self._sshd_log) as fd:
+            lines.extend(fd.readlines())
         lines.extend(['>>--curl log ----------------------------------------------\n'])
-        lines.extend(open(os.path.join(self._tmp_dir, 'curl.stderr')))
+        with open(os.path.join(self._tmp_dir, 'curl.stderr')) as fd:
+            lines.extend(fd.readlines())
         lines.append('<<-------------------------------------------------------\n')
         return ''.join(lines)
 
@@ -195,7 +203,7 @@ class Sshd:
             self._process.terminate()
             self._process.wait(timeout=2)
             self._process = None
-            return not wait_dead or True
+        self.close_log()
         return True
 
     def restart(self):
@@ -233,8 +241,8 @@ class Sshd:
         run_env = os.environ.copy()
         # does not have any effect, sadly
         # run_env['HOME'] = f'{self._home_dir}'
-        procerr = open(self._sshd_log, 'a')
-        self._process = subprocess.Popen(args=args, stderr=procerr, env=run_env)
+        self._error_fd = open(self._sshd_log, 'a')
+        self._process = subprocess.Popen(args=args, stderr=self._error_fd, env=run_env)
         if self._process.returncode is not None:
             return False
         return self.wait_live(timeout=timedelta(seconds=Env.SERVER_TIMEOUT))

diff --git a/tests/http/testenv/vsftpd.py b/tests/http/testenv/vsftpd.py
index ace2884c7d3529f2036c016fe50bd216ebfb3046..6c2d1db8c5f9a3b1ca9a9624ef92cc95225490d1 100644 (file)
--- a/tests/http/testenv/vsftpd.py
+++ b/tests/http/testenv/vsftpd.py
@@ -68,6 +68,7 @@ class VsFTPD:
         self._conf_file = os.path.join(self._vsftpd_dir, 'test.conf')
         self._pid_file = os.path.join(self._vsftpd_dir, 'vsftpd.pid')
         self._error_log = os.path.join(self._vsftpd_dir, 'vsftpd.log')
+        self._error_fd = None
         self._process = None
 
         self.clear_logs()
@@ -84,6 +85,11 @@ class VsFTPD:
     def port(self) -> int:
         return self._port
 
+    def close_log(self):
+        if self._error_fd:
+            self._error_fd.close()
+            self._error_fd = None
+
     def clear_logs(self):
         self._rmf(self._error_log)
 
@@ -107,7 +113,9 @@ class VsFTPD:
             self._process.terminate()
             self._process.wait(timeout=2)
             self._process = None
+            self.close_log()
             return not wait_dead or self.wait_dead(timeout=timedelta(seconds=5))
+        self.close_log()
         return True
 
     def restart(self):
@@ -138,8 +146,8 @@ class VsFTPD:
             self._cmd,
             f'{self._conf_file}',
         ]
-        procerr = open(self._error_log, 'a')
-        self._process = subprocess.Popen(args=args, stderr=procerr)
+        self._error_fd = open(self._error_log, 'a')
+        self._process = subprocess.Popen(args=args, stderr=self._error_fd)
         if self._process.returncode is not None:
             return False
         return not wait_live or self.wait_live(timeout=timedelta(seconds=Env.SERVER_TIMEOUT))