gunittest: Exclude broken tests (#1587)

* Adds configuration file to gunittest which can contain list of files to exclude.
* Excluding all currently broken tests and one sometimes failing doctests.
* Print line with failed test name in pytest style.
* Include .git in hardcoded excluded dirs.
* Make the default min-success 100% (assuming no fluctuations with exclusions).
* No tests found ends with an error message, not traceback.
* Increase min success to 100% in CI.

.github/workflows/test_thorough.sh

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

.gunittest.cfg

@@ -0,0 +1,39 @@
+[gunittest]
+
+# Files (or wildcard patterns) to exclude from testing separated by newline or
+# space. This would be ideally empty or it would include just special cases,
+# but it includes mainly tests which can (and should) be fixed.
+exclude =
+    gui/wxpython/core/testsuite/toolboxes.sh
+    lib/init/testsuite/test_grass_tmp_mapset.py
+    python/grass/gunittest/testsuite/test_assertions_rast3d.py
+    python/grass/gunittest/testsuite/test_assertions_vect.py
+    python/grass/gunittest/testsuite/test_gmodules.py
+    python/grass/gunittest/testsuite/test_gunitest_doctests.py
+    python/grass/gunittest/testsuite/test_module_assertions.py
+    python/grass/pygrass/raster/testsuite/test_pygrass_raster_doctests.py
+    python/grass/pygrass/rpc/testsuite/test_pygrass_rpc_doctests.py
+    python/grass/script/testsuite/test_script_doctests.py
+    python/grass/temporal/testsuite/unittests_temporal_raster_algebra_equal_ts.py
+    python/grass/temporal/testsuite/unittests_temporal_raster_conditionals_complement_else.py
+    raster/r.contour/testsuite/testrc.py
+    raster/r.geomorphon/testsuite/test_r_geom.py
+    raster/r.in.gdal/testsuite/test_r_in_gdal.py
+    raster/r.in.lidar/testsuite/test_base_resolution.sh
+    raster/r.in.lidar/testsuite/test_base_resolution.sh
+    scripts/g.search.modules/testsuite/test_g_search_modules.py
+    temporal/t.connect/testsuite/test_distr_tgis_db_raster3d.py
+    temporal/t.connect/testsuite/test_distr_tgis_db_raster.py
+    temporal/t.connect/testsuite/test_distr_tgis_db_vector.py
+    temporal/t.info/testsuite/test.t.info.sh
+    temporal/t.rast.aggregate/testsuite/test_aggregation_relative.py
+    temporal/t.rast.series/testsuite/test_series.py
+    vector/v.in.lidar/testsuite/decimation_test.py
+    vector/v.in.lidar/testsuite/mask_test.py
+    vector/v.in.lidar/testsuite/test_v_in_lidar_basic.py
+    vector/v.in.lidar/testsuite/test_v_in_lidar_filter.py
+    vector/v.in.pdal/testsuite/test_v_in_pdal_basic.py
+    vector/v.in.pdal/testsuite/test_v_in_pdal_filter.py
+    vector/v.out.lidar/testsuite/test_v_out_lidar.py
+    vector/v.what/testsuite/test_vwhat_layers.py
+    vector/v.what/testsuite/test_vwhat_ncspm.py

python/grass/docs/src/gunittest_running_tests.rst

@@ -52,7 +52,7 @@ Then run the file as a Python script::
 
 If the file is a ``gunittest``-based or ``unittest``-based test,
 you will receive a textual output with failed individual tests (test methods).
-If the file is a general Python scriptyou need to examine the output carefully
+If the file is a general Python script you need to examine the output carefully
 as well as source code itself to see what is expected behavior.
 
 The same as for general Python scripts, applies also to Shell scripts,
@@ -78,11 +78,31 @@ use::
 
     python -m grass.gunittest.main ... --min-success 60
 
-If all tests should succeed, use ``--min-success 100``. If you want
+If all tests should succeed, use ``--min-success 100`` (the default). If you want
 to run the test and ``grass.gunittest.main`` returning zero return code
 even if some tests fail, use ``--min-success 0``
 
 
+Excluding test files from testing
+---------------------------------
+
+Test files which would be otherwise collected and executed can be excluded
+from the testing using the ``exclude`` key in the ``.gunittest.cfg``
+configuration file under the ``.gunittest`` section.
+The value of ``exclude`` is whitespace-separated list of exclude files
+possibly with wildcards (from Python fnmatch).
+Directory separators are converted to ``/`` for the matching.
+Paths are relative to the directory the testing was started from.
+
+For example, to exclude the whole directory ``vector`` and one specific file,
+you would use::
+
+    [gunittest]
+    exclude =
+        vector/*
+        raster/r.contour/testsuite/testrc.py
+
+
 Running tests and creating report
 ---------------------------------
 

python/grass/gunittest/invoker.py

@@ -266,7 +266,7 @@ class GrassTestFilesInvoker(object):
         if self.clean_mapsets:
             shutil.rmtree(mapset_dir)
 
-    def run_in_location(self, gisdbase, location, location_type, results_dir):
+    def run_in_location(self, gisdbase, location, location_type, results_dir, exclude):
         """Run tests in a given location
 
         Returns an object with counting attributes of GrassTestFilesCountingReporter,
@@ -304,6 +304,7 @@ class GrassTestFilesInvoker(object):
             all_locations_value=GrassTestLoader.all_tests_value,
             universal_location_value=GrassTestLoader.universal_tests_value,
             import_modules=False,
+            exclude=exclude,
         )
 
         self.reporter.start(results_dir)

python/grass/gunittest/loader.py

@@ -16,10 +16,50 @@ import collections
 import re
 
 
+def fnmatch_exclude_with_base(files, base, exclude):
+    """Return list of files not matching any exclusion pattern
+
+    :param files: list of file names
+    :param base: directory (path) where the files are
+    :param exclude: list of fnmatch glob patterns for exclusion
+    """
+    not_excluded = []
+    patterns = []
+    # Make all dir separators slashes and drop leading current dir
+    # for both patterns and (later) for files.
+    for pattern in exclude:
+        pattern = pattern.replace(os.sep, "/")
+        if pattern.startswith("./"):
+            patterns.append(pattern[2:])
+        else:
+            patterns.append(pattern)
+    for filename in files:
+        full_file_path = os.path.join(base, filename)
+        test_filename = full_file_path.replace(os.sep, "/")
+        if full_file_path.startswith("./"):
+            test_filename = full_file_path[2:]
+        matches = False
+        for pattern in patterns:
+            if fnmatch.fnmatch(test_filename, pattern):
+                matches = True
+                break
+        if not matches:
+            not_excluded.append(filename)
+    return not_excluded
+
+
 # TODO: resolve test file versus test module
 GrassTestPythonModule = collections.namedtuple(
     "GrassTestPythonModule",
-    ["name", "module", "file_type", "tested_dir", "file_dir", "abs_file_path"],
+    [
+        "name",
+        "module",
+        "file_type",
+        "tested_dir",
+        "file_dir",
+        "file_path",
+        "abs_file_path",
+    ],
 )
 
 
@@ -35,6 +75,7 @@ def discover_modules(
     add_failed_imports=True,
     file_pattern=None,
     file_regexp=None,
+    exclude=None,
 ):
     """Find all test files (modules) in a directory tree.
 
@@ -80,6 +121,8 @@ def discover_modules(
                 files = fnmatch.filter(all_files, file_pattern)
             if file_regexp:
                 files = [f for f in all_files if re.match(file_regexp, f)]
+            if exclude:
+                files = fnmatch_exclude_with_base(files, full, exclude)
             files = sorted(files)
             # get test/module name without .py
             # extpecting all files to end with .py
@@ -146,6 +189,7 @@ def discover_modules(
                             tested_dir=root,
                             file_dir=full,
                             abs_file_path=abs_file_path,
+                            file_path=os.path.join(full, file_name),
                             file_type=file_type,
                         )
                     )
@@ -158,7 +202,7 @@ def discover_modules(
 class GrassTestLoader(unittest.TestLoader):
     """Class handles GRASS-specific loading of test modules."""
 
-    skip_dirs = [".svn", "dist.*", "bin.*", "OBJ.*"]
+    skip_dirs = [".git", ".svn", "dist.*", "bin.*", "OBJ.*"]
     testsuite_dir = "testsuite"
     files_in_testsuite = "*.py"
     all_tests_value = "all"

python/grass/gunittest/main.py

@@ -12,6 +12,8 @@ for details.
 import os
 import sys
 import argparse
+import configparser
+from pathlib import Path
 
 from unittest.main import TestProgram
 
@@ -126,8 +128,20 @@ def discovery():
     sys.exit(not program.result.wasSuccessful())
 
 
-# TODO: makefile rule should depend on the whole build
-# TODO: create a full interface (using grass parser or argparse)
+CONFIG_FILENAME = ".gunittest.cfg"
+
+
+def get_config(start_directory):
+    """Read configuration if available, return empty dict if not"""
+    config_parser = configparser.ConfigParser()
+    config_file = Path(start_directory) / CONFIG_FILENAME
+    if config_file.is_file():
+        config_parser.read(config_file)
+    if "gunittest" in config_parser:
+        return config_parser["gunittest"]
+    return {}
+
+
 def main():
     parser = argparse.ArgumentParser(
         description="Run test files in all testsuite directories starting"
@@ -166,7 +180,7 @@ def main():
         "--min-success",
         dest="min_success",
         action="store",
-        default="90",
+        default="100",
         type=int,
         help=(
             "Minimum success percentage (lower percentage"
@@ -204,6 +218,9 @@ def main():
 
     start_dir = "."
     abs_start_dir = os.path.abspath(start_dir)
+
+    config = get_config(start_dir)
+
     invoker = GrassTestFilesInvoker(
         start_dir=start_dir,
         file_anonymizer=FileAnonymizer(paths_to_remove=[abs_start_dir]),
@@ -217,7 +234,10 @@ def main():
         location=location,
         location_type=location_type,
         results_dir=results_dir,
+        exclude=config.get("exclude", "").split(),
     )
+    if not reporter.test_files:
+        return "No tests found or executed"
     if reporter.file_pass_per >= args.min_success:
         return 0
     return 1

python/grass/gunittest/reporters.py

@@ -992,11 +992,7 @@ class GrassTestFilesTextReporter(GrassTestFilesCountingReporter):
 
     def start_file_test(self, module):
         super(GrassTestFilesTextReporter, self).start_file_test(module)
-        self._stream.write(
-            "Running {name} ({directory})...\n".format(
-                directory=module.tested_dir, name=module.name
-            )
-        )
+        self._stream.write("Running {file}...\n".format(file=module.file_path))
         # get the above line and all previous ones to the report
         self._stream.flush()
 
@@ -1013,9 +1009,7 @@ class GrassTestFilesTextReporter(GrassTestFilesCountingReporter):
                 self._stream.write(text.read())
             self._stream.write(width * "=")
             self._stream.write("\n")
-            self._stream.write(
-                "{m} from {d} failed".format(d=module.tested_dir, m=module.name)
-            )
+            self._stream.write("FAILED {file}".format(file=module.file_path))
             num_failed = test_summary.get("failures", 0)
             num_failed += test_summary.get("errors", 0)
             if num_failed: