Invoke pytest runner from run.sh

Utilize developers' muscle memory to incentivize using the pytest runner
instead of the legacy one. The script also serves as basic examples of
how to run the pyest command to achieve the same results as the legacy
runner.

Invoking pytest directly should be the end goal, since it offers many
potentially useful options (refer to pytest --help).

Author: nicki-krizek

bin/tests/system/.gitignore

@@ -14,7 +14,7 @@ named.run
 /*.log
 /*.trs
 /resolve
-/run.sh
+/legacy.run.sh
 /run.log
 /start.sh
 /stop.sh

bin/tests/system/Makefile.am

@@ -230,9 +230,9 @@ LOG_DRIVER_V_1 = --verbose yes
 LOG_DRIVER = $(srcdir)/custom-test-driver
 AM_LOG_DRIVER_FLAGS = $(LOG_DRIVER_V)
 
-LOG_COMPILER = $(builddir)/run.sh
+LOG_COMPILER = $(builddir)/legacy.run.sh
 AM_LOG_FLAGS = -r
 
-$(TESTS): run.sh
+$(TESTS): legacy.run.sh
 
 test-local: check

bin/tests/system/README

@@ -119,11 +119,11 @@ Running an Individual Test
 ---
 The tests can be run individually using the following command:
 
-    sh run.sh [flags] <test-name> [<test-arguments>]
+    sh legacy.run.sh [flags] <test-name> [<test-arguments>]
 
 e.g.
 
-    sh run.sh [flags] notify
+    sh legacy.run.sh [flags] notify
 
 Optional flags are:
 
@@ -241,9 +241,9 @@ Re-Running the Tests
 ---
 If there is a requirement to re-run a test (or the entire test suite), the
 files produced by the tests should be deleted first.  Normally, these files are
-deleted if the test succeeds but are retained on error.  The run.sh script
-automatically calls a given test's clean.sh script before invoking its setup.sh
-script.
+deleted if the test succeeds but are retained on error.  The legacy.run.sh
+script automatically calls a given test's clean.sh script before invoking its
+setup.sh script.
 
 Deletion of the files produced by the set of tests (e.g. after the execution of
 make) can be carried out using the command:
@@ -283,8 +283,8 @@ tests.sh    Runs the actual tests.  This file is mandatory.
 
 clean.sh    Run at the end to clean up temporary files, but only if the test
             was completed successfully and its running was not inhibited by the
-	    "-n" switch being passed to "run.sh".  Otherwise the temporary
-	    files are left in place for inspection.
+            "-n" switch being passed to "legacy.run.sh".  Otherwise the
+            temporary files are left in place for inspection.
 
 ns<N>       These subdirectories contain test name servers that can be queried
 	    or can interact with each other.  The value of N indicates the
@@ -303,8 +303,8 @@ ans<N>      Like ns[X], but these are simple mock name servers implemented in
 Port Usage
 ---
 In order for the tests to run in parallel, each test requires a unique set of
-ports.  These are specified by the "-p" option passed to "run.sh", which sets
-environment variables that the scripts listed above can reference.
+ports.  These are specified by the "-p" option passed to "legacy.run.sh", which
+sets environment variables that the scripts listed above can reference.
 
 The convention used in the system tests is that the number passed is the start
 of a range of 100 ports.  The test is free to use the ports as required,
@@ -356,10 +356,10 @@ General
 directory.
 
 2. Arguments can be only passed to the script if the test is being run as a
-one-off with "run.sh".  In this case, everything on the command line after the
-name of the test is passed to each script.  For example, the command:
+one-off with "legacy.run.sh".  In this case, everything on the command line
+after the name of the test is passed to each script.  For example, the command:
 
-    sh run.sh -p 12300 mytest -D xyz
+    sh legacy.run.sh -p 12300 mytest -D xyz
 
 ... will run "mytest" with a port range of 12300 to 12399.  Each of the
 framework scripts provided by the test will be invoked using the remaining
@@ -530,8 +530,8 @@ Ideally, the directory numbers should start at 1 and work upwards.
 When running a test, the servers are started using "start.sh" (which is nothing
 more than a wrapper for start.pl).  The options for "start.pl" are documented
 in the header for that file, so will not be repeated here.  In summary, when
-invoked by "run.sh", start.pl looks for directories named "nsN" or "ansN" in
-the test directory and starts the servers it finds there.
+invoked by "legacy.run.sh", start.pl looks for directories named "nsN" or
+"ansN" in the test directory and starts the servers it finds there.
 
 
 "named" Command-Line Options
@@ -628,8 +628,8 @@ the options available are listed in the file's header and will not be repeated
 here.
 
 In summary though, the nameservers for a given test, if left running by
-specifying the "-k" flag to "run.sh" when the test is started, can be stopped
-by the command:
+specifying the "-k" flag to "legacy.run.sh" when the test is started, can be
+stopped by the command:
 
     sh stop.sh <test-name> [server]
 
@@ -738,9 +738,10 @@ This section is aimed at developers maintaining BIND's system test framework.
 
 Notes on Parallel Execution
 ---
-Although execution of an individual test is controlled by "run.sh", which
-executes the above shell scripts (and starts the relevant servers) for each
-test, the running of all tests in the test suite is controlled by the Makefile.
+Although execution of an individual test is controlled by "legacy.run.sh",
+which executes the above shell scripts (and starts the relevant servers) for
+each test, the running of all tests in the test suite is controlled by the
+Makefile.
 
 All system tests are capable of being run in parallel.  For this to work, each
 test needs to use a unique set of ports.  To avoid the need to define which
@@ -763,7 +764,7 @@ If the test fails, all these files are retained.  But if the test succeeds,
 they are cleaned up at different times:
 
 1. Files generated by the test itself are cleaned up by the test's own
-"clean.sh", which is called from "run.sh".
+"clean.sh", which is called from "legacy.run.sh".
 
 2. Files that may not be cleaned up if named exits abnormally can be removed
 using the "cleanall.sh" script.

bin/tests/system/conf.sh.common

@@ -695,7 +695,7 @@ get_named_xfer_stats() {
 #
 # Convenience function to copy a configuration file, replacing the tokens
 # QUERYPORT, CONTROLPORT and EXTRAPORT[1-8] with the values of the equivalent
-# environment variables. (These values are set by "run.sh", which calls the
+# environment variables. (These values are set by test runner, which calls the
 # scripts invoking this function.)
 #
 # Usage:

bin/tests/system/get_algorithms.py

@@ -230,7 +230,7 @@ def main():
     except Exception:
         # if anything goes wrong, the conf.sh ignores error codes, so make sure
         # we set an environment variable to an error value that can be checked
-        # later by run.sh
+        # later by the test runner and/or tests themselves
         print("export ALGORITHM_SET=error")
         raise
     else:

bin/tests/system/run.sh.in renamed to bin/tests/system/legacy.run.sh.in

@@ -0,0 +1,87 @@
+#!/usr/bin/python3
+#
+# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+#
+# SPDX-License-Identifier: MPL-2.0
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, you can obtain one at https://mozilla.org/MPL/2.0/.
+#
+# See the COPYRIGHT file distributed with this work for additional
+# information regarding copyright ownership.
+
+#
+# Run system test using the pytest runner. This is a simple wrapper around
+# pytest for convenience.
+#
+
+import argparse
+import sys
+import time
+
+import pytest
+
+
+def into_pytest_args(in_args):
+    args = []
+    if in_args.expression is None:
+        # running all tests - execute in parallel
+        args.extend(["-n", "auto"])
+        args.extend(["--dist", "loadscope"])
+    else:
+        args.extend(["-k", in_args.expression])
+    if in_args.noclean:
+        args.append("--noclean")
+    if in_args.keep:
+        print(
+            "ERROR -k / --keep option not implemented.\n"
+            "Please contact QA with your use-case and use ./legacy.run.sh in the meantime."
+        )
+        sys.exit(1)
+    return args
+
+
+def main():
+    print(
+        "----- WARNING -----\n"
+        "Using pytest system test runner\n\n"
+        'Please consider invoking "pytest" directly for more control:\n'
+        "  single test:     pytest -k dns64\n"
+        "  parallel tests:  pytest -n auto --dist loadscope\n\n"
+        "Alternately, use ./legacy.run.sh for the legacy system test runner.\n"
+    )
+
+    parser = argparse.ArgumentParser(
+        description="Wrapper script for launching system tests"
+    )
+    parser.add_argument(
+        "--noclean",
+        action="store_true",
+        help="don't clean tmpdir after test run",
+    )
+    parser.add_argument(
+        "-k",
+        "--keep",
+        action="store_true",
+        help="unused - not implemented",
+    )
+    parser.add_argument(
+        "expression",
+        type=str,
+        nargs="?",
+        help="select which test(s) to run",
+    )
+
+    args = into_pytest_args(parser.parse_args())
+    print(f"$ pytest {' '.join(args)}\n" "---------------------------\n")
+
+    time.sleep(2)  # force the user to stare at the warning message
+
+    sys.exit(pytest.main(args))
+
+
+if __name__ == "__main__":
+    main()
+
+# vim: set filetype=python :

bin/tests/system/run.sh

@@ -1624,8 +1624,8 @@ AC_CONFIG_FILES([bin/tests/Makefile
 
 AC_CONFIG_FILES([bin/tests/system/ifconfig.sh],
 		[chmod +x bin/tests/system/ifconfig.sh])
-AC_CONFIG_FILES([bin/tests/system/run.sh],
-		[chmod +x bin/tests/system/run.sh])
+AC_CONFIG_FILES([bin/tests/system/legacy.run.sh],
+		[chmod +x bin/tests/system/legacy.run.sh])
 AC_CONFIG_FILES([bin/tests/system/start.sh],
 		[chmod +x bin/tests/system/start.sh])
 AC_CONFIG_FILES([bin/tests/system/stop.sh],

configure.ac

@@ -432,7 +432,7 @@ into 'consumed' and 'remaining'.
 
 When parsing a message, the message to be parsed in in the 'used'
 part of the buffer.  As the message is parsed, the 'consumed'
-subregion grows and the 'remaining' subregion shrinks. 
+subregion grows and the 'remaining' subregion shrinks.
 
 When creating a message, data is written into the 'available'
 subregion, which then becomes part of 'used'.
@@ -533,7 +533,7 @@ memory context is freed before all references have been cleaned up.
                 /* Populate other isc_foo members here */
 
                 foo->magic = ISC_FOO_MAGIC;
-                
+
                 *foop = foo;
                 return (ISC_R_SUCCESS);
         }
@@ -649,7 +649,7 @@ More macros are provided for iterating the list:
 
         isc_foo_t *foo;
         for (foo = ISC_LIST_HEAD(foolist);
-             foo != NULL; 
+             foo != NULL;
              foo = ISC_LIST_NEXT(foo, link))
         {
                 /* do things */
@@ -818,7 +818,7 @@ The return value may be:
 * `dns_name_commonancestor`: name1 and name2 share some labels
 * `dns_name_equal`: name1 and name2 are the same
 
-Some simpler comparison functions are provided for convenience when 
+Some simpler comparison functions are provided for convenience when
 not all of this information is required:
 
 * `dns_name_compare()`: returns the sort order of two names but
@@ -889,7 +889,7 @@ sets have been defined:
 
 Each of these has a `first()`, `next()` and `current()` function; for
 example, `dns_rdataset_first()`, `dns_rdataset_next()`, and
-`dns_rdataset_current()`. 
+`dns_rdataset_current()`.
 
 The `first()` and `next()` functions move the iterator's cursor and so that
 the data at a new location can be retrieved.  (Most of these can only step
@@ -1038,7 +1038,7 @@ messages up to the current debugging level are written to the channel.
 
 These objects -- the category, module, and channel -- direct hessages
 to desired destinations.  Each category/module pair can be associated
-with a specific channel, and the correct destination will be used 
+with a specific channel, and the correct destination will be used
 when a message is logged by `isc_log_write()`.
 
 In `isc_log_write()`, the logging system first looks up a list that
@@ -1171,7 +1171,7 @@ to control the closing of log files.
 
         void isc_log_setdebuglevel(isc_log_t *lctx, unsigned int level);
         unsigned int isc_log_getdebuglevel(isc_log_t *lctx);
-        
+
 These set and retrieve the current debugging level of the program.
 `isc_log_getdebuglevel()` can be used so that you need not keep track of
 the level yourself in another variable.
@@ -1417,7 +1417,7 @@ be triggered at that time.
          result = isc_timer_reset(timermgr, isc_timertype_once, NULL,
                                   interval, false);
 
-An event can also be explicitly triggered via `isc_task_send()`.  
+An event can also be explicitly triggered via `isc_task_send()`.
 
         static void
         do_things(isc_task_t *task, isc_event_t *event) {
@@ -1427,7 +1427,7 @@ An event can also be explicitly triggered via `isc_task_send()`.
         ...
 
         /*
-         * Allocate an event that calls 'do_things' with a 
+         * Allocate an event that calls 'do_things' with a
          * NULL argument, using 'myself' as ev_sender.
          *
          * DNS_EVENT_DOTHINGS must be defined in <dns/events.h>.