Clarify output

script_umdp3_checker/umdp3_checker_rules.py

@@ -236,7 +236,7 @@ def openmp_sentinels_in_column_one(self, lines: List[str]) -> TestResult:
                 )
         output = f"Checked {count+1} lines, found {failures} failures."
         return TestResult(
-            checker_name="Capitalised Keywords",
+            checker_name="OpenMP sentinels not in column one",
             failure_count=failures,
             passed=(failures == 0),
             output=output,
@@ -252,7 +252,8 @@ def unseparated_keywords(self, lines: List[str]) -> TestResult:
             if line.lstrip(" ").startswith("!"):
                 continue
             clean_line = self.remove_quoted(line)
-            for pattern in [f"\\b{kw}\\b" for kw in unseparated_keywords_list]:
+            for pattern in [f"\\b{kw}\\b" for kw in
+                            unseparated_keywords_list]:
                 if re.search(pattern, clean_line, re.IGNORECASE):
                     self.add_extra_error(f"unseparated keyword in line: "
                                          f"{line.strip()}")

script_umdp3_checker/umdp3_conformance.py

@@ -21,8 +21,15 @@
 
 @dataclass
 class CheckResult:
-    """Result from running a style checker on a file."""
-
+    """
+    Docstring for CheckResult
+        A class to hold the results of running a style checker on a file.
+        It contains the file path, the number of tests failed, whether all
+        tests passed, and a list of TestResult objects for each test run on
+        that file.
+    """
+    """TODO : Might be better to store number of tests run, and number passed,
+    rather than just number failed and whether all passed."""
     file_path: str = "No file provided"
     tests_failed: int = 0
     all_passed: bool = False
@@ -173,6 +180,7 @@ def __init__(
         file_extensions: Set[str],
         check_functions: Dict[str, Callable],
         changed_files: List[Path] = [],
+        print_volume: int = 3,
     ):
         self.name = name
         self.file_extensions = file_extensions or set()
@@ -182,12 +190,12 @@ def __init__(
             if changed_files
             else []
         )
-        # Should wrap the following in some kind of verbosity control
-        # print(f"UMDP3_checker initialized :\n"
-        #       f"    Name : {self.name}\n"
-        #       f"    Has {len(self.check_functions)} check functions\n"
-        #       f"    Using {len(self.file_extensions)} file extensions\n"
-        #       f"    Gives {len(self.files_to_check)} files to check.")
+        if print_volume >= 5:
+            print(f"UMDP3_checker initialized :\n"
+                  f"    Name : {self.name}\n"
+                  f"    Has {len(self.check_functions)} check functions\n"
+                  f"    Using {len(self.file_extensions)} file extensions\n"
+                  f"    Gives {len(self.files_to_check)} files to check.")
 
     def get_name(self) -> str:
         return self.name
@@ -226,6 +234,7 @@ def __init__(
         file_extensions: Set[str],
         check_functions: Dict[str, List[str]],
         changed_files: List[Path],
+        print_volume: int = 3,
     ):
         self.name = name
         self.file_extensions = file_extensions or set()
@@ -235,12 +244,12 @@ def __init__(
             if changed_files
             else []
         )
-        # Should wrap the following in some kind of verbosity control
-        # print(f"ExternalChecker initialized :\n"
-        #       f"    Name : {self.name}\n"
-        #       f"    Has {len(self.check_commands)} check commands\n"
-        #       f"    Using {len(self.file_extensions)} file extensions\n"
-        #       f"    Gives {len(self.files_to_check)} files to check.")
+        if print_volume >= 5:
+            print(f"ExternalChecker initialized :\n"
+                  f"    Name : {self.name}\n"
+                  f"    Has {len(self.check_commands)} check commands\n"
+                  f"    Using {len(self.file_extensions)} file extensions\n"
+                  f"    Gives {len(self.files_to_check)} files to check.")
 
     def get_name(self) -> str:
         return self.name
@@ -353,7 +362,8 @@ def check_files(self) -> None:
         self.results = results
         return
 
-    def print_results(self, print_volume: int = 3) -> bool:
+    def print_results(self, print_volume: int = 3,
+                      quiet_pass: bool = True) -> bool:
         """Print results and return True if all checks passed.
         ========================================================"""
         """
@@ -367,35 +377,31 @@ def print_results(self, print_volume: int = 3) -> bool:
             # Lousy variable names here: 'result' is the CheckResult for a file
             # which had multiple tests, so result.all_passed is for that file.
             all_passed = all_passed and result.all_passed
-            if print_volume >= 2:
-                print(f"{file_status:7s} file : {result.file_path:50s}")
-            if print_volume < 4 and result.all_passed:
+            # verbosity level 4 overides quiet_pass for file summary.
+            if quiet_pass and result.all_passed and print_volume < 4:
                 continue
+            print(f"{file_status:7s} file : {result.file_path:50s}")
+            if print_volume >= 3 and not result.all_passed:
+                print(" " * 4 + line_2(86))
             for test_result in result.test_results:
-                # TODO : The output logic here is a bit of a mess.
                 if print_volume < 5 and test_result.passed:
                     continue
-                if print_volume >= 4:
-                    print(
-                        " " * 5
-                        + "-" * 50
-                        + " " * 5
-                        + f"\n     {test_result.checker_name} Output :\n"
-                        + " " * 5
-                        + f"{test_result.output}\n"
-                        + " " * 5
-                        + "-" * 50
-                    )
-                    if test_result.errors:
-                        print(" " * 5 + "-=-" * 30)
-                        print(" " * 5 + " Std Error :")
+                if print_volume >= 3 and not test_result.passed:
+                    plural = "" if test_result.failure_count == 1 else "s"
+                    print(f"     {test_result.checker_name:60s} : Found "
+                          + f"{test_result.failure_count:3} failure{plural}.")
+                    if test_result.errors and print_volume >= 4:
+                        print(" " * 8 + line_2(82))
                         for count, (title, info) in enumerate(
                             test_result.errors.items()
                         ):
-                            print(f"      {count:2} : {title} : {info}")
-                        print(" " * 5 + "-=-" * 30)
-                elif print_volume > 2:
-                    print(f"     {test_result.checker_name:60s} : ✗ FAIL")
+                            print(" " * 8 +
+                                  f"{count + 1:2} : {title} : {info}")
+                        print(" " * 8 + line_2(82))
+                elif print_volume >= 3:
+                    print(f"     {test_result.checker_name:60s} : ✓ PASS")
+            if print_volume >= 3 and not result.all_passed:
+                print(" " * 4 + line_2(86))
         return all_passed
 
 
@@ -427,10 +433,21 @@ def process_arguments():
         help="Maximum number of parallel workers"
     )
     parser.add_argument(
+        "--fullcheck", action="store_true",
+        help="Instead of just checking changed files, check all files in "
+             "the repository"
+    )
+    parser.add_argument(
+        "--printpass", action="store_true",
+        help="Print details of passed checks as well as failed ones.\n"
+             "By default, only failed checks are printed in detail."
+    )
+    group = parser.add_mutually_exclusive_group()
+    group.add_argument(
         "-v", "--verbose", action="count", default=0,
         help="Increase output verbosity"
     )
-    parser.add_argument(
+    group.add_argument(
         "-q", "--quiet", action="count", default=0,
         help="Decrease output verbosity"
     )
@@ -449,22 +466,69 @@ def process_arguments():
     return args
 
 
-def which_cms_is_it(path: str) -> CMSSystem:
+def line_1(length: int = 80) -> str:
+    """Helper function to print a line for separating output sections."""
+    repeats = length // 3
+    pads = length % 3
+    line = ""
+    if pads > 1:
+        line += "="
+    line += "-=-" * repeats
+    if pads > 0:
+        line += "="
+    return line
+
+
+def line_2(length: int = 80) -> str:
+    """Helper function to print a line for separating output sections."""
+    return "-" * length
+
+
+def which_cms_is_it(path: str, print_volume: int = 3) -> CMSSystem:
     """Determine which CMS is in use based on the presence of certain files."""
     repo_path = Path(path)
     if (repo_path / ".git").is_dir():
-        return GitBdiffWrapper(repo_path)
+        cms = GitBdiffWrapper(repo_path)
     elif (repo_path / ".svn").is_dir():
         """
         TODO : If we still want this to work reliably with FCM, it will need
         to also accept URLs and not just local paths."""
-        return FCMBdiffWrapper(repo_path)
+        cms = FCMBdiffWrapper(repo_path)
     else:
         raise RuntimeError("Unknown CMS type at path: " + str(path))
+    branch_name = cms.get_branch_name()
+    if not cms.is_branch():
+        # TODO : This /might/ be better as a raise ValueError to allow
+        # printing the help message, but for now just print and exit.
+        print(
+            f"The path {path} is not a branch."
+            f"\nReported branch name is : {branch_name}"
+            "\nThe meaning of differences is unclear, and so"
+            " checking is aborted.\n"
+            f"Please try switching on the full check option"
+        )
+        # Soft exit mainly so nightly testing on main doesn't flag failure.
+        exit(0)
+    else:
+        if print_volume >= 2:
+            print(f"Found branch, {branch_name}, at path {path}.")
+        if print_volume >= 4:
+            print("The files changed on this branch are:")
+            changed_files = cms.get_changed_files()
+            no_of_changed_files = len(changed_files)
+            extras = no_of_changed_files - 10
+            if no_of_changed_files > 10:
+                changed_files = changed_files[:10]
+            for changed_file in changed_files:
+                print(f"    {changed_file}")
+            if no_of_changed_files > 10:
+                print(f"    ... and {extras} more changed files.")
+    return cms
 
 
 def create_style_checkers(
-    file_types: List[str], changed_files: List[Path]
+    file_types: List[str], changed_files: List[Path],
+    print_volume: int = 3
 ) -> List[StyleChecker]:
     """Create style checkers based on requested file types."""
     dispatch_tables = CheckerDispatchTables()
@@ -475,7 +539,8 @@ def create_style_checkers(
         fortran_diff_table = dispatch_tables.get_diff_dispatch_table_fortran()
         fortran_file_table = dispatch_tables.get_file_dispatch_table_fortran()
         generic_file_table = dispatch_tables.get_file_dispatch_table_all()
-        print("Configuring Fortran checkers:")
+        if print_volume >= 3:
+            print("Configuring Fortran checkers:")
         combined_checkers = fortran_diff_table | fortran_file_table | \
             generic_file_table
         fortran_file_checker = UMDP3_checker.from_full_list(
@@ -484,7 +549,8 @@ def create_style_checkers(
         )
         checkers.append(fortran_file_checker)
     if "Python" in file_types:
-        print("Setting up Python external checkers.")
+        if print_volume >= 3:
+            print("Configuring External Python checkers:")
         file_extensions = {".py"}
         python_checkers = {
             # "flake 8":     ["flake8", "-q"],
@@ -493,11 +559,13 @@ def create_style_checkers(
             "ruff":        ["ruff", "check"],
         }
         python_file_checker = ExternalChecker(
-            "Python External Checkers", file_extensions,
+            "External Python Checkers", file_extensions,
             python_checkers, changed_files
         )
         checkers.append(python_file_checker)
     if "Generic" in file_types or file_types == []:
+        if print_volume >= 3:
+            print("Configuring Generic File Checkers:")
         all_file_dispatch_table = dispatch_tables.get_file_dispatch_table_all()
         generic_checker = UMDP3_checker(
             "Generic File Checker", set(), all_file_dispatch_table,
@@ -508,27 +576,49 @@ def create_style_checkers(
     return checkers
 
 
-# Example usage
+def get_files_to_check(path: str, full_check: bool,
+                       print_volume: int = 3) -> List[Path]:
+    """
+    Docstring for get_files_to_check : A routine to get the list of files to
+    check based on the CMS or the full check override.
+
+    :param path: The top level path of the direcotry or clone of the
+    repository to check.
+    :type path: str
+    :param full_check: Logical to focre checking of all files in the
+    repository, rather than just the changed files.
+    :type full_check: bool
+    :param print_volume: Verbosity level for printing. Default is 3.
+    :type print_volume: int
+    :return: List of relative file paths to check.
+    :rtype: List[Path]
+    """
+    if full_check:  # Override to check all files present.
+        repo_path = Path(path)
+        all_files = [f for f in repo_path.rglob("*") if f.is_file()]
+        if print_volume >= 1:
+            print("Full check override enabled.")
+        if print_volume >= 3:
+            print(f"    Found {len(all_files)} files to "
+                  f"check in repository at path: {path}")
+        return all_files
+    else:  # Configure CMS, and check we've been passed a branch
+        if print_volume >= 1:
+            print("Using a CMS to determine changed files.")
+        cms = which_cms_is_it(path, print_volume)
+        changed_files = cms.get_changed_files()
+        return changed_files
+
+
+# Usage when run from command line.
 if __name__ == "__main__":
     args = process_arguments()
 
-    # Configure CMS, and check we've been passed a branch
-    cms = which_cms_is_it(args.path)
-    branch_name = cms.get_branch_name()
-    if not cms.is_branch():
-        print(
-            f"The path {args.path} is not a branch."
-            f"\nReported branch name is : {branch_name}"
-            "\nThe meaning of differences is unclear, and so"
-            " checking is aborted."
-        )
-        exit(1)
-    else:
-        print(f"The branch, {branch_name}, at path {args.path} is a branch.")
-        if args.volume >= 5:
-            print("The files changed on this branch are:")
-            for changed_file in cms.get_changed_files():
-                print(f"  {changed_file}")
+    log_volume = args.volume
+    quiet_pass = not args.printpass
+
+    file_paths = get_files_to_check(args.path, args.fullcheck, log_volume)
+    full_file_paths = [Path(f"{args.path}/{f}") for f in file_paths]
 
     # Configure checkers
     """
@@ -538,8 +628,6 @@ def create_style_checkers(
         checkers to use for each file type."""
     checkers = []
 
-    full_file_paths = [f"{args.path}/{f}" for f in cms.get_changed_files()]
-    full_file_paths = [Path(f) for f in full_file_paths]
     active_checkers = create_style_checkers(args.file_types,
                                             full_file_paths)
 
@@ -554,7 +642,18 @@ def create_style_checkers(
 
     checker.check_files()
 
-    all_passed = checker.print_results(print_volume=args.volume)
+    if log_volume >= 3:
+        print(line_1(81))
+        print("## Results :" + " "*67 + "##")
+        print(line_1(81) + "\n")
+    else:
+        print("Results  :")
+    all_passed = checker.print_results(print_volume=log_volume,
+                                       quiet_pass=quiet_pass)
+    if log_volume >= 4:
+        print("\n" + line_1(81))
+        print("## Summary :" + " "*67 + "##")
+        print(line_1(81))
     print(f"Total files checked: {len(checker.results)}")
     print(f"Total files failed: "
           f"{sum(1 for r in checker.results if not r.all_passed)}")