grass.gunittest: Add option to run test files in random order

.github/workflows/test_thorough.bat

@@ -2,4 +2,4 @@ set grass=%1
 set python=%2
 
 call %grass% --tmp-project XY --exec g.download.project url=https://grass.osgeo.org/sampledata/north_carolina/nc_spm_full_v2beta1.tar.gz path=%USERPROFILE%
-call %grass% --tmp-project XY --exec %python% -m grass.gunittest.main --grassdata %USERPROFILE% --location nc_spm_full_v2beta1 --location-type nc --min-success 97.3 --config .github\workflows\osgeo4w_gunittest.cfg
+call %grass% --tmp-project XY --exec %python% -m grass.gunittest.main --grassdata %USERPROFILE% --location nc_spm_full_v2beta1 --location-type nc --min-success 97.3 --random-order --config .github\workflows\osgeo4w_gunittest.cfg

.github/workflows/test_thorough.sh

@@ -11,4 +11,4 @@ grass --tmp-project XY --exec \
 grass --tmp-project XY --exec \
     python3 -m grass.gunittest.main \
     --grassdata $HOME --location nc_spm_full_v2beta1 --location-type nc \
-    --min-success 100 $@
+    --min-success 100 --random-order $@

python/grass/docs/src/gunittest_running_tests.rst

@@ -83,6 +83,32 @@ to run the test and ``grass.gunittest.main`` returning zero return code
 even if some tests fail, use ``--min-success 0``
 
 
+Running test files in random order
+----------------------------------
+
+By default, test files run in the same order every time. A fixed order can
+hide a test which passes only because of what an earlier test did,
+for example a file an earlier test left in a shared directory. To run the
+test files in a random order, use::
+
+    python -m grass.gunittest.main ... --random-order
+
+The seed of the random order is printed at the start and stored in the
+report. To run the test files again in the same order, pass this seed::
+
+    python -m grass.gunittest.main ... --randomly-seed 1234
+
+To reuse the seed of the previous run, which is read from the report in the
+output directory, use ``--randomly-seed last``.
+
+The same seed gives the same order only for the same set of test files.
+Regardless of the order the test files run in, the reports list them in the
+usual order, so that reports from different runs can be compared.
+Tests inside a test file still run in their usual order.
+The ``--randomly-seed`` option and the printed seed follow the
+*pytest-randomly* plugin, but the order is random only when requested.
+
+
 Excluding test files from testing
 ---------------------------------
 

python/grass/gunittest/invoker.py

@@ -12,6 +12,7 @@
 import collections
 import os
 from pathlib import Path
+import random
 import shutil
 import subprocess
 import sys
@@ -140,7 +141,6 @@ def _run_test_module(
         self, module, results_dir: StrPath, gisdbase, location, timeout: float | None
     ) -> None:
         """Run one test file."""
-        self.testsuite_dirs[module.tested_dir].append(module.name)
         cwd: str = os.path.join(results_dir, module.tested_dir, module.name)
         data_dir: str = os.path.join(module.file_dir, "data")
         if Path(data_dir).exists():
@@ -284,31 +284,43 @@ def try_decode(data, encodings):
                 shutil.rmtree(mapset_dir, ignore_errors=True)
 
     def run_in_location(
-        self, gisdbase, location, location_type, results_dir: StrPath, exclude
+        self,
+        gisdbase,
+        location,
+        location_type,
+        results_dir: StrPath,
+        exclude,
+        seed: int | None = None,
     ) -> GrassTestFilesMultiReporter:
         """Run tests in a given location
 
         Returns an object with counting attributes of GrassTestFilesCountingReporter,
         i.e., a file-oriented reporter as opposed to testsuite-oriented one.
         Use only the attributes related to the summary, such as file_pass_per,
         not to one file as these will simply contain the last executed file.
+
+        :param seed: if not None, test files run in a random order
+            generated from this seed (the same seed gives the same order
+            for the same set of test files); reports still list the test
+            files in discovery order
         """
         if os.path.abspath(results_dir) == os.path.abspath(self.start_dir):
             msg = (
                 "Results root directory should not be the same"
                 " as discovery start directory"
             )
             raise RuntimeError(msg)
+        info = {"location": location, "location_type": location_type}
+        if seed is not None:
+            info["random_seed"] = seed
         self.reporter = GrassTestFilesMultiReporter(
             reporters=[
                 GrassTestFilesTextReporter(stream=sys.stderr),
                 GrassTestFilesHtmlReporter(
                     file_anonymizer=self._file_anonymizer,
                     main_page_name="testfiles.html",
                 ),
-                GrassTestFilesKeyValueReporter(
-                    info={"location": location, "location_type": location_type}
-                ),
+                GrassTestFilesKeyValueReporter(info=info),
             ]
         )
         self.testsuite_dirs = collections.defaultdict(
@@ -326,6 +338,10 @@ def run_in_location(
             import_modules=False,
             exclude=exclude,
         )
+        for module in modules:
+            self.testsuite_dirs[module.tested_dir].append(module.name)
+        if seed is not None:
+            random.Random(seed).shuffle(modules)
 
         self.reporter.start(results_dir)
         for module in modules:
@@ -340,16 +356,23 @@ def run_in_location(
 
         # TODO: move this to some (new?) reporter
         # TODO: add basic summary of linked files so that the page is not empty
+        if seed is None:
+            order_text = ""
+        else:
+            order_text = f"<p>Test files ran in random order with seed {seed}.</p>"
         Path(os.path.join(results_dir, "index.html")).write_text(
             "<html><body>"
             "<h1>Tests for &lt;{location}&gt;"
             " using &lt;{type}&gt; type tests</h1>"
+            "{order_text}"
             "<ul>"
             '<li><a href="testsuites.html">Results by testsuites</a>'
             " (testsuite directories)</li>"
             '<li><a href="testfiles.html">Results by test files</a></li>'
             "<ul>"
-            "</body></html>".format(location=location, type=location_type),
+            "</body></html>".format(
+                location=location, type=location_type, order_text=order_text
+            ),
             encoding="utf-8",
         )
 

python/grass/gunittest/loader.py

@@ -57,6 +57,16 @@ class GrassTestPythonModule(NamedTuple):
     abs_file_path: str
 
 
+def discovery_sort_key(module: GrassTestPythonModule) -> tuple[tuple[str, ...], str]:
+    """Return a key which sorts test files in the order of discovery
+
+    Sorting by this key gives the order in which :func:`discover_modules`
+    returns the test files, so reports can keep that order even when
+    the test files run in a different one.
+    """
+    return PurePath(module.tested_dir).parts, os.path.basename(module.file_path)
+
+
 # TODO: implement loading without the import
 def discover_modules(
     start_dir,

python/grass/gunittest/main.py

@@ -8,6 +8,7 @@
 """
 
 import os
+import random
 import sys
 import argparse
 import configparser
@@ -17,6 +18,7 @@
 
 import grass.script.core as gs
 
+from .checkers import text_to_keyvalue
 from .loader import GrassTestLoader
 from .runner import (
     GrassTestRunner,
@@ -168,6 +170,28 @@ def get_config(start_directory, config_file):
     return config_parser["gunittest"]
 
 
+def seed_type(value: str) -> int | str:
+    """Convert a seed option value to int unless it is the special value last"""
+    if value == "last":
+        return value
+    try:
+        return int(value)
+    except ValueError:
+        msg = f"invalid seed value: {value!r} (use an integer or 'last')"
+        raise argparse.ArgumentTypeError(msg) from None
+
+
+def get_last_seed(results_dir) -> int | None:
+    """Return the seed of the previous run stored in its report, if any"""
+    summary_file = Path(results_dir) / "test_keyvalue_result.txt"
+    try:
+        text = summary_file.read_text(encoding="utf-8")
+    except OSError:
+        return None
+    seed = text_to_keyvalue(text, sep="=").get("random_seed")
+    return seed if isinstance(seed, int) else None
+
+
 def main():
     parser = argparse.ArgumentParser(
         description="Run test files in all testsuite directories starting"
@@ -220,6 +244,28 @@ def main():
         type=str,
         help=f"Path to a configuration file (default: {CONFIG_FILENAME})",
     )
+    parser.add_argument(
+        "--random-order",
+        dest="random_order",
+        action="store_true",
+        help=(
+            "Run test files in random order using a newly generated seed"
+            " (reports still list test files in the usual order)"
+        ),
+    )
+    parser.add_argument(
+        "--randomly-seed",
+        dest="randomly_seed",
+        action="store",
+        type=seed_type,
+        default=None,
+        help=(
+            "Run test files in random order generated from this seed,"
+            " or pass the special value 'last' to reuse the seed from the"
+            " previous run with the same output directory"
+            " (implies --random-order)"
+        ),
+    )
     args = parser.parse_args()
     gisdbase = args.gisdbase
     if gisdbase is None:
@@ -238,6 +284,16 @@ def main():
             f" does not exist in GRASS Database <{gisdbase}>\n"
         )
     results_dir = args.output
+    seed = args.randomly_seed
+    if seed == "last":
+        # The previous report is removed below, so get the seed first.
+        seed = get_last_seed(results_dir)
+        if seed is None:
+            return (
+                f"No seed of a previous run in random order found in <{results_dir}>\n"
+            )
+    elif args.random_order and seed is None:
+        seed = random.randrange(2**32)
     silent_rmtree(results_dir)  # TODO: too brute force?
 
     start_dir = "."
@@ -248,6 +304,9 @@ def main():
     except OSError as error:
         return f"Error reading configuration: {error}"
 
+    if seed is not None:
+        print(f"Using --randomly-seed={seed}", file=sys.stderr)
+
     invoker = GrassTestFilesInvoker(
         start_dir=start_dir,
         file_anonymizer=FileAnonymizer(paths_to_remove=[abs_start_dir]),
@@ -263,6 +322,7 @@ def main():
         location_type=location_type,
         results_dir=results_dir,
         exclude=config.get("exclude", "").split(),
+        seed=seed,
     )
     if not reporter.test_files:
         return "No tests found or executed"

python/grass/gunittest/reporters.py

@@ -23,6 +23,7 @@
 
 from .utils import add_gitignore_to_dir, ensure_dir
 from .checkers import text_to_keyvalue
+from .loader import discovery_sort_key
 
 
 from io import StringIO
@@ -560,13 +561,17 @@ def __init__(self, file_anonymizer, main_page_name: str = "index.html"):
         self.main_index = None
         self._file_anonymizer = file_anonymizer
         self._main_page_name = main_page_name
+        self._main_page_path = None
+        self._main_page_header = None
+        self._main_page_rows = []
 
     def start(self, results_dir):
         super().start(results_dir)
         # having all variables public although not really part of API
-        main_page_name = os.path.join(results_dir, self._main_page_name)
+        self._main_page_path = os.path.join(results_dir, self._main_page_name)
+        self._main_page_rows = []
         # TODO: Ensure file is closed in all situations
-        self.main_index = open(main_page_name, "w", encoding="utf-8")  # noqa: SIM115
+        self.main_index = open(self._main_page_path, "w", encoding="utf-8")  # noqa: SIM115
 
         # TODO: this can be moved to the counter class
         self.failures = 0
@@ -589,7 +594,7 @@ def start(self, results_dir):
             svn_text = ('SVN revision <a href="{url}">{rev}</a>').format(
                 url=url, rev=svn_info["revision"]
             )
-        self.main_index.write(
+        self._main_page_header = (
             "<html><body>"
             "<h1>Test results</h1>"
             "{time:%Y-%m-%d %H:%M:%S}"
@@ -603,6 +608,7 @@ def start(self, results_dir):
             "<th>Failed</th><th>Percent successful</th>"
             "</tr></thead><tbody>".format(time=self.main_start_time, svn=svn_text)
         )
+        self.main_index.write(self._main_page_header)
 
     def finish(self):
         super().finish()
@@ -639,12 +645,16 @@ def finish(self):
             )
         )
 
-        self.main_index.write(
-            "<tbody>{tfoot}</table><p>{summary}</p></body></html>".format(
-                tfoot=tfoot, summary=summary_sentence
-            )
-        )
         self.main_index.close()
+        self._main_page_rows.sort()
+        with open(self._main_page_path, "w", encoding="utf-8") as main_page:
+            main_page.write(self._main_page_header)
+            main_page.writelines(row for unused_key, row in self._main_page_rows)
+            main_page.write(
+                "<tbody>{tfoot}</table><p>{summary}</p></body></html>".format(
+                    tfoot=tfoot, summary=summary_sentence
+                )
+            )
 
     def start_file_test(self, module):
         super().start_file_test(module)
@@ -694,7 +704,7 @@ def end_file_test(
         else:
             total = successes = pass_per = self.unknown_number
         bad_ones = failures + errors
-        self.main_index.write(
+        row = (
             "<tr><td>{d}</td>"
             '<td><a href="{d}/{m}/index.html">{m}</a></td>'
             "<td>{status}</td>"
@@ -710,6 +720,8 @@ def end_file_test(
                 ptests=pass_per,
             )
         )
+        self.main_index.write(row)
+        self._main_page_rows.append((discovery_sort_key(module), row))
         wrap_stdstream_to_html(
             infile=stdout,
             outfile=os.path.join(cwd, "stdout.html"),
@@ -852,6 +864,7 @@ def start(self, results_dir) -> None:
         self.names = []
         self.tested_dirs = []
         self.files_returncodes = []
+        self._sort_keys = []
 
         # sets (no size specified)
         self.modules = set()
@@ -860,6 +873,11 @@ def start(self, results_dir) -> None:
     def finish(self):
         super().finish()
 
+        order = sorted(range(len(self.names)), key=self._sort_keys.__getitem__)
+        self.names = [self.names[i] for i in order]
+        self.tested_dirs = [self.tested_dirs[i] for i in order]
+        self.files_returncodes = [self.files_returncodes[i] for i in order]
+
         # this should be moved to some additional meta passed in constructor
         svn_info = get_svn_info()
         svn_revision = "" if not svn_info else svn_info["revision"]
@@ -942,6 +960,7 @@ def end_file_test(
 
         self.tested_dirs.append(module.tested_dir)
         self.names.append(module.name)
+        self._sort_keys.append(discovery_sort_key(module))
 
         modules = test_summary.get("tested_modules", None)
         if modules: