Refactor shutdown test into more helper functions

Improve code readability by splitting the test into more functions. Some
could be re-used later on for more general-purpose subprocess handling
or named checks.

(cherry picked from commit 9d64f1c1edf66e907c0e83f8b38a2e31b970d429)

diff --git a/bin/tests/system/shutdown/tests_shutdown.py b/bin/tests/system/shutdown/tests_shutdown.py
index 67a2374e778230e39d86acd10e588af663ac515a..8e31f8fc460e46eeb04790f3256c99c019508568 100755 (executable)
--- a/bin/tests/system/shutdown/tests_shutdown.py
+++ b/bin/tests/system/shutdown/tests_shutdown.py
@@ -131,6 +131,31 @@ def do_work(named_proc, resolver, rndc_cmd, kill_method, n_workers, n_queries):
             assert ret_code == 0
 
 
+def wait_for_named_loaded(resolver, retries=10):
+    for _ in range(retries):
+        try:
+            resolver.query("version.bind", "TXT", "CH")
+            return True
+        except (dns.resolver.NoNameservers, dns.exception.Timeout):
+            time.sleep(1)
+    return False
+
+
+def wait_for_proc_termination(proc, max_timeout=10):
+    for _ in range(max_timeout):
+        if proc.poll() is not None:
+            return True
+        time.sleep(1)
+
+    proc.send_signal(signal.SIGABRT)
+    for _ in range(max_timeout):
+        if proc.poll() is not None:
+            return True
+        time.sleep(1)
+
+    return False
+
+
 def test_named_shutdown(named_port, control_port):
     # pylint: disable-msg=too-many-locals
     cfg_dir = os.path.join(os.getcwd(), "resolver")
@@ -167,40 +192,15 @@ def test_named_shutdown(named_port, control_port):
     for kill_method in ("rndc", "sigterm"):
         named_cmdline = [named, "-c", cfg_file, "-f"]
         with subprocess.Popen(named_cmdline, cwd=cfg_dir) as named_proc:
-            # Ensure named is running
-            assert named_proc.poll() is None
-            # wait for named to finish loading
-            for _ in range(10):
-                try:
-                    resolver.query("version.bind", "TXT", "CH")
-                    break
-                except (dns.resolver.NoNameservers, dns.exception.Timeout):
-                    time.sleep(1)
-
+            assert named_proc.poll() is None, "named isn't running"
+            assert wait_for_named_loaded(resolver)
             do_work(
-                named_proc, resolver, rndc_cmd, kill_method, n_workers=12, n_queries=16
+                named_proc,
+                resolver,
+                rndc_cmd,
+                kill_method,
+                n_workers=12,
+                n_queries=16,
             )
-
-            # Wait named to exit for a maximum of MAX_TIMEOUT seconds.
-            MAX_TIMEOUT = 10
-            is_dead = False
-            for _ in range(MAX_TIMEOUT):
-                if named_proc.poll() is not None:
-                    is_dead = True
-                    break
-                time.sleep(1)
-
-            if not is_dead:
-                named_proc.send_signal(signal.SIGABRT)
-                for _ in range(MAX_TIMEOUT):
-                    if named_proc.poll() is not None:
-                        is_dead = True
-                        break
-                    time.sleep(1)
-                if not is_dead:
-                    named_proc.kill()
-
-            assert is_dead
-            # Ensures that named exited gracefully.
-            # If it crashed (abort()) exitcode will be non zero.
-            assert named_proc.returncode == 0
+            assert wait_for_proc_termination(named_proc)
+            assert named_proc.returncode == 0, "named crashed"