From 538362496af0517979a88861619dfa41df4d7244 Mon Sep 17 00:00:00 2001 From: r-sharp Date: Fri, 20 Feb 2026 13:00:39 +0000 Subject: [PATCH 01/15] Yet another weird round of having to repeatedly accept the same changes which git / VS Code are marking as in conflict. Annoyingly, this seems to have introduced a bug as well. --- script_umdp3_checker/umdp3_conformance.py | 207 ++++++++++++---------- 1 file changed, 117 insertions(+), 90 deletions(-) diff --git a/script_umdp3_checker/umdp3_conformance.py b/script_umdp3_checker/umdp3_conformance.py index a5d48035..2193b0c7 100644 --- a/script_umdp3_checker/umdp3_conformance.py +++ b/script_umdp3_checker/umdp3_conformance.py @@ -111,11 +111,11 @@ def get_branch_name(self) -> str: return self.bdiff_obj.branch -class StyleChecker: +class StyleChecker(): """A Class intended to coordinate the running of a set of style checks on - a set of files. - The checks are a dictionary of named callable routines. - The files are a list of file paths to check.""" + a set of files. + The checks are a dictionary of named callable routines. + The files are a list of file paths to check.""" """ TODO: This is where it might be good to set up a threadsafe @@ -130,16 +130,19 @@ class instance to hold the 'expanded' check outputs. a TestResult object directly, which includes the extra error info, so that each thread can work independently.""" name: str + # file_extensions: Set[str] check_functions: Dict[str, Callable] files_to_check: List[Path] def __init__( self, name: str, + # file_extensions: Set[str], check_functions: Dict[str, Callable], changed_files: List[Path], ): self.name = name + # self.file_extensions = file_extensions or set() self.check_functions = check_functions or {} self.files_to_check = changed_files or [] @@ -153,7 +156,8 @@ def check(self, file_path: Path) -> CheckResult: for check_name, check_function in self.check_functions.items(): file_results.append(check_function(lines)) - tests_failed = sum([0 if result.passed else 1 for result in file_results]) + tests_failed = sum([0 if result.passed else 1 for + result in file_results]) return CheckResult( file_path=str(file_path), tests_failed=tests_failed, @@ -170,22 +174,27 @@ def from_full_list( changed_files: List[Path], print_volume: int = 3, ): - files_to_check = ( - cls.filter_files(changed_files, file_extensions) if changed_files else [] + cls.name = name + cls.file_extensions = file_extensions or set() + cls.check_commands = check_functions or {} + cls.files_to_check = ( + cls.filter_files(changed_files, cls.file_extensions) + if changed_files + else [] ) if print_volume >= 5: print( f"ExternalChecker initialized :\n" - f" Name : {name}\n" - f" Has {len(check_functions)} check commands\n" - f" Using {len(file_extensions)} file extensions\n" - f" Gives {len(files_to_check)} files to check." + f" Name : {cls.name}\n" + f" Has {len(cls.check_commands)} check commands\n" + f" Using {len(cls.file_extensions)} file extensions\n" + f" Gives {len(cls.files_to_check)} files to check." ) return cls(name, check_functions, files_to_check) @staticmethod def filter_files( - files: List[Path], file_extensions: Optional[Set[str]] = None + files: List[Path], file_extensions: Set[str] = set() ) -> List[Path]: """Filter files based on the checker's file extensions.""" if not file_extensions: @@ -198,66 +207,61 @@ def create_external_runners( name: str, commands: List[List[str]], all_files: List[Path], - file_extensions: Set[str], + file_extensions: Set[str] = set(), ) -> "StyleChecker": """Create a StyleChecker instance filtering files from a full list.""" filtered_files = cls.filter_files(all_files, file_extensions) - print( - f"Creating external runners for {name} with {len(commands)} " - f"commands and {len(filtered_files)} files to check from a " - f"total of {len(all_files)} files." - ) + print(f"Creating external runners for {name} with {len(commands)} " + f"commands and {len(filtered_files)} files to check from a " + f"total of {len(all_files)} files.") check_functions = {} + # file_results = [] for command in commands: + print("Configuring external runner for command: " + f"{' '.join(command)}") external_opname = f"External_operation_{command[0]}" - free_runner = cls.create_free_runner(command, external_opname) - check_functions[external_opname] = free_runner - return cls(name, check_functions, filtered_files) - - @staticmethod - def create_free_runner( - command: List[str], external_opname: str - ) -> Callable[[Path], TestResult]: - """Method to create a free runner function for a given external - command with it's checker name for output.""" - - def new_free_runner(file_name: Path) -> TestResult: - cmd = command + [str(file_name)] - tests_failed = 0 - try: - result = subprocess.run(cmd, capture_output=True, text=True, timeout=60) - except subprocess.TimeoutExpired: - failure_count = 1 - passed = False - output = f"Checker {external_opname} timed out" - errors = {external_opname: "TimeoutExpired"} - tests_failed += 1 - except Exception as e: - failure_count = 1 - passed = False - output = str(e) - errors = {external_opname: str(e)} - tests_failed += 1 - else: - error_text = result.stderr if result.stderr else "" - failure_count = 0 if result.returncode == 0 else 1 - passed = result.returncode == 0 - output = result.stdout - if error_text: - errors = {external_opname: error_text} - else: - errors = {} - if result.returncode != 0: + print(f"Checker name for this command is : {external_opname}") + + def new_mangle(file_name: Path) -> TestResult: + cmd = command + [str(file_name)] + tests_failed = 0 + try: + result = subprocess.run(cmd, capture_output=True, + text=True, timeout=60) + except subprocess.TimeoutExpired: + failure_count = 1 + passed = False + output = f"Checker {external_opname} timed out" + errors = {external_opname: "TimeoutExpired"} tests_failed += 1 - return TestResult( - checker_name=external_opname, - failure_count=failure_count, - passed=passed, - output=output, - errors=errors, - ) - - return new_free_runner + except Exception as e: + failure_count = 1 + passed = False + output = str(e) + errors = {external_opname: str(e)} + tests_failed += 1 + else: + error_text = result.stderr if result.stderr else "" + failure_count = 0 if result.returncode == 0 else 1 + passed = result.returncode == 0 + output = result.stdout + if error_text: + errors = {external_opname: error_text} + else: + errors = {} + if result.returncode != 0: + tests_failed += 1 + # --- + return (TestResult( + checker_name=external_opname, + failure_count=failure_count, + passed=passed, + output=output, + errors=errors, + )) + + check_functions[external_opname] = new_mangle + return cls(name, check_functions, filtered_files) class Check_Runner(StyleChecker): @@ -270,7 +274,11 @@ def check(self, file_path: Path) -> CheckResult: file_results = [] # list of TestResult objects for check_name, check_function in self.check_functions.items(): file_results.append(check_function(file_path)) - tests_failed = sum([0 if result.passed else 1 for result in file_results]) + print(f"Finished running {check_name} on {file_path}. ") + print(f"Self described as: {file_results[-1].checker_name}") + + tests_failed = sum([0 if result.passed else 1 for result in + file_results]) return CheckResult( file_path=str(file_path), tests_failed=tests_failed, @@ -339,7 +347,8 @@ def check_files(self) -> None: self.results = results return - def print_results(self, print_volume: int = 3, quiet_pass: bool = True) -> bool: + def print_results(self, print_volume: int = 3, + quiet_pass: bool = True) -> bool: """Print results and return True if all checks passed. ========================================================""" """ @@ -373,7 +382,8 @@ def print_results(self, print_volume: int = 3, quiet_pass: bool = True) -> bool: for count, (title, info) in enumerate( test_result.errors.items() ): - print(" " * 8 + f"{count + 1:2} : {title} : {info}") + 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") @@ -403,7 +413,8 @@ def process_arguments(): "-p", "--path", type=str, default="./", help="path to repository" ) parser.add_argument( - "--max-workers", type=int, default=8, help="Maximum number of parallel workers" + "--max-workers", type=int, default=8, + help="Maximum number of parallel workers" ) parser.add_argument( "--fullcheck", @@ -419,10 +430,12 @@ def process_arguments(): ) group = parser.add_mutually_exclusive_group() group.add_argument( - "-v", "--verbose", action="count", default=0, help="Increase output verbosity" + "-v", "--verbose", action="count", default=0, + help="Increase output verbosity" ) group.add_argument( - "-q", "--quiet", action="count", default=0, help="Decrease output verbosity" + "-q", "--quiet", action="count", default=0, + help="Decrease output verbosity" ) # The following are not yet implemented, but may become useful # branch and base branch could be used to configure the CMS diff @@ -526,36 +539,47 @@ def create_style_checkers( dispatch_tables = CheckerDispatchTables() checkers = [] if "Fortran" in file_types: - file_extensions = {".f", ".for", ".f90", ".f95", ".f03", ".f08", ".F90"} - """ - TODO : I /think/ the old version also checked '.h' files as Fortran. - Not sure if that is still needed.""" + file_extensions = {".f", ".for", ".f90", ".f95", + ".f03", ".f08", ".F90"} 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() if print_volume >= 3: print("Configuring Fortran checkers:") - combined_checkers = fortran_diff_table | fortran_file_table | generic_file_table + combined_checkers = fortran_diff_table | fortran_file_table | \ + generic_file_table fortran_file_checker = StyleChecker.from_full_list( - "Fortran Checker", file_extensions, combined_checkers, changed_files + "Fortran Checker", file_extensions, + combined_checkers, changed_files ) checkers.append(fortran_file_checker) + """if "Python" in file_types: + if print_volume >= 3: + print("Configuring External Python checkers:") + file_extensions = {".py"} + python_checkers = { + # "flake 8": ["flake8", "-q"], + # "black": ["black", "--check"], + # "pylint": ["pylint", "-E"], + "ruff": ["ruff", "check"], + } + python_file_checker = ExternalChecker( + "External Python Checkers", file_extensions, python_checkers, + changed_files + ) + checkers.append(python_file_checker) """ if "Python" in file_types: - print("Configuring External Linters for Python files.") + print("Setting up External Runners for Python files.") file_extensions = {".py"} external_commands = [ ["ruff", "check"], - """TODO : The following need 'tweaking' to replicate what's run as - part of the CI on GitHub.""", - # ["flake8", "-q"], - # ["black", "--check"], - # ["pylint", "-E"], + ["flake8", "-q"], + ["black", "--check"], + ["pylint", "-E"], ] python_file_checker = Check_Runner.create_external_runners( - "Python External Checkers", - external_commands, - changed_files, - file_extensions, + "Python External Checkers", external_commands, changed_files, + file_extensions ) checkers.append(python_file_checker) if "Generic" in file_types or file_types == []: @@ -563,7 +587,8 @@ def create_style_checkers( print("Configuring Generic File Checkers:") all_file_dispatch_table = dispatch_tables.get_file_dispatch_table_all() generic_checker = StyleChecker( - "Generic File Checker", all_file_dispatch_table, changed_files + "Generic File Checker", all_file_dispatch_table, + changed_files ) checkers.append(generic_checker) @@ -643,12 +668,14 @@ def get_files_to_check( print(line_1(81) + "\n") else: print("Results :") - all_passed = checker.print_results(print_volume=log_volume, quiet_pass=quiet_pass) + 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: {sum(1 for r in checker.results if not r.all_passed)}") + print(f"Total files failed: {sum(1 for r in checker.results if + not r.all_passed)}") exit(0 if all_passed else 1) From 999879260a2f21870edd6c6d6c998a69f0f38b8f Mon Sep 17 00:00:00 2001 From: r-sharp Date: Fri, 20 Feb 2026 16:44:36 +0000 Subject: [PATCH 02/15] Fixing issue with external runners not establishing properly. --- script_umdp3_checker/umdp3_conformance.py | 122 ++++++++++------------ 1 file changed, 56 insertions(+), 66 deletions(-) diff --git a/script_umdp3_checker/umdp3_conformance.py b/script_umdp3_checker/umdp3_conformance.py index 2193b0c7..72485216 100644 --- a/script_umdp3_checker/umdp3_conformance.py +++ b/script_umdp3_checker/umdp3_conformance.py @@ -217,52 +217,56 @@ def create_external_runners( check_functions = {} # file_results = [] for command in commands: - print("Configuring external runner for command: " - f"{' '.join(command)}") external_opname = f"External_operation_{command[0]}" - print(f"Checker name for this command is : {external_opname}") - - def new_mangle(file_name: Path) -> TestResult: - cmd = command + [str(file_name)] - tests_failed = 0 - try: - result = subprocess.run(cmd, capture_output=True, - text=True, timeout=60) - except subprocess.TimeoutExpired: - failure_count = 1 - passed = False - output = f"Checker {external_opname} timed out" - errors = {external_opname: "TimeoutExpired"} - tests_failed += 1 - except Exception as e: - failure_count = 1 - passed = False - output = str(e) - errors = {external_opname: str(e)} - tests_failed += 1 - else: - error_text = result.stderr if result.stderr else "" - failure_count = 0 if result.returncode == 0 else 1 - passed = result.returncode == 0 - output = result.stdout - if error_text: - errors = {external_opname: error_text} - else: - errors = {} - if result.returncode != 0: - tests_failed += 1 - # --- - return (TestResult( - checker_name=external_opname, - failure_count=failure_count, - passed=passed, - output=output, - errors=errors, - )) - - check_functions[external_opname] = new_mangle + free_runner = cls.create_free_runner(command, external_opname) + check_functions[external_opname] = free_runner return cls(name, check_functions, filtered_files) + @staticmethod + def create_free_runner(command: List[str], + external_opname: str) -> Callable[[Path], + TestResult]: + """Method to create a free runner function for a given external + command with it's checker name for output.""" + def new_free_runner(file_name: Path) -> TestResult: + cmd = command + [str(file_name)] + tests_failed = 0 + try: + result = subprocess.run(cmd, capture_output=True, + text=True, timeout=60) + except subprocess.TimeoutExpired: + failure_count = 1 + passed = False + output = f"Checker {external_opname} timed out" + errors = {external_opname: "TimeoutExpired"} + tests_failed += 1 + except Exception as e: + failure_count = 1 + passed = False + output = str(e) + errors = {external_opname: str(e)} + tests_failed += 1 + else: + error_text = result.stderr if result.stderr else "" + failure_count = 0 if result.returncode == 0 else 1 + passed = result.returncode == 0 + output = result.stdout + if error_text: + errors = {external_opname: error_text} + else: + errors = {} + if result.returncode != 0: + tests_failed += 1 + # --- + return (TestResult( + checker_name=external_opname, + failure_count=failure_count, + passed=passed, + output=output, + errors=errors, + )) + return new_free_runner + class Check_Runner(StyleChecker): """A subclass of StyleChecker that is used to run external commands on a @@ -274,9 +278,6 @@ def check(self, file_path: Path) -> CheckResult: file_results = [] # list of TestResult objects for check_name, check_function in self.check_functions.items(): file_results.append(check_function(file_path)) - print(f"Finished running {check_name} on {file_path}. ") - print(f"Self described as: {file_results[-1].checker_name}") - tests_failed = sum([0 if result.passed else 1 for result in file_results]) return CheckResult( @@ -550,32 +551,21 @@ def create_style_checkers( generic_file_table fortran_file_checker = StyleChecker.from_full_list( "Fortran Checker", file_extensions, - combined_checkers, changed_files + combined_checkers, changed_files # type: ignore ) + # TODO : The type:ignore above disables something that pylance is + # complaining about in VS Code, (the combined_checkers argument) + # but I can't translate the 'explanation' of what the issue is into + # something meaningful. checkers.append(fortran_file_checker) - """if "Python" in file_types: - if print_volume >= 3: - print("Configuring External Python checkers:") - file_extensions = {".py"} - python_checkers = { - # "flake 8": ["flake8", "-q"], - # "black": ["black", "--check"], - # "pylint": ["pylint", "-E"], - "ruff": ["ruff", "check"], - } - python_file_checker = ExternalChecker( - "External Python Checkers", file_extensions, python_checkers, - changed_files - ) - checkers.append(python_file_checker) """ if "Python" in file_types: - print("Setting up External Runners for Python files.") + print("Configuring External Linters for Python files.") file_extensions = {".py"} external_commands = [ ["ruff", "check"], - ["flake8", "-q"], - ["black", "--check"], - ["pylint", "-E"], + # ["flake8", "-q"], + # ["black", "--check"], + # ["pylint", "-E"], ] python_file_checker = Check_Runner.create_external_runners( "Python External Checkers", external_commands, changed_files, From ac9f9f3734a34e13d14fc2b9bd344abea7668afa Mon Sep 17 00:00:00 2001 From: r-sharp Date: Fri, 20 Feb 2026 18:05:26 +0000 Subject: [PATCH 03/15] Fixing an error that seems to have crept in during merging main and resolving clashes. But looking at it, I really can't fathom out 'how' --- script_umdp3_checker/umdp3_conformance.py | 27 +++++++++-------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/script_umdp3_checker/umdp3_conformance.py b/script_umdp3_checker/umdp3_conformance.py index 72485216..477c0c1e 100644 --- a/script_umdp3_checker/umdp3_conformance.py +++ b/script_umdp3_checker/umdp3_conformance.py @@ -169,26 +169,23 @@ def check(self, file_path: Path) -> CheckResult: def from_full_list( cls, name: str, - file_extensions: Set[str], - check_functions: Dict[str, Callable], - changed_files: List[Path], + file_extensions: Set[str] = set(), + check_functions: Dict[str, Callable] = {}, + changed_files: List[Path] = [], print_volume: int = 3, ): - cls.name = name - cls.file_extensions = file_extensions or set() - cls.check_commands = check_functions or {} - cls.files_to_check = ( - cls.filter_files(changed_files, cls.file_extensions) + files_to_check = ( + cls.filter_files(changed_files, file_extensions) if changed_files else [] ) if print_volume >= 5: print( f"ExternalChecker initialized :\n" - f" Name : {cls.name}\n" - f" Has {len(cls.check_commands)} check commands\n" - f" Using {len(cls.file_extensions)} file extensions\n" - f" Gives {len(cls.files_to_check)} files to check." + f" Name : {name}\n" + f" Has {len(check_functions)} check commands\n" + f" Using {len(file_extensions)} file extensions\n" + f" Gives {len(files_to_check)} files to check." ) return cls(name, check_functions, files_to_check) @@ -551,12 +548,8 @@ def create_style_checkers( generic_file_table fortran_file_checker = StyleChecker.from_full_list( "Fortran Checker", file_extensions, - combined_checkers, changed_files # type: ignore + combined_checkers, changed_files ) - # TODO : The type:ignore above disables something that pylance is - # complaining about in VS Code, (the combined_checkers argument) - # but I can't translate the 'explanation' of what the issue is into - # something meaningful. checkers.append(fortran_file_checker) if "Python" in file_types: print("Configuring External Linters for Python files.") From 44fcee8562965772c8a3da8ea7c53ef382ed1134 Mon Sep 17 00:00:00 2001 From: r-sharp Date: Fri, 20 Feb 2026 18:06:37 +0000 Subject: [PATCH 04/15] Where do these line length errors in the linter(s) come from - I haven't edited this file... --- script_umdp3_checker/umdp3_checker_rules.py | 72 ++++++++++++++------- 1 file changed, 48 insertions(+), 24 deletions(-) diff --git a/script_umdp3_checker/umdp3_checker_rules.py b/script_umdp3_checker/umdp3_checker_rules.py index a9a56edc..e9c28f6e 100644 --- a/script_umdp3_checker/umdp3_checker_rules.py +++ b/script_umdp3_checker/umdp3_checker_rules.py @@ -254,7 +254,8 @@ def unseparated_keywords(self, lines: List[str]) -> TestResult: clean_line = self.remove_quoted(line) 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: {line.strip()}") + self.add_extra_error(f"unseparated keyword in line: " + f"{line.strip()}") failures += 1 error_log = self.add_error_log( error_log, @@ -279,7 +280,8 @@ def go_to_other_than_9999(self, lines: List[str]) -> TestResult: clean_line = self.remove_quoted(line) clean_line = re.sub(r"!.*$", "", clean_line) - if match := re.search(r"\bGO\s*TO\s+(\d+)", clean_line, re.IGNORECASE): + if match := re.search(r"\bGO\s*TO\s+(\d+)", clean_line, + re.IGNORECASE): label = match.group(1) if label != "9999": self.add_extra_error(f"GO TO {label}") @@ -305,10 +307,12 @@ def write_using_default_format(self, lines: List[str]) -> TestResult: clean_line = self.remove_quoted(line) clean_line = re.sub(r"!.*$", "", clean_line) - if re.search(r"\bWRITE\s*\(\s*\*\s*,\s*\*\s*\)", clean_line, re.IGNORECASE): + if re.search(r"\bWRITE\s*\(\s*\*\s*,\s*\*\s*\)", clean_line, + re.IGNORECASE): self.add_extra_error("WRITE(*,*) found") failures += 1 - error_log = self.add_error_log(error_log, "WRITE(*,*) found", count + 1) + error_log = self.add_error_log(error_log, "WRITE(*,*) found", + count + 1) output = f"Checked {count + 1} lines, found {failures} failures." return TestResult( checker_name="WRITE using default format", @@ -345,7 +349,8 @@ def lowercase_variable_names(self, lines: List[str]) -> TestResult: clean_line, ) if match := re.search(r"([A-Z]{2,})", clean_line): - self.add_extra_error(f"UPPERCASE variable name : {match[1]}") + self.add_extra_error("UPPERCASE variable name : " + f"{match[1]}") failures += 1 error_log = self.add_error_log( error_log, f"UPPERCASE variable name {match[1]}", count @@ -418,7 +423,8 @@ def forbidden_keywords(self, lines: List[str]) -> TestResult: clean_line = self.remove_quoted(line) clean_line = re.sub(r"!.*$", "", clean_line) - if re.search(r"\b(EQUIVALENCE|PAUSE)\b", clean_line, re.IGNORECASE): + if re.search(r"\b(EQUIVALENCE|PAUSE)\b", clean_line, + re.IGNORECASE): self.add_extra_error("forbidden keyword") failures += 1 error_log = self.add_error_log( @@ -453,7 +459,8 @@ def forbidden_operators(self, lines: List[str]) -> TestResult: ) return TestResult( - checker_name="Use of older form of relational operator " + "(.GT. etc.)", + checker_name="Use of older form of relational operator " + + "(.GT. etc.)", failure_count=failures, passed=(failures == 0), output=f"Checked {count + 1} lines, found {failures} failures.", @@ -469,7 +476,8 @@ def line_over_80chars(self, lines: List[str]) -> TestResult: if len(line.rstrip()) > 80: self.add_extra_error("line too long") failures += 1 - error_log = self.add_error_log(error_log, "line too long", count + 1) + error_log = self.add_error_log(error_log, "line too long", + count + 1) return TestResult( checker_name="Line longer than 80 characters", @@ -533,7 +541,8 @@ def printstar(self, lines: List[str]) -> TestResult: if re.search(r"\bPRINT\s*\*", clean_line, re.IGNORECASE): self.add_extra_error("PRINT * used") failures += 1 - error_log = self.add_error_log(error_log, "PRINT * used", count + 1) + error_log = self.add_error_log(error_log, "PRINT * used", + count + 1) return TestResult( checker_name="Use of PRINT rather than umMessage and umPrint", @@ -555,7 +564,8 @@ def write6(self, lines: List[str]) -> TestResult: if re.search(r"\bWRITE\s*\(\s*6\s*,", clean_line, re.IGNORECASE): self.add_extra_error("WRITE(6) used") failures += 1 - error_log = self.add_error_log(error_log, "WRITE(6) used", count + 1) + error_log = self.add_error_log(error_log, "WRITE(6) used", + count + 1) return TestResult( checker_name="Use of WRITE(6) rather than umMessage and umPrint", @@ -611,10 +621,12 @@ def omp_missing_dollar(self, lines: List[str]) -> TestResult: error_log = {} count = -1 for count, line in enumerate(lines): - if re.search(r"!\s*OMP\b", line) and not re.search(r"!\$OMP", line): + if (re.search(r"!\s*OMP\b", line) and + not re.search(r"!\$OMP", line)): self.add_extra_error("!OMP without $") failures += 1 - error_log = self.add_error_log(error_log, "!OMP without $", count + 1) + error_log = self.add_error_log(error_log, "!OMP without $", + count + 1) return TestResult( checker_name="!OMP without $", @@ -662,7 +674,8 @@ def cpp_comment(self, lines: List[str]) -> TestResult: self.add_extra_error("Fortran comment in CPP directive") failures += 1 error_log = self.add_error_log( - error_log, "Fortran comment in CPP directive", count + 1 + error_log, "Fortran comment in CPP directive", + count + 1 ) return TestResult( @@ -687,7 +700,8 @@ def obsolescent_fortran_intrinsic(self, lines: List[str]) -> TestResult: self.add_extra_error(f"obsolescent intrinsic: {intrinsic}") failures += 1 error_log = self.add_error_log( - error_log, f"obsolescent intrinsic: {intrinsic}", count + 1 + error_log, f"obsolescent intrinsic: {intrinsic}", + count + 1 ) return TestResult( @@ -735,8 +749,10 @@ def intrinsic_modules(self, lines: List[str]) -> TestResult: for module in intrinsic_modules: if re.search( rf"\bUSE\s+(::)*\s*{module}\b", clean_line, re.IGNORECASE - ) and not re.search(r"\bINTRINSIC\b", clean_line, re.IGNORECASE): - self.add_extra_error(f"intrinsic module {module} without INTRINSIC") + ) and not re.search(r"\bINTRINSIC\b", clean_line, + re.IGNORECASE): + self.add_extra_error(f"intrinsic module {module} " + "without INTRINSIC") failures += 1 error_log = self.add_error_log( error_log, @@ -761,7 +777,8 @@ def read_unit_args(self, lines: List[str]) -> TestResult: clean_line = self.remove_quoted(line) clean_line = re.sub(r"!.*$", "", clean_line) - if match := re.search(r"\bREAD\s*\(\s*([^,)]+)", clean_line, re.IGNORECASE): + if match := re.search(r"\bREAD\s*\(\s*([^,)]+)", clean_line, + re.IGNORECASE): first_arg = match.group(1).strip() if not first_arg.upper().startswith("UNIT="): self.add_extra_error("READ without explicit UNIT=") @@ -803,7 +820,8 @@ def retire_if_def(self, lines: List[str]) -> TestResult: self.add_extra_error(f"retired if-def: {match.group(1)}") failures += 1 error_log = self.add_error_log( - error_log, f"retired if-def: {match.group(1)}", count + 1 + error_log, f"retired if-def: {match.group(1)}", + count + 1 ) return TestResult( checker_name="retired if-def", @@ -845,7 +863,8 @@ def forbidden_stop(self, lines: List[str]) -> TestResult: clean_line = self.remove_quoted(line) clean_line = re.sub(r"!.*$", "", clean_line) - if re.search(r"\b(STOP|CALL\s+abort)\b", clean_line, re.IGNORECASE): + if re.search(r"\b(STOP|CALL\s+abort)\b", clean_line, + re.IGNORECASE): self.add_extra_error("STOP or CALL abort used") failures += 1 error_log = self.add_error_log( @@ -910,7 +929,8 @@ def check_crown_copyright(self, lines: List[str]) -> TestResult: found_copyright = True if not found_copyright: - self.add_extra_error("missing copyright or crown copyright statement") + self.add_extra_error( + "missing copyright or crown copyright statement") error_log = self.add_error_log( error_log, "missing copyright or crown copyright statement", 0 ) @@ -935,14 +955,16 @@ def check_code_owner(self, lines: List[str]) -> TestResult: file_content = "\n".join(lines) found_code_owner = False error_log = {} - if "Code Owner:" in file_content or "code owner" in file_content.lower(): + if ("Code Owner:" in file_content or + "code owner" in file_content.lower()): # print(f"Debug: Found {file_content.lower()}") found_code_owner = True # This is often a warning rather than an error if not found_code_owner: self.add_extra_error("missing code owner comment") - error_log = self.add_error_log(error_log, "missing code owner comment", 0) + error_log = self.add_error_log(error_log, + "missing code owner comment", 0) return TestResult( checker_name="Code Owner Comment", failure_count=0 if found_code_owner else 1, @@ -1014,7 +1036,8 @@ def c_deprecated(self, lines: List[str]) -> int: for line in lines: for identifier in deprecated_c_identifiers: if re.search(rf"\b{identifier}\b", line): - self.add_extra_error(f"deprecated C identifier: {identifier}") + self.add_extra_error( + f"deprecated C identifier: {identifier}") failures += 1 return failures @@ -1041,7 +1064,8 @@ def c_openmp_define_no_combine(self, lines: List[str]) -> int: ) or re.search( r"&&.*_OPENMP.*&&.*SHUM_USE_C_OPENMP_VIA_THREAD_UTILS", line ): - self.add_extra_error("OpenMP defines combined with third macro") + self.add_extra_error( + "OpenMP defines combined with third macro") failures += 1 return failures From add428acb38573ce99f3a6e3cc0be1388fd3cce0 Mon Sep 17 00:00:00 2001 From: r-sharp Date: Fri, 20 Feb 2026 18:21:09 +0000 Subject: [PATCH 05/15] quick "improvement" to the error reporting in one of the tests --- script_umdp3_checker/umdp3_checker_rules.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/script_umdp3_checker/umdp3_checker_rules.py b/script_umdp3_checker/umdp3_checker_rules.py index e9c28f6e..3585394a 100644 --- a/script_umdp3_checker/umdp3_checker_rules.py +++ b/script_umdp3_checker/umdp3_checker_rules.py @@ -353,7 +353,8 @@ def lowercase_variable_names(self, lines: List[str]) -> TestResult: f"{match[1]}") failures += 1 error_log = self.add_error_log( - error_log, f"UPPERCASE variable name {match[1]}", count + error_log, f"UPPERCASE variable name {match[1]}", + count + 1 ) output = f"Checked {count + 1} lines, found {failures} failures." From 6e7dd8f09353bf77be1dc847bfc00670b0d3b07f Mon Sep 17 00:00:00 2001 From: r-sharp Date: Fri, 20 Feb 2026 18:36:07 +0000 Subject: [PATCH 06/15] Why am I constantly having to re-impliment fixes/tidying I'm sure I've done before. --- script_umdp3_checker/umdp3_conformance.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/script_umdp3_checker/umdp3_conformance.py b/script_umdp3_checker/umdp3_conformance.py index 477c0c1e..bb118b33 100644 --- a/script_umdp3_checker/umdp3_conformance.py +++ b/script_umdp3_checker/umdp3_conformance.py @@ -11,7 +11,6 @@ # Add custom modules to Python path if needed # Add the repository root to access fcm_bdiff and git_bdiff packages import sys - sys.path.insert(0, str(Path(__file__).parent.parent)) """ @@ -130,19 +129,16 @@ class instance to hold the 'expanded' check outputs. a TestResult object directly, which includes the extra error info, so that each thread can work independently.""" name: str - # file_extensions: Set[str] check_functions: Dict[str, Callable] files_to_check: List[Path] def __init__( self, name: str, - # file_extensions: Set[str], check_functions: Dict[str, Callable], changed_files: List[Path], ): self.name = name - # self.file_extensions = file_extensions or set() self.check_functions = check_functions or {} self.files_to_check = changed_files or [] @@ -212,7 +208,6 @@ def create_external_runners( f"commands and {len(filtered_files)} files to check from a " f"total of {len(all_files)} files.") check_functions = {} - # file_results = [] for command in commands: external_opname = f"External_operation_{command[0]}" free_runner = cls.create_free_runner(command, external_opname) @@ -254,7 +249,6 @@ def new_free_runner(file_name: Path) -> TestResult: errors = {} if result.returncode != 0: tests_failed += 1 - # --- return (TestResult( checker_name=external_opname, failure_count=failure_count, @@ -556,6 +550,8 @@ def create_style_checkers( file_extensions = {".py"} external_commands = [ ["ruff", "check"], + """TODO : The following need 'tweaking' to replicate what's run as + part of the CI on GitHub.""" # ["flake8", "-q"], # ["black", "--check"], # ["pylint", "-E"], From 6a72f7f2674a1ec7977d5888b9447af08af2c160 Mon Sep 17 00:00:00 2001 From: r-sharp Date: Fri, 20 Feb 2026 19:20:50 +0000 Subject: [PATCH 07/15] And now ruff_format wants to change them back again... --- script_umdp3_checker/umdp3_checker_rules.py | 75 +++++++------------ script_umdp3_checker/umdp3_conformance.py | 83 +++++++++------------ 2 files changed, 62 insertions(+), 96 deletions(-) diff --git a/script_umdp3_checker/umdp3_checker_rules.py b/script_umdp3_checker/umdp3_checker_rules.py index 3585394a..68f03fd4 100644 --- a/script_umdp3_checker/umdp3_checker_rules.py +++ b/script_umdp3_checker/umdp3_checker_rules.py @@ -254,8 +254,7 @@ def unseparated_keywords(self, lines: List[str]) -> TestResult: clean_line = self.remove_quoted(line) 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()}") + self.add_extra_error(f"unseparated keyword in line: {line.strip()}") failures += 1 error_log = self.add_error_log( error_log, @@ -280,8 +279,7 @@ def go_to_other_than_9999(self, lines: List[str]) -> TestResult: clean_line = self.remove_quoted(line) clean_line = re.sub(r"!.*$", "", clean_line) - if match := re.search(r"\bGO\s*TO\s+(\d+)", clean_line, - re.IGNORECASE): + if match := re.search(r"\bGO\s*TO\s+(\d+)", clean_line, re.IGNORECASE): label = match.group(1) if label != "9999": self.add_extra_error(f"GO TO {label}") @@ -307,12 +305,10 @@ def write_using_default_format(self, lines: List[str]) -> TestResult: clean_line = self.remove_quoted(line) clean_line = re.sub(r"!.*$", "", clean_line) - if re.search(r"\bWRITE\s*\(\s*\*\s*,\s*\*\s*\)", clean_line, - re.IGNORECASE): + if re.search(r"\bWRITE\s*\(\s*\*\s*,\s*\*\s*\)", clean_line, re.IGNORECASE): self.add_extra_error("WRITE(*,*) found") failures += 1 - error_log = self.add_error_log(error_log, "WRITE(*,*) found", - count + 1) + error_log = self.add_error_log(error_log, "WRITE(*,*) found", count + 1) output = f"Checked {count + 1} lines, found {failures} failures." return TestResult( checker_name="WRITE using default format", @@ -349,12 +345,10 @@ def lowercase_variable_names(self, lines: List[str]) -> TestResult: clean_line, ) if match := re.search(r"([A-Z]{2,})", clean_line): - self.add_extra_error("UPPERCASE variable name : " - f"{match[1]}") + self.add_extra_error(f"UPPERCASE variable name : {match[1]}") failures += 1 error_log = self.add_error_log( - error_log, f"UPPERCASE variable name {match[1]}", - count + 1 + error_log, f"UPPERCASE variable name {match[1]}", count + 1 ) output = f"Checked {count + 1} lines, found {failures} failures." @@ -424,8 +418,7 @@ def forbidden_keywords(self, lines: List[str]) -> TestResult: clean_line = self.remove_quoted(line) clean_line = re.sub(r"!.*$", "", clean_line) - if re.search(r"\b(EQUIVALENCE|PAUSE)\b", clean_line, - re.IGNORECASE): + if re.search(r"\b(EQUIVALENCE|PAUSE)\b", clean_line, re.IGNORECASE): self.add_extra_error("forbidden keyword") failures += 1 error_log = self.add_error_log( @@ -460,8 +453,7 @@ def forbidden_operators(self, lines: List[str]) -> TestResult: ) return TestResult( - checker_name="Use of older form of relational operator " + - "(.GT. etc.)", + checker_name="Use of older form of relational operator " + "(.GT. etc.)", failure_count=failures, passed=(failures == 0), output=f"Checked {count + 1} lines, found {failures} failures.", @@ -477,8 +469,7 @@ def line_over_80chars(self, lines: List[str]) -> TestResult: if len(line.rstrip()) > 80: self.add_extra_error("line too long") failures += 1 - error_log = self.add_error_log(error_log, "line too long", - count + 1) + error_log = self.add_error_log(error_log, "line too long", count + 1) return TestResult( checker_name="Line longer than 80 characters", @@ -542,8 +533,7 @@ def printstar(self, lines: List[str]) -> TestResult: if re.search(r"\bPRINT\s*\*", clean_line, re.IGNORECASE): self.add_extra_error("PRINT * used") failures += 1 - error_log = self.add_error_log(error_log, "PRINT * used", - count + 1) + error_log = self.add_error_log(error_log, "PRINT * used", count + 1) return TestResult( checker_name="Use of PRINT rather than umMessage and umPrint", @@ -565,8 +555,7 @@ def write6(self, lines: List[str]) -> TestResult: if re.search(r"\bWRITE\s*\(\s*6\s*,", clean_line, re.IGNORECASE): self.add_extra_error("WRITE(6) used") failures += 1 - error_log = self.add_error_log(error_log, "WRITE(6) used", - count + 1) + error_log = self.add_error_log(error_log, "WRITE(6) used", count + 1) return TestResult( checker_name="Use of WRITE(6) rather than umMessage and umPrint", @@ -622,12 +611,10 @@ def omp_missing_dollar(self, lines: List[str]) -> TestResult: error_log = {} count = -1 for count, line in enumerate(lines): - if (re.search(r"!\s*OMP\b", line) and - not re.search(r"!\$OMP", line)): + if re.search(r"!\s*OMP\b", line) and not re.search(r"!\$OMP", line): self.add_extra_error("!OMP without $") failures += 1 - error_log = self.add_error_log(error_log, "!OMP without $", - count + 1) + error_log = self.add_error_log(error_log, "!OMP without $", count + 1) return TestResult( checker_name="!OMP without $", @@ -675,8 +662,7 @@ def cpp_comment(self, lines: List[str]) -> TestResult: self.add_extra_error("Fortran comment in CPP directive") failures += 1 error_log = self.add_error_log( - error_log, "Fortran comment in CPP directive", - count + 1 + error_log, "Fortran comment in CPP directive", count + 1 ) return TestResult( @@ -701,8 +687,7 @@ def obsolescent_fortran_intrinsic(self, lines: List[str]) -> TestResult: self.add_extra_error(f"obsolescent intrinsic: {intrinsic}") failures += 1 error_log = self.add_error_log( - error_log, f"obsolescent intrinsic: {intrinsic}", - count + 1 + error_log, f"obsolescent intrinsic: {intrinsic}", count + 1 ) return TestResult( @@ -750,10 +735,8 @@ def intrinsic_modules(self, lines: List[str]) -> TestResult: for module in intrinsic_modules: if re.search( rf"\bUSE\s+(::)*\s*{module}\b", clean_line, re.IGNORECASE - ) and not re.search(r"\bINTRINSIC\b", clean_line, - re.IGNORECASE): - self.add_extra_error(f"intrinsic module {module} " - "without INTRINSIC") + ) and not re.search(r"\bINTRINSIC\b", clean_line, re.IGNORECASE): + self.add_extra_error(f"intrinsic module {module} without INTRINSIC") failures += 1 error_log = self.add_error_log( error_log, @@ -778,8 +761,7 @@ def read_unit_args(self, lines: List[str]) -> TestResult: clean_line = self.remove_quoted(line) clean_line = re.sub(r"!.*$", "", clean_line) - if match := re.search(r"\bREAD\s*\(\s*([^,)]+)", clean_line, - re.IGNORECASE): + if match := re.search(r"\bREAD\s*\(\s*([^,)]+)", clean_line, re.IGNORECASE): first_arg = match.group(1).strip() if not first_arg.upper().startswith("UNIT="): self.add_extra_error("READ without explicit UNIT=") @@ -821,8 +803,7 @@ def retire_if_def(self, lines: List[str]) -> TestResult: self.add_extra_error(f"retired if-def: {match.group(1)}") failures += 1 error_log = self.add_error_log( - error_log, f"retired if-def: {match.group(1)}", - count + 1 + error_log, f"retired if-def: {match.group(1)}", count + 1 ) return TestResult( checker_name="retired if-def", @@ -864,8 +845,7 @@ def forbidden_stop(self, lines: List[str]) -> TestResult: clean_line = self.remove_quoted(line) clean_line = re.sub(r"!.*$", "", clean_line) - if re.search(r"\b(STOP|CALL\s+abort)\b", clean_line, - re.IGNORECASE): + if re.search(r"\b(STOP|CALL\s+abort)\b", clean_line, re.IGNORECASE): self.add_extra_error("STOP or CALL abort used") failures += 1 error_log = self.add_error_log( @@ -930,8 +910,7 @@ def check_crown_copyright(self, lines: List[str]) -> TestResult: found_copyright = True if not found_copyright: - self.add_extra_error( - "missing copyright or crown copyright statement") + self.add_extra_error("missing copyright or crown copyright statement") error_log = self.add_error_log( error_log, "missing copyright or crown copyright statement", 0 ) @@ -956,16 +935,14 @@ def check_code_owner(self, lines: List[str]) -> TestResult: file_content = "\n".join(lines) found_code_owner = False error_log = {} - if ("Code Owner:" in file_content or - "code owner" in file_content.lower()): + if "Code Owner:" in file_content or "code owner" in file_content.lower(): # print(f"Debug: Found {file_content.lower()}") found_code_owner = True # This is often a warning rather than an error if not found_code_owner: self.add_extra_error("missing code owner comment") - error_log = self.add_error_log(error_log, - "missing code owner comment", 0) + error_log = self.add_error_log(error_log, "missing code owner comment", 0) return TestResult( checker_name="Code Owner Comment", failure_count=0 if found_code_owner else 1, @@ -1037,8 +1014,7 @@ def c_deprecated(self, lines: List[str]) -> int: for line in lines: for identifier in deprecated_c_identifiers: if re.search(rf"\b{identifier}\b", line): - self.add_extra_error( - f"deprecated C identifier: {identifier}") + self.add_extra_error(f"deprecated C identifier: {identifier}") failures += 1 return failures @@ -1065,8 +1041,7 @@ def c_openmp_define_no_combine(self, lines: List[str]) -> int: ) or re.search( r"&&.*_OPENMP.*&&.*SHUM_USE_C_OPENMP_VIA_THREAD_UTILS", line ): - self.add_extra_error( - "OpenMP defines combined with third macro") + self.add_extra_error("OpenMP defines combined with third macro") failures += 1 return failures diff --git a/script_umdp3_checker/umdp3_conformance.py b/script_umdp3_checker/umdp3_conformance.py index bb118b33..49ac429b 100644 --- a/script_umdp3_checker/umdp3_conformance.py +++ b/script_umdp3_checker/umdp3_conformance.py @@ -11,6 +11,7 @@ # Add custom modules to Python path if needed # Add the repository root to access fcm_bdiff and git_bdiff packages import sys + sys.path.insert(0, str(Path(__file__).parent.parent)) """ @@ -110,11 +111,11 @@ def get_branch_name(self) -> str: return self.bdiff_obj.branch -class StyleChecker(): +class StyleChecker: """A Class intended to coordinate the running of a set of style checks on - a set of files. - The checks are a dictionary of named callable routines. - The files are a list of file paths to check.""" + a set of files. + The checks are a dictionary of named callable routines. + The files are a list of file paths to check.""" """ TODO: This is where it might be good to set up a threadsafe @@ -152,8 +153,7 @@ def check(self, file_path: Path) -> CheckResult: for check_name, check_function in self.check_functions.items(): file_results.append(check_function(lines)) - tests_failed = sum([0 if result.passed else 1 for - result in file_results]) + tests_failed = sum([0 if result.passed else 1 for result in file_results]) return CheckResult( file_path=str(file_path), tests_failed=tests_failed, @@ -171,9 +171,7 @@ def from_full_list( print_volume: int = 3, ): files_to_check = ( - cls.filter_files(changed_files, file_extensions) - if changed_files - else [] + cls.filter_files(changed_files, file_extensions) if changed_files else [] ) if print_volume >= 5: print( @@ -204,9 +202,11 @@ def create_external_runners( ) -> "StyleChecker": """Create a StyleChecker instance filtering files from a full list.""" filtered_files = cls.filter_files(all_files, file_extensions) - print(f"Creating external runners for {name} with {len(commands)} " - f"commands and {len(filtered_files)} files to check from a " - f"total of {len(all_files)} files.") + print( + f"Creating external runners for {name} with {len(commands)} " + f"commands and {len(filtered_files)} files to check from a " + f"total of {len(all_files)} files." + ) check_functions = {} for command in commands: external_opname = f"External_operation_{command[0]}" @@ -215,17 +215,17 @@ def create_external_runners( return cls(name, check_functions, filtered_files) @staticmethod - def create_free_runner(command: List[str], - external_opname: str) -> Callable[[Path], - TestResult]: + def create_free_runner( + command: List[str], external_opname: str + ) -> Callable[[Path], TestResult]: """Method to create a free runner function for a given external command with it's checker name for output.""" + def new_free_runner(file_name: Path) -> TestResult: cmd = command + [str(file_name)] tests_failed = 0 try: - result = subprocess.run(cmd, capture_output=True, - text=True, timeout=60) + result = subprocess.run(cmd, capture_output=True, text=True, timeout=60) except subprocess.TimeoutExpired: failure_count = 1 passed = False @@ -249,13 +249,14 @@ def new_free_runner(file_name: Path) -> TestResult: errors = {} if result.returncode != 0: tests_failed += 1 - return (TestResult( + return TestResult( checker_name=external_opname, failure_count=failure_count, passed=passed, output=output, errors=errors, - )) + ) + return new_free_runner @@ -269,8 +270,7 @@ def check(self, file_path: Path) -> CheckResult: file_results = [] # list of TestResult objects for check_name, check_function in self.check_functions.items(): file_results.append(check_function(file_path)) - tests_failed = sum([0 if result.passed else 1 for result in - file_results]) + tests_failed = sum([0 if result.passed else 1 for result in file_results]) return CheckResult( file_path=str(file_path), tests_failed=tests_failed, @@ -339,8 +339,7 @@ def check_files(self) -> None: self.results = results return - def print_results(self, print_volume: int = 3, - quiet_pass: bool = True) -> bool: + def print_results(self, print_volume: int = 3, quiet_pass: bool = True) -> bool: """Print results and return True if all checks passed. ========================================================""" """ @@ -374,8 +373,7 @@ def print_results(self, print_volume: int = 3, for count, (title, info) in enumerate( test_result.errors.items() ): - print(" " * 8 + - f"{count + 1:2} : {title} : {info}") + 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") @@ -405,8 +403,7 @@ def process_arguments(): "-p", "--path", type=str, default="./", help="path to repository" ) parser.add_argument( - "--max-workers", type=int, default=8, - help="Maximum number of parallel workers" + "--max-workers", type=int, default=8, help="Maximum number of parallel workers" ) parser.add_argument( "--fullcheck", @@ -422,12 +419,10 @@ def process_arguments(): ) group = parser.add_mutually_exclusive_group() group.add_argument( - "-v", "--verbose", action="count", default=0, - help="Increase output verbosity" + "-v", "--verbose", action="count", default=0, help="Increase output verbosity" ) group.add_argument( - "-q", "--quiet", action="count", default=0, - help="Decrease output verbosity" + "-q", "--quiet", action="count", default=0, help="Decrease output verbosity" ) # The following are not yet implemented, but may become useful # branch and base branch could be used to configure the CMS diff @@ -531,18 +526,15 @@ def create_style_checkers( dispatch_tables = CheckerDispatchTables() checkers = [] if "Fortran" in file_types: - file_extensions = {".f", ".for", ".f90", ".f95", - ".f03", ".f08", ".F90"} + file_extensions = {".f", ".for", ".f90", ".f95", ".f03", ".f08", ".F90"} 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() if print_volume >= 3: print("Configuring Fortran checkers:") - combined_checkers = fortran_diff_table | fortran_file_table | \ - generic_file_table + combined_checkers = fortran_diff_table | fortran_file_table | generic_file_table fortran_file_checker = StyleChecker.from_full_list( - "Fortran Checker", file_extensions, - combined_checkers, changed_files + "Fortran Checker", file_extensions, combined_checkers, changed_files ) checkers.append(fortran_file_checker) if "Python" in file_types: @@ -551,14 +543,16 @@ def create_style_checkers( external_commands = [ ["ruff", "check"], """TODO : The following need 'tweaking' to replicate what's run as - part of the CI on GitHub.""" + part of the CI on GitHub.""", # ["flake8", "-q"], # ["black", "--check"], # ["pylint", "-E"], ] python_file_checker = Check_Runner.create_external_runners( - "Python External Checkers", external_commands, changed_files, - file_extensions + "Python External Checkers", + external_commands, + changed_files, + file_extensions, ) checkers.append(python_file_checker) if "Generic" in file_types or file_types == []: @@ -566,8 +560,7 @@ def create_style_checkers( print("Configuring Generic File Checkers:") all_file_dispatch_table = dispatch_tables.get_file_dispatch_table_all() generic_checker = StyleChecker( - "Generic File Checker", all_file_dispatch_table, - changed_files + "Generic File Checker", all_file_dispatch_table, changed_files ) checkers.append(generic_checker) @@ -647,14 +640,12 @@ def get_files_to_check( print(line_1(81) + "\n") else: print("Results :") - all_passed = checker.print_results(print_volume=log_volume, - quiet_pass=quiet_pass) + 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: {sum(1 for r in checker.results if - not r.all_passed)}") + print(f"Total files failed: {sum(1 for r in checker.results if not r.all_passed)}") exit(0 if all_passed else 1) From a012b8b5ddddea58c9ac12a32072dba5fe34178f Mon Sep 17 00:00:00 2001 From: r-sharp Date: Mon, 2 Mar 2026 17:46:49 +0000 Subject: [PATCH 08/15] Add some basic tests --- .../tests/test_umdp3_conformance.py | 182 ++++++++++++++++++ script_umdp3_checker/umdp3_conformance.py | 5 +- 2 files changed, 186 insertions(+), 1 deletion(-) create mode 100644 script_umdp3_checker/tests/test_umdp3_conformance.py diff --git a/script_umdp3_checker/tests/test_umdp3_conformance.py b/script_umdp3_checker/tests/test_umdp3_conformance.py new file mode 100644 index 00000000..64ebadcc --- /dev/null +++ b/script_umdp3_checker/tests/test_umdp3_conformance.py @@ -0,0 +1,182 @@ +import sys +import subprocess +from pathlib import Path +from types import SimpleNamespace +import pytest + +# Add the current directory to Python path +sys.path.insert(0, str(Path(__file__).parent.parent)) + +from umdp3_conformance import StyleChecker, detangle_file_types, \ + ALLOWABLE_FILE_TYPES, GROUP_FILE_TYPES + + +def test_from_full_list_filters_by_extension(): + changed_files = [ + Path("src/code.F90"), + Path("src/module.f"), + Path("src/script.py"), + Path("README.md"), + ] + check_functions = {"dummy_check": lambda lines: None} + + checker = StyleChecker.from_full_list( + name="Fortran Checker", + file_extensions={".F90", ".f"}, + check_functions=check_functions, + changed_files=changed_files, + ) + + assert checker.get_name() == "Fortran Checker" + assert checker.check_functions == check_functions + assert checker.files_to_check == [Path("src/code.F90"), Path("src/module.f")] + + +def test_from_full_list_returns_all_files_when_no_extensions(): + changed_files = [Path("a.F90"), Path("b.py"), Path("c.txt")] + + checker = StyleChecker.from_full_list( + name="Any Checker", + file_extensions=set(), + check_functions={}, + changed_files=changed_files, + ) + + assert checker.files_to_check == changed_files + + +def test_from_full_list_with_no_changed_files_returns_empty_list(): + checker = StyleChecker.from_full_list( + name="Empty Checker", + file_extensions={".py"}, + check_functions={}, + changed_files=[], + ) + + assert checker.files_to_check == [] + + +def test_from_full_list_verbose_output(capsys): + changed_files = [Path("a.py"), Path("b.F90")] + + StyleChecker.from_full_list( + name="Verbose Checker", + file_extensions={".py"}, + check_functions={"dummy_check": lambda lines: None}, + changed_files=changed_files, + print_volume=5, + ) + + output = capsys.readouterr().out + assert "StyleChecker initialized using a filtered list:" in output + assert "Name : Verbose Checker" in output + assert "Has 1 check commands" in output + assert "Using 1 file extensions" in output + assert "Gives 1 files to check." in output + + +def test_filter_files_with_extensions_only_returns_matches(): + files = [Path("a.py"), Path("b.F90"), Path("c.txt"), Path("d.py")] + + result = StyleChecker.filter_files(files, {".py", ".F90"}) + + assert result == [Path("a.py"), Path("b.F90"), Path("d.py")] + + +def test_filter_files_with_empty_extensions_returns_original_list(): + files = [Path("a.py"), Path("b.F90")] + + result = StyleChecker.filter_files(files, set()) + + assert result == files + + +def test_filter_files_with_no_matches_returns_empty_list(): + files = [Path("a.py"), Path("b.F90")] + + result = StyleChecker.filter_files(files, {".md"}) + + assert result == [] + + +def test_create_free_runner_success(monkeypatch): + def fake_run(cmd, capture_output, text, timeout): + return SimpleNamespace(returncode=0, stdout="all good", stderr="") + + monkeypatch.setattr("umdp3_conformance.subprocess.run", fake_run) + runner = StyleChecker.create_free_runner(["dummy_checker", "--flag"], "Dummy") + + result = runner(Path("sample.py")) + + assert result.checker_name == "Dummy" + assert result.failure_count == 0 + assert result.passed is True + assert result.output == "all good" + assert result.errors == {} + + +def test_create_free_runner_checker_failure(monkeypatch): + def fake_run(cmd, capture_output, text, timeout): + return SimpleNamespace(returncode=2, stdout="", stderr="some stderr") + + monkeypatch.setattr("umdp3_conformance.subprocess.run", fake_run) + runner = StyleChecker.create_free_runner(["dummy_checker"], "Dummy") + + result = runner(Path("sample.py")) + + assert result.checker_name == "Dummy" + assert result.failure_count == 1 + assert result.passed is False + assert result.errors == {"Dummy": "some stderr"} + + +def test_create_free_runner_timeout(monkeypatch): + def fake_run(cmd, capture_output, text, timeout): + raise subprocess.TimeoutExpired(cmd=cmd, timeout=timeout) + + monkeypatch.setattr("umdp3_conformance.subprocess.run", fake_run) + runner = StyleChecker.create_free_runner(["slow_checker"], "SlowChecker") + + result = runner(Path("sample.py")) + + assert result.checker_name == "SlowChecker" + assert result.failure_count == 1 + assert result.passed is False + assert "timed out" in result.output + assert result.errors == {"SlowChecker": "TimeoutExpired"} + + +def test_detangle_file_types_expands_ci_group(): + result = detangle_file_types({"CI"}) + + assert result == {"Fortran", "Python"} + assert result == GROUP_FILE_TYPES["CI"] + + +def test_detangle_file_types_expands_all_group(): + result = detangle_file_types({"ALL"}) + + assert result == {"Fortran", "Python", "Generic"} + assert result == GROUP_FILE_TYPES["ALL"] + assert result == set(ALLOWABLE_FILE_TYPES) + + + +def test_detangle_file_types_mixed_group_and_explicit_type(): + result = detangle_file_types({"CI", "Generic"}) + + assert result == {"Fortran", "Python", "Generic"} + assert result == GROUP_FILE_TYPES["CI"].union({"Generic"}) + + +def test_detangle_file_types_passthrough_without_groups(): + input_types = {"Fortran", "Generic"} + + result = detangle_file_types(set(input_types)) + + assert result == input_types + + +def test_detangle_file_types_raises_for_invalid_type(): + with pytest.raises(ValueError, match="Invalid file types specified"): + detangle_file_types({"Fortran", "Java"}) diff --git a/script_umdp3_checker/umdp3_conformance.py b/script_umdp3_checker/umdp3_conformance.py index 49ac429b..aa3ded1e 100644 --- a/script_umdp3_checker/umdp3_conformance.py +++ b/script_umdp3_checker/umdp3_conformance.py @@ -170,12 +170,15 @@ def from_full_list( changed_files: List[Path] = [], print_volume: int = 3, ): + """Create a StyleChecker instance whilst simultaneously filtering files + from a full list. + Returns a StyleChecker instance.""" files_to_check = ( cls.filter_files(changed_files, file_extensions) if changed_files else [] ) if print_volume >= 5: print( - f"ExternalChecker initialized :\n" + f"StyleChecker initialized using a filtered list:\n" f" Name : {name}\n" f" Has {len(check_functions)} check commands\n" f" Using {len(file_extensions)} file extensions\n" From 49a6e6f9b3946492b8afad1d57559e3189582409 Mon Sep 17 00:00:00 2001 From: r-sharp Date: Wed, 11 Mar 2026 13:09:20 +0000 Subject: [PATCH 09/15] Using suggestions from CoPilot, inseret what it considers the high priority checks missing from umdp3_conformance.py. That was fun --- .../tests/test_umdp3_conformance.py | 292 +++++++++++++++--- 1 file changed, 254 insertions(+), 38 deletions(-) diff --git a/script_umdp3_checker/tests/test_umdp3_conformance.py b/script_umdp3_checker/tests/test_umdp3_conformance.py index 64ebadcc..7e1237e2 100644 --- a/script_umdp3_checker/tests/test_umdp3_conformance.py +++ b/script_umdp3_checker/tests/test_umdp3_conformance.py @@ -8,7 +8,7 @@ sys.path.insert(0, str(Path(__file__).parent.parent)) from umdp3_conformance import StyleChecker, detangle_file_types, \ - ALLOWABLE_FILE_TYPES, GROUP_FILE_TYPES + ALLOWABLE_FILE_TYPES, GROUP_FILE_TYPES, Check_Runner, get_files_to_check, which_cms_is_it, ConformanceChecker, CheckResult def test_from_full_list_filters_by_extension(): @@ -56,25 +56,6 @@ def test_from_full_list_with_no_changed_files_returns_empty_list(): assert checker.files_to_check == [] -def test_from_full_list_verbose_output(capsys): - changed_files = [Path("a.py"), Path("b.F90")] - - StyleChecker.from_full_list( - name="Verbose Checker", - file_extensions={".py"}, - check_functions={"dummy_check": lambda lines: None}, - changed_files=changed_files, - print_volume=5, - ) - - output = capsys.readouterr().out - assert "StyleChecker initialized using a filtered list:" in output - assert "Name : Verbose Checker" in output - assert "Has 1 check commands" in output - assert "Using 1 file extensions" in output - assert "Gives 1 files to check." in output - - def test_filter_files_with_extensions_only_returns_matches(): files = [Path("a.py"), Path("b.F90"), Path("c.txt"), Path("d.py")] @@ -84,7 +65,7 @@ def test_filter_files_with_extensions_only_returns_matches(): def test_filter_files_with_empty_extensions_returns_original_list(): - files = [Path("a.py"), Path("b.F90")] + files = [Path("a.py"), Path("b.F90"), Path("c.txt")] result = StyleChecker.filter_files(files, set()) @@ -92,30 +73,34 @@ def test_filter_files_with_empty_extensions_returns_original_list(): def test_filter_files_with_no_matches_returns_empty_list(): - files = [Path("a.py"), Path("b.F90")] + files = [Path("a.py"), Path("b.F90"), Path("c.txt")] result = StyleChecker.filter_files(files, {".md"}) assert result == [] -def test_create_free_runner_success(monkeypatch): - def fake_run(cmd, capture_output, text, timeout): - return SimpleNamespace(returncode=0, stdout="all good", stderr="") +def test_create_free_runner_success(monkeypatch: pytest.MonkeyPatch): + """Test that create_free_runner returns a runner that correctly captures the + output of a successful command. We use monkeypatch to replace subprocess.run with + a fake function that simulates a successful command execution.""" + def fake_run(cmd, capture_output, text, timeout): + return SimpleNamespace(returncode=0, stdout="all good", stderr="") - monkeypatch.setattr("umdp3_conformance.subprocess.run", fake_run) - runner = StyleChecker.create_free_runner(["dummy_checker", "--flag"], "Dummy") + monkeypatch.setattr("umdp3_conformance.subprocess.run", fake_run) + runner = StyleChecker.create_free_runner(["dummy_checker", "--flag"], "Dummy") - result = runner(Path("sample.py")) + result = runner(Path("sample.py")) - assert result.checker_name == "Dummy" - assert result.failure_count == 0 - assert result.passed is True - assert result.output == "all good" - assert result.errors == {} + assert result.checker_name == "Dummy" + assert result.failure_count == 0 + assert result.passed is True + assert result.output == "all good" + assert result.errors == {} -def test_create_free_runner_checker_failure(monkeypatch): +def test_create_free_runner_checker_failure(monkeypatch: pytest.MonkeyPatch): + """As above, but returning an error code""" def fake_run(cmd, capture_output, text, timeout): return SimpleNamespace(returncode=2, stdout="", stderr="some stderr") @@ -130,7 +115,9 @@ def fake_run(cmd, capture_output, text, timeout): assert result.errors == {"Dummy": "some stderr"} -def test_create_free_runner_timeout(monkeypatch): +def test_create_free_runner_timeout(monkeypatch: pytest.MonkeyPatch): + """As above, but simulating a timeout by having the fake subprocess.run raise a + TimeoutExpired exception.""" def fake_run(cmd, capture_output, text, timeout): raise subprocess.TimeoutExpired(cmd=cmd, timeout=timeout) @@ -156,8 +143,6 @@ def test_detangle_file_types_expands_ci_group(): def test_detangle_file_types_expands_all_group(): result = detangle_file_types({"ALL"}) - assert result == {"Fortran", "Python", "Generic"} - assert result == GROUP_FILE_TYPES["ALL"] assert result == set(ALLOWABLE_FILE_TYPES) @@ -165,7 +150,6 @@ def test_detangle_file_types_expands_all_group(): def test_detangle_file_types_mixed_group_and_explicit_type(): result = detangle_file_types({"CI", "Generic"}) - assert result == {"Fortran", "Python", "Generic"} assert result == GROUP_FILE_TYPES["CI"].union({"Generic"}) @@ -180,3 +164,235 @@ def test_detangle_file_types_passthrough_without_groups(): def test_detangle_file_types_raises_for_invalid_type(): with pytest.raises(ValueError, match="Invalid file types specified"): detangle_file_types({"Fortran", "Java"}) + + +def test_stylechecker_check_aggregates_results(tmp_path: Path): + """Test that StyleChecker.check() correctly aggregates results from multiple check functions. + Here, seen is checking the lines passed to the checks are the ones written by this + test.""" + file_path = tmp_path / "sample.txt" + file_path.write_text("a\nb\n") + + seen = [] + + def check_pass(lines): + seen.append(lines) + return SimpleNamespace(passed=True) + + def check_fail(lines): + seen.append(lines) + return SimpleNamespace(passed=False) + + checker = StyleChecker("StyleChecker test", {"pass test": check_pass, "fail test": check_fail}, []) + result = checker.check(file_path) + + assert seen == [["a", "b"], ["a", "b"]] + assert result.file_path == str(file_path) + assert result.tests_failed == 1 + assert result.all_passed is False + assert len(result.test_results) == 2 + + +def test_check_runner_check_passes_path_not_lines(tmp_path: Path): + """Test that Check_Runner.check() passes the file path to the check function, not the file lines. This being it's sole difference from StyleChecker.""" + file_path = tmp_path / "sample.py" + file_path.write_text("print('x')\n") + + seen = [] + + def check_pass(path): + seen.append(path) + return SimpleNamespace(passed=True) + + def check_fail(path): + seen.append(path) + return SimpleNamespace(passed=False) + + checker = Check_Runner("CheckRunner test", {"pass test": check_pass, "fail test": check_fail}, []) + result = checker.check(file_path) + + assert seen == [file_path, file_path] + assert result.tests_failed == 1 + assert result.all_passed is False + + +def test_create_external_runners_builds_expected_checkers(monkeypatch: pytest.MonkeyPatch): + """Warning! when using monkeypatch, make sure you're testing your own code on not basic Python intrinsics. + In this case, in order to test the class method, StyleChecker.create_external_runners, we're patching StyleChecker.create_free_runner, which is another static method. In reality StyleChecker.create_free_runner does most of the heavy lifting and StyleChecker.create_external_runners is just a thin wrapper to turn a list (of lists) which form commands into a dictionary of callables.""" + callables = [] + + # (fake_)create_free_runner will be called once for each + # command, and it will receive the command and the generated + # external_opname. We want to capture those arguments to verify + # them later, and then return a dummy runner that always + # returns passed=True. + def fake_create_free_runner(command, external_opname): + callables.append((command, external_opname)) + return lambda _: SimpleNamespace(passed=True) + + monkeypatch.setattr( + StyleChecker, "create_free_runner", staticmethod(fake_create_free_runner) + ) + + checker = StyleChecker.create_external_runners( + "Python External Checkers", + commands=[["ruff", "check"], ["black", "--check"]], + all_files=[Path("a.py"), Path("b.txt"), Path("c.F90"), Path("d.py")], + file_extensions={".py"}, + ) + + # All_files got filtered to only those with 'py' extension. + assert checker.files_to_check == [Path("a.py"), Path("d.py")] + # The two commands get given generated 'names'. + assert set(checker.check_functions.keys()) == { + "External_operation_ruff", + "External_operation_black", + } + # Check the arguments passed to create_free_runner are as expected. + assert callables == [ + (["ruff", "check"], "External_operation_ruff"), + (["black", "--check"], "External_operation_black"), + ] + + +def test_get_files_to_check_full_check_returns_all_files(tmp_path: Path): + (tmp_path / "a.txt").write_text("a") + (tmp_path / "sub").mkdir() + (tmp_path / "sub" / "b.py").write_text("b") + + result = get_files_to_check(str(tmp_path), full_check=True, print_volume=0) + + assert set(result) == {tmp_path / "a.txt", tmp_path / "sub" / "b.py"} + + +def test_get_files_to_check_uses_cms_when_not_fullcheck(monkeypatch: pytest.MonkeyPatch): + class FakeCMS: + def get_changed_files(self): + return [Path("x.py"), Path("y.F90")] + + monkeypatch.setattr("umdp3_conformance.which_cms_is_it", lambda p, v: FakeCMS()) + + result = get_files_to_check("repo", full_check=False, print_volume=0) + + assert result == [Path("x.py"), Path("y.F90")] + + +def test_which_cms_is_it_git_branch(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + """CoPilot suggested this one. We're testing that the check for a ".git" directory + returned true from a directory we created and put a ".git" directory in...""" + (tmp_path / ".git").mkdir() + + class FakeGitWrapper: + def __init__(self, repo_path): + self.repo_path = repo_path + + def get_branch_name(self): + return "feature/test" + + def is_branch(self): + return True + + def get_changed_files(self): + return [Path("a.py")] + + monkeypatch.setattr("umdp3_conformance.GitBdiffWrapper", FakeGitWrapper) + + cms = which_cms_is_it(str(tmp_path), print_volume=0) + + assert isinstance(cms, FakeGitWrapper) + assert cms.repo_path == tmp_path + + +def test_which_cms_is_it_raises_for_unknown_cms(tmp_path: Path): + with pytest.raises(RuntimeError, match="Unknown CMS type"): + which_cms_is_it(str(tmp_path), print_volume=0) + + +def test_which_cms_is_it_exits_when_not_branch(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + (tmp_path / ".git").mkdir() + + class FakeGitWrapper: + def __init__(self, repo_path): + self.repo_path = repo_path + + def get_branch_name(self): + return "main" + + def is_branch(self): + return False + + def get_changed_files(self): + return [] + + monkeypatch.setattr("umdp3_conformance.GitBdiffWrapper", FakeGitWrapper) + + with pytest.raises(SystemExit) as exc: + which_cms_is_it(str(tmp_path), print_volume=0) + assert exc.value.code == 0 + + +def test_conformance_checker_check_files_collects_all_results(): + class DummyChecker: + def __init__(self, files): + self.files_to_check = files + + def check(self, file_path): + return CheckResult( + file_path=str(file_path), + tests_failed=0, + all_passed=True, + test_results=[], + ) + + checkers = [ + DummyChecker([Path("a.py"), Path("b.py")]), + DummyChecker([Path("c.py")]), + ] + cc = ConformanceChecker(checkers, max_workers=2) # type: ignore + + cc.check_files() + + assert len(cc.results) == 3 + assert {r.file_path for r in cc.results} == {"a.py", "b.py", "c.py"} + + +""" +These last two are CoPilot Specials. I'm not sure we need to test the logic of +changing print verbosity, but we've got them now. Perhaps not worth maintaining though. +""" +def test_conformance_checker_print_results_quiet_pass_suppresses_passed(capsys: pytest.CaptureFixture[str]): + fail_tr = SimpleNamespace( + checker_name="RuleX", failure_count=1, passed=False, errors={"RuleX": "bad"} + ) + pass_tr = SimpleNamespace( + checker_name="RuleY", failure_count=0, passed=True, errors={} + ) + + cc = ConformanceChecker([]) + cc.results = [ + CheckResult("ok.py", tests_failed=0, all_passed=True, test_results=[pass_tr]), # type: ignore + CheckResult("bad.py", tests_failed=1, all_passed=False, test_results=[fail_tr]), # type: ignore + ] + + all_passed = cc.print_results(print_volume=3, quiet_pass=True) + out = capsys.readouterr().out + + assert all_passed is False + assert "bad.py" in out + assert "ok.py" not in out + + +def test_conformance_checker_print_results_volume4_prints_error_details(capsys: pytest.CaptureFixture[str]): + fail_tr = SimpleNamespace( + checker_name="RuleZ", failure_count=2, passed=False, errors={"RuleZ": "detail"} + ) + + cc = ConformanceChecker([]) + cc.results = [CheckResult("bad.py", tests_failed=1, all_passed=False, + test_results=[fail_tr])] # type: ignore + + cc.print_results(print_volume=4, quiet_pass=True) + out = capsys.readouterr().out + + assert "RuleZ" in out + assert "detail" in out \ No newline at end of file From f30894dacdfded5fb154d7a757a38465f88e1178 Mon Sep 17 00:00:00 2001 From: r-sharp Date: Wed, 11 Mar 2026 15:32:55 +0000 Subject: [PATCH 10/15] ScoobyDoobyDOOOOoooooooo --- script_umdp3_checker/umdp3_checker_rules.py | 2 +- script_umdp3_checker/umdp3_conformance.py | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/script_umdp3_checker/umdp3_checker_rules.py b/script_umdp3_checker/umdp3_checker_rules.py index e14ffe1d..5aea535b 100644 --- a/script_umdp3_checker/umdp3_checker_rules.py +++ b/script_umdp3_checker/umdp3_checker_rules.py @@ -342,7 +342,7 @@ def lowercase_variable_names(self, lines: List[str]) -> TestResult: self.add_extra_error(f"UPPERCASE variable name : {match[1]}") failures += 1 error_log = self.add_error_log( - error_log, f"UPPERCASE variable name {match[1]}", count + 1 + error_log, f"UPPERCASE variable name {match[1]}", count ) output = f"Checked {count + 1} lines, found {failures} failures." diff --git a/script_umdp3_checker/umdp3_conformance.py b/script_umdp3_checker/umdp3_conformance.py index 7848cb13..856b51f9 100644 --- a/script_umdp3_checker/umdp3_conformance.py +++ b/script_umdp3_checker/umdp3_conformance.py @@ -165,20 +165,17 @@ def check(self, file_path: Path) -> CheckResult: def from_full_list( cls, name: str, - file_extensions: Set[str] = set(), - check_functions: Dict[str, Callable] = {}, - changed_files: List[Path] = [], + file_extensions: Set[str], + check_functions: Dict[str, Callable], + changed_files: List[Path], print_volume: int = 3, ): - """Create a StyleChecker instance whilst simultaneously filtering files - from a full list. - Returns a StyleChecker instance.""" files_to_check = ( cls.filter_files(changed_files, file_extensions) if changed_files else [] ) if print_volume >= 5: print( - f"StyleChecker initialized using a filtered list:\n" + f"ExternalChecker initialized :\n" f" Name : {name}\n" f" Has {len(check_functions)} check commands\n" f" Using {len(file_extensions)} file extensions\n" @@ -188,7 +185,7 @@ def from_full_list( @staticmethod def filter_files( - files: List[Path], file_extensions: Set[str] = set() + files: List[Path], file_extensions: Optional[Set[str]] = None ) -> List[Path]: """Filter files based on the checker's file extensions.""" if not file_extensions: @@ -201,7 +198,7 @@ def create_external_runners( name: str, commands: List[List[str]], all_files: List[Path], - file_extensions: Set[str] = set(), + file_extensions: Set[str], ) -> "StyleChecker": """Create a StyleChecker instance filtering files from a full list.""" filtered_files = cls.filter_files(all_files, file_extensions) @@ -526,6 +523,9 @@ def create_style_checkers( checkers = [] if "Fortran" in file_types: file_extensions = {".f", ".for", ".f90", ".f95", ".f03", ".f08", ".F90"} + """ + TODO : I /think/ the old version also checked '.h' files as Fortran. + Not sure if that is still needed.""" 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() From 3e83a44efed2e3778e6d8a064449326ce9d9eee6 Mon Sep 17 00:00:00 2001 From: r-sharp Date: Wed, 11 Mar 2026 15:37:04 +0000 Subject: [PATCH 11/15] Minor tweak to output text that seems to have got lost in previous branches. --- script_umdp3_checker/umdp3_conformance.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script_umdp3_checker/umdp3_conformance.py b/script_umdp3_checker/umdp3_conformance.py index 856b51f9..9aad49f2 100644 --- a/script_umdp3_checker/umdp3_conformance.py +++ b/script_umdp3_checker/umdp3_conformance.py @@ -175,7 +175,7 @@ def from_full_list( ) if print_volume >= 5: print( - f"ExternalChecker initialized :\n" + f"StyleChecker initialized using filtered full list:\n" f" Name : {name}\n" f" Has {len(check_functions)} check commands\n" f" Using {len(file_extensions)} file extensions\n" From 84885e088c5bfbe974f7f8d90a4aff2f64bbe858 Mon Sep 17 00:00:00 2001 From: r-sharp Date: Wed, 18 Mar 2026 17:50:02 +0000 Subject: [PATCH 12/15] Tweaking the tests and taking suggestions from CoPilot for further tests - I think the power has gone to it's head... --- .../tests/test_umdp3_conformance.py | 225 ++++++++++++++++-- 1 file changed, 211 insertions(+), 14 deletions(-) diff --git a/script_umdp3_checker/tests/test_umdp3_conformance.py b/script_umdp3_checker/tests/test_umdp3_conformance.py index 7e1237e2..cf95b847 100644 --- a/script_umdp3_checker/tests/test_umdp3_conformance.py +++ b/script_umdp3_checker/tests/test_umdp3_conformance.py @@ -8,7 +8,7 @@ sys.path.insert(0, str(Path(__file__).parent.parent)) from umdp3_conformance import StyleChecker, detangle_file_types, \ - ALLOWABLE_FILE_TYPES, GROUP_FILE_TYPES, Check_Runner, get_files_to_check, which_cms_is_it, ConformanceChecker, CheckResult + ALLOWABLE_FILE_TYPES, GROUP_FILE_TYPES, Check_Runner, get_files_to_check, line_1, line_2, which_cms_is_it, ConformanceChecker, CheckResult, process_arguments def test_from_full_list_filters_by_extension(): @@ -260,11 +260,30 @@ def test_get_files_to_check_full_check_returns_all_files(tmp_path: Path): (tmp_path / "sub").mkdir() (tmp_path / "sub" / "b.py").write_text("b") - result = get_files_to_check(str(tmp_path), full_check=True, print_volume=0) + result = get_files_to_check(str(tmp_path), full_check=True) - assert set(result) == {tmp_path / "a.txt", tmp_path / "sub" / "b.py"} + # Function contract is relative paths when full_check=True + assert set(result) == {Path("a.txt"), Path("sub/b.py")} +def test_get_files_to_check_passes_path_and_volume_to_cms(monkeypatch: pytest.MonkeyPatch): + calls = [] + + class FakeCMS: + def get_changed_files(self): + return [Path("x.py")] + + def fake_which(path, volume): + calls.append((path, volume)) + return FakeCMS() + + monkeypatch.setattr("umdp3_conformance.which_cms_is_it", fake_which) + + result = get_files_to_check("repo", full_check=False, print_volume=7) + + assert result == [Path("x.py")] + assert calls == [("repo", 7)] + def test_get_files_to_check_uses_cms_when_not_fullcheck(monkeypatch: pytest.MonkeyPatch): class FakeCMS: def get_changed_files(self): @@ -303,6 +322,30 @@ def get_changed_files(self): assert cms.repo_path == tmp_path +def test_which_cms_is_it_svn_branch(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + (tmp_path / ".svn").mkdir() + + class FakeFCMWrapper: + def __init__(self, repo_path): + self.repo_path = repo_path + + def get_branch_name(self): + return "fcm/branch" + + def is_branch(self): + return True + + def get_changed_files(self): + return [Path("x.F90")] + + monkeypatch.setattr("umdp3_conformance.FCMBdiffWrapper", FakeFCMWrapper) + + cms = which_cms_is_it(str(tmp_path), print_volume=0) + + assert isinstance(cms, FakeFCMWrapper) + assert cms.repo_path == tmp_path + + def test_which_cms_is_it_raises_for_unknown_cms(tmp_path: Path): with pytest.raises(RuntimeError, match="Unknown CMS type"): which_cms_is_it(str(tmp_path), print_volume=0) @@ -331,7 +374,41 @@ def get_changed_files(self): assert exc.value.code == 0 -def test_conformance_checker_check_files_collects_all_results(): +""" +These last few are CoPilot Specials. I'm not sure how much some are actually testing our +code and how much is testing MonkeyPatch and pytest. Equally some of the latter ones I +might argue aren't really worth having... but we've got them now. Perhaps +not worth maintaining though. +""" +class _FakeFuture: + def __init__(self, value=None, exc=None): + self._value = value + self._exc = exc + + def result(self): + if self._exc is not None: + raise self._exc + return self._value + + +class _FakeProcessPoolExecutor: + def __init__(self, max_workers=None): + self.max_workers = max_workers + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc, tb): + return False + + def submit(self, fn, *args, **kwargs): + try: + return _FakeFuture(value=fn(*args, **kwargs)) + except Exception as e: + return _FakeFuture(exc=e) + + +def test_conformance_checker_check_files_collects_all_results(monkeypatch: pytest.MonkeyPatch): class DummyChecker: def __init__(self, files): self.files_to_check = files @@ -344,23 +421,52 @@ def check(self, file_path): test_results=[], ) + monkeypatch.setattr( + "umdp3_conformance.concurrent.futures.ProcessPoolExecutor", + _FakeProcessPoolExecutor, + ) + monkeypatch.setattr( + "umdp3_conformance.concurrent.futures.as_completed", + lambda futures: list(futures), + ) + checkers = [ DummyChecker([Path("a.py"), Path("b.py")]), DummyChecker([Path("c.py")]), ] - cc = ConformanceChecker(checkers, max_workers=2) # type: ignore + cc = ConformanceChecker(checkers, max_workers=2) # type: ignore cc.check_files() - + # These asserts are basically just checking that the results from the two "dummy" checkers,one of which checked 2 files, are collected into a single list of three results objects. assert len(cc.results) == 3 assert {r.file_path for r in cc.results} == {"a.py", "b.py", "c.py"} -""" -These last two are CoPilot Specials. I'm not sure we need to test the logic of -changing print verbosity, but we've got them now. Perhaps not worth maintaining though. -""" -def test_conformance_checker_print_results_quiet_pass_suppresses_passed(capsys: pytest.CaptureFixture[str]): +def test_conformance_checker_check_files_propagates_worker_exception(monkeypatch: pytest.MonkeyPatch): + class BadChecker: + def __init__(self): + self.files_to_check = [Path("bad.py")] + + def check(self, file_path): + raise RuntimeError("boom") + + monkeypatch.setattr( + "umdp3_conformance.concurrent.futures.ProcessPoolExecutor", + _FakeProcessPoolExecutor, + ) + monkeypatch.setattr( + "umdp3_conformance.concurrent.futures.as_completed", + lambda futures: list(futures), + ) + + cc = ConformanceChecker([BadChecker()], max_workers=1) # type: ignore + + with pytest.raises(RuntimeError, match="boom"): + cc.check_files() + + +def test_conformance_checker_print_results_quiet_pass_suppresses_passed( + capsys: pytest.CaptureFixture[str]): fail_tr = SimpleNamespace( checker_name="RuleX", failure_count=1, passed=False, errors={"RuleX": "bad"} ) @@ -376,7 +482,8 @@ def test_conformance_checker_print_results_quiet_pass_suppresses_passed(capsys: all_passed = cc.print_results(print_volume=3, quiet_pass=True) out = capsys.readouterr().out - + """That seems to be a lot of effort to create fake results in a fake conformance + checker just to check the text from the 'passed' test wasnt in the output written""" assert all_passed is False assert "bad.py" in out assert "ok.py" not in out @@ -393,6 +500,96 @@ def test_conformance_checker_print_results_volume4_prints_error_details(capsys: cc.print_results(print_volume=4, quiet_pass=True) out = capsys.readouterr().out - + """Again, that seems to be a lot of effort to create fake results in a fake + conformance checker just to check the detail text from the test was in the output + written""" assert "RuleZ" in out - assert "detail" in out \ No newline at end of file + assert "detail" in out + + +def test_stylechecker_check_with_no_check_functions_passes(tmp_path: Path): + # CoPilot described this as an 'edge case'. I'm not sure it shouldn't be + # checked for in the code and a PEBCAK error raised. + file_path = tmp_path / "sample.txt" + file_path.write_text("content\n") + + checker = StyleChecker("EmptyChecks", {}, [file_path]) + result = checker.check(file_path) + + assert result.file_path == str(file_path) + assert result.tests_failed == 0 + assert result.all_passed is True + assert result.test_results == [] + + +def test_create_free_runner_generic_exception(monkeypatch: pytest.MonkeyPatch): + def fake_run(cmd, capture_output, text, timeout): + raise OSError("cannot execute") + + monkeypatch.setattr("umdp3_conformance.subprocess.run", fake_run) + runner = StyleChecker.create_free_runner(["dummy_checker"], "Dummy") + + result = runner(Path("sample.py")) + + assert result.checker_name == "Dummy" + assert result.failure_count == 1 + assert result.passed is False + assert result.errors == {"Dummy": "cannot execute"} + + +def test_create_external_runners_with_empty_commands(): + # Again, is this something we should be ensuring works, or testing for in the code + # to raise a PEBCAK error if this is ever the case? Although it's a 'coder error' + # not a user error if it does. + checker = StyleChecker.create_external_runners( + "Noop External", + commands=[], + all_files=[Path("a.py"), Path("b.py")], + file_extensions={".py"}, + ) + + assert checker.files_to_check == [Path("a.py"), Path("b.py")] + assert checker.check_functions == {} + +# Okay, c'mon CoPilot - where's the value in testing a tiny function to write a string +# of characters from a runtime supplied length is the length you asked for ? +def test_line_helpers_basic_shapes(): + assert line_1(0) == "" + assert len(line_1(80)) == 80 + assert line_2(5) == "-----" + + +# hmmmm, isn't this really just testing that argparse is doing what we expect it to do +# with the arguments we give it. Again, is this something we should be testing for, or +# just ensuring that our code is using argparse correctly and trusting that argparse +# itself works as advertised ? +def test_process_arguments_defaults(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setattr(sys, "argv", ["umdp3_conformance.py"]) + + args = process_arguments() + # All the below are the defaults assigned if no args were given... + assert args.path == "./" + assert args.max_workers == 2 + assert args.fullcheck is False + assert args.printpass is False + assert args.volume == 3 + assert args.file_types == {"Fortran"} + + +def test_process_arguments_verbose_and_filetype_group_expansion(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setattr( + sys, + "argv", + ["umdp3_conformance.py", "--file-types", "CI", "Generic", "-vv"], + ) + + args = process_arguments() + + assert args.volume == 5 + assert args.file_types == {"Fortran", "Python", "Generic"} + + +def test_process_arguments_rejects_verbose_and_quiet_together(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setattr(sys, "argv", ["umdp3_conformance.py", "-v", "-q"]) + with pytest.raises(SystemExit): + process_arguments() \ No newline at end of file From 58a5b2f958d7456e5036d34a9ea732a4175fd94c Mon Sep 17 00:00:00 2001 From: r-sharp Date: Thu, 19 Mar 2026 16:38:26 +0000 Subject: [PATCH 13/15] adapting to the latest crazy linting settings --- .../tests/test_umdp3_conformance.py | 248 ++++++++++-------- 1 file changed, 145 insertions(+), 103 deletions(-) diff --git a/script_umdp3_checker/tests/test_umdp3_conformance.py b/script_umdp3_checker/tests/test_umdp3_conformance.py index cf95b847..64fe35f6 100644 --- a/script_umdp3_checker/tests/test_umdp3_conformance.py +++ b/script_umdp3_checker/tests/test_umdp3_conformance.py @@ -7,83 +7,96 @@ # Add the current directory to Python path sys.path.insert(0, str(Path(__file__).parent.parent)) -from umdp3_conformance import StyleChecker, detangle_file_types, \ - ALLOWABLE_FILE_TYPES, GROUP_FILE_TYPES, Check_Runner, get_files_to_check, line_1, line_2, which_cms_is_it, ConformanceChecker, CheckResult, process_arguments +from umdp3_conformance import ( + StyleChecker, + detangle_file_types, + ALLOWABLE_FILE_TYPES, + GROUP_FILE_TYPES, + Check_Runner, + get_files_to_check, + line_1, + line_2, + which_cms_is_it, + ConformanceChecker, + CheckResult, + process_arguments, +) def test_from_full_list_filters_by_extension(): - changed_files = [ - Path("src/code.F90"), - Path("src/module.f"), - Path("src/script.py"), - Path("README.md"), - ] - check_functions = {"dummy_check": lambda lines: None} - - checker = StyleChecker.from_full_list( - name="Fortran Checker", - file_extensions={".F90", ".f"}, - check_functions=check_functions, - changed_files=changed_files, - ) - - assert checker.get_name() == "Fortran Checker" - assert checker.check_functions == check_functions - assert checker.files_to_check == [Path("src/code.F90"), Path("src/module.f")] + changed_files = [ + Path("src/code.F90"), + Path("src/module.f"), + Path("src/script.py"), + Path("README.md"), + ] + check_functions = {"dummy_check": lambda lines: None} + + checker = StyleChecker.from_full_list( + name="Fortran Checker", + file_extensions={".F90", ".f"}, + check_functions=check_functions, + changed_files=changed_files, + ) + + assert checker.get_name() == "Fortran Checker" + assert checker.check_functions == check_functions + assert checker.files_to_check == [Path("src/code.F90"), Path("src/module.f")] def test_from_full_list_returns_all_files_when_no_extensions(): - changed_files = [Path("a.F90"), Path("b.py"), Path("c.txt")] + changed_files = [Path("a.F90"), Path("b.py"), Path("c.txt")] - checker = StyleChecker.from_full_list( - name="Any Checker", - file_extensions=set(), - check_functions={}, - changed_files=changed_files, - ) + checker = StyleChecker.from_full_list( + name="Any Checker", + file_extensions=set(), + check_functions={}, + changed_files=changed_files, + ) - assert checker.files_to_check == changed_files + assert checker.files_to_check == changed_files def test_from_full_list_with_no_changed_files_returns_empty_list(): - checker = StyleChecker.from_full_list( - name="Empty Checker", - file_extensions={".py"}, - check_functions={}, - changed_files=[], - ) + checker = StyleChecker.from_full_list( + name="Empty Checker", + file_extensions={".py"}, + check_functions={}, + changed_files=[], + ) - assert checker.files_to_check == [] + assert checker.files_to_check == [] def test_filter_files_with_extensions_only_returns_matches(): - files = [Path("a.py"), Path("b.F90"), Path("c.txt"), Path("d.py")] + files = [Path("a.py"), Path("b.F90"), Path("c.txt"), Path("d.py")] - result = StyleChecker.filter_files(files, {".py", ".F90"}) + result = StyleChecker.filter_files(files, {".py", ".F90"}) - assert result == [Path("a.py"), Path("b.F90"), Path("d.py")] + assert result == [Path("a.py"), Path("b.F90"), Path("d.py")] def test_filter_files_with_empty_extensions_returns_original_list(): - files = [Path("a.py"), Path("b.F90"), Path("c.txt")] + files = [Path("a.py"), Path("b.F90"), Path("c.txt")] - result = StyleChecker.filter_files(files, set()) + result = StyleChecker.filter_files(files, set()) - assert result == files + assert result == files def test_filter_files_with_no_matches_returns_empty_list(): - files = [Path("a.py"), Path("b.F90"), Path("c.txt")] + files = [Path("a.py"), Path("b.F90"), Path("c.txt")] - result = StyleChecker.filter_files(files, {".md"}) + result = StyleChecker.filter_files(files, {".md"}) - assert result == [] + assert result == [] def test_create_free_runner_success(monkeypatch: pytest.MonkeyPatch): """Test that create_free_runner returns a runner that correctly captures the - output of a successful command. We use monkeypatch to replace subprocess.run with - a fake function that simulates a successful command execution.""" + output of a successful command. We use monkeypatch to replace subprocess.run with + a fake function that simulates a successful command execution.""" + def fake_run(cmd, capture_output, text, timeout): return SimpleNamespace(returncode=0, stdout="all good", stderr="") @@ -100,70 +113,71 @@ def fake_run(cmd, capture_output, text, timeout): def test_create_free_runner_checker_failure(monkeypatch: pytest.MonkeyPatch): - """As above, but returning an error code""" - def fake_run(cmd, capture_output, text, timeout): - return SimpleNamespace(returncode=2, stdout="", stderr="some stderr") + """As above, but returning an error code""" - monkeypatch.setattr("umdp3_conformance.subprocess.run", fake_run) - runner = StyleChecker.create_free_runner(["dummy_checker"], "Dummy") + def fake_run(cmd, capture_output, text, timeout): + return SimpleNamespace(returncode=2, stdout="", stderr="some stderr") - result = runner(Path("sample.py")) + monkeypatch.setattr("umdp3_conformance.subprocess.run", fake_run) + runner = StyleChecker.create_free_runner(["dummy_checker"], "Dummy") - assert result.checker_name == "Dummy" - assert result.failure_count == 1 - assert result.passed is False - assert result.errors == {"Dummy": "some stderr"} + result = runner(Path("sample.py")) + + assert result.checker_name == "Dummy" + assert result.failure_count == 1 + assert result.passed is False + assert result.errors == {"Dummy": "some stderr"} def test_create_free_runner_timeout(monkeypatch: pytest.MonkeyPatch): - """As above, but simulating a timeout by having the fake subprocess.run raise a + """As above, but simulating a timeout by having the fake subprocess.run raise a TimeoutExpired exception.""" - def fake_run(cmd, capture_output, text, timeout): - raise subprocess.TimeoutExpired(cmd=cmd, timeout=timeout) - monkeypatch.setattr("umdp3_conformance.subprocess.run", fake_run) - runner = StyleChecker.create_free_runner(["slow_checker"], "SlowChecker") + def fake_run(cmd, capture_output, text, timeout): + raise subprocess.TimeoutExpired(cmd=cmd, timeout=timeout) + + monkeypatch.setattr("umdp3_conformance.subprocess.run", fake_run) + runner = StyleChecker.create_free_runner(["slow_checker"], "SlowChecker") - result = runner(Path("sample.py")) + result = runner(Path("sample.py")) - assert result.checker_name == "SlowChecker" - assert result.failure_count == 1 - assert result.passed is False - assert "timed out" in result.output - assert result.errors == {"SlowChecker": "TimeoutExpired"} + assert result.checker_name == "SlowChecker" + assert result.failure_count == 1 + assert result.passed is False + assert "timed out" in result.output + assert result.errors == {"SlowChecker": "TimeoutExpired"} def test_detangle_file_types_expands_ci_group(): - result = detangle_file_types({"CI"}) + result = detangle_file_types({"CI"}) - assert result == {"Fortran", "Python"} - assert result == GROUP_FILE_TYPES["CI"] + assert result == {"Fortran", "Python"} + assert result == GROUP_FILE_TYPES["CI"] def test_detangle_file_types_expands_all_group(): - result = detangle_file_types({"ALL"}) - - assert result == set(ALLOWABLE_FILE_TYPES) + result = detangle_file_types({"ALL"}) + assert result == set(ALLOWABLE_FILE_TYPES) def test_detangle_file_types_mixed_group_and_explicit_type(): - result = detangle_file_types({"CI", "Generic"}) + result = detangle_file_types({"CI", "Generic"}) - assert result == GROUP_FILE_TYPES["CI"].union({"Generic"}) + assert result == GROUP_FILE_TYPES["CI"].union({"Generic"}) def test_detangle_file_types_passthrough_without_groups(): - input_types = {"Fortran", "Generic"} + input_types = {"Fortran", "Generic"} - result = detangle_file_types(set(input_types)) + result = detangle_file_types(set(input_types)) - assert result == input_types + assert result == input_types def test_detangle_file_types_raises_for_invalid_type(): - with pytest.raises(ValueError, match="Invalid file types specified"): - detangle_file_types({"Fortran", "Java"}) + with pytest.raises(ValueError, match="Invalid file types specified"): + detangle_file_types({"Fortran", "Java"}) def test_stylechecker_check_aggregates_results(tmp_path: Path): @@ -183,7 +197,9 @@ def check_fail(lines): seen.append(lines) return SimpleNamespace(passed=False) - checker = StyleChecker("StyleChecker test", {"pass test": check_pass, "fail test": check_fail}, []) + checker = StyleChecker( + "StyleChecker test", {"pass test": check_pass, "fail test": check_fail}, [] + ) result = checker.check(file_path) assert seen == [["a", "b"], ["a", "b"]] @@ -208,7 +224,9 @@ def check_fail(path): seen.append(path) return SimpleNamespace(passed=False) - checker = Check_Runner("CheckRunner test", {"pass test": check_pass, "fail test": check_fail}, []) + checker = Check_Runner( + "CheckRunner test", {"pass test": check_pass, "fail test": check_fail}, [] + ) result = checker.check(file_path) assert seen == [file_path, file_path] @@ -216,16 +234,18 @@ def check_fail(path): assert result.all_passed is False -def test_create_external_runners_builds_expected_checkers(monkeypatch: pytest.MonkeyPatch): +def test_create_external_runners_builds_expected_checkers( + monkeypatch: pytest.MonkeyPatch, +): """Warning! when using monkeypatch, make sure you're testing your own code on not basic Python intrinsics. In this case, in order to test the class method, StyleChecker.create_external_runners, we're patching StyleChecker.create_free_runner, which is another static method. In reality StyleChecker.create_free_runner does most of the heavy lifting and StyleChecker.create_external_runners is just a thin wrapper to turn a list (of lists) which form commands into a dictionary of callables.""" callables = [] - # (fake_)create_free_runner will be called once for each - # command, and it will receive the command and the generated - # external_opname. We want to capture those arguments to verify - # them later, and then return a dummy runner that always - # returns passed=True. + # (fake_)create_free_runner will be called once for each + # command, and it will receive the command and the generated + # external_opname. We want to capture those arguments to verify + # them later, and then return a dummy runner that always + # returns passed=True. def fake_create_free_runner(command, external_opname): callables.append((command, external_opname)) return lambda _: SimpleNamespace(passed=True) @@ -241,7 +261,7 @@ def fake_create_free_runner(command, external_opname): file_extensions={".py"}, ) - # All_files got filtered to only those with 'py' extension. + # All_files got filtered to only those with 'py' extension. assert checker.files_to_check == [Path("a.py"), Path("d.py")] # The two commands get given generated 'names'. assert set(checker.check_functions.keys()) == { @@ -266,7 +286,9 @@ def test_get_files_to_check_full_check_returns_all_files(tmp_path: Path): assert set(result) == {Path("a.txt"), Path("sub/b.py")} -def test_get_files_to_check_passes_path_and_volume_to_cms(monkeypatch: pytest.MonkeyPatch): +def test_get_files_to_check_passes_path_and_volume_to_cms( + monkeypatch: pytest.MonkeyPatch, +): calls = [] class FakeCMS: @@ -284,7 +306,10 @@ def fake_which(path, volume): assert result == [Path("x.py")] assert calls == [("repo", 7)] -def test_get_files_to_check_uses_cms_when_not_fullcheck(monkeypatch: pytest.MonkeyPatch): + +def test_get_files_to_check_uses_cms_when_not_fullcheck( + monkeypatch: pytest.MonkeyPatch, +): class FakeCMS: def get_changed_files(self): return [Path("x.py"), Path("y.F90")] @@ -351,7 +376,9 @@ def test_which_cms_is_it_raises_for_unknown_cms(tmp_path: Path): which_cms_is_it(str(tmp_path), print_volume=0) -def test_which_cms_is_it_exits_when_not_branch(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): +def test_which_cms_is_it_exits_when_not_branch( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): (tmp_path / ".git").mkdir() class FakeGitWrapper: @@ -380,6 +407,8 @@ def get_changed_files(self): might argue aren't really worth having... but we've got them now. Perhaps not worth maintaining though. """ + + class _FakeFuture: def __init__(self, value=None, exc=None): self._value = value @@ -408,7 +437,9 @@ def submit(self, fn, *args, **kwargs): return _FakeFuture(exc=e) -def test_conformance_checker_check_files_collects_all_results(monkeypatch: pytest.MonkeyPatch): +def test_conformance_checker_check_files_collects_all_results( + monkeypatch: pytest.MonkeyPatch, +): class DummyChecker: def __init__(self, files): self.files_to_check = files @@ -442,7 +473,9 @@ def check(self, file_path): assert {r.file_path for r in cc.results} == {"a.py", "b.py", "c.py"} -def test_conformance_checker_check_files_propagates_worker_exception(monkeypatch: pytest.MonkeyPatch): +def test_conformance_checker_check_files_propagates_worker_exception( + monkeypatch: pytest.MonkeyPatch, +): class BadChecker: def __init__(self): self.files_to_check = [Path("bad.py")] @@ -466,7 +499,8 @@ def check(self, file_path): def test_conformance_checker_print_results_quiet_pass_suppresses_passed( - capsys: pytest.CaptureFixture[str]): + capsys: pytest.CaptureFixture[str], +): fail_tr = SimpleNamespace( checker_name="RuleX", failure_count=1, passed=False, errors={"RuleX": "bad"} ) @@ -476,8 +510,8 @@ def test_conformance_checker_print_results_quiet_pass_suppresses_passed( cc = ConformanceChecker([]) cc.results = [ - CheckResult("ok.py", tests_failed=0, all_passed=True, test_results=[pass_tr]), # type: ignore - CheckResult("bad.py", tests_failed=1, all_passed=False, test_results=[fail_tr]), # type: ignore + CheckResult("ok.py", tests_failed=0, all_passed=True, test_results=[pass_tr]), # type: ignore + CheckResult("bad.py", tests_failed=1, all_passed=False, test_results=[fail_tr]), # type: ignore ] all_passed = cc.print_results(print_volume=3, quiet_pass=True) @@ -489,14 +523,17 @@ def test_conformance_checker_print_results_quiet_pass_suppresses_passed( assert "ok.py" not in out -def test_conformance_checker_print_results_volume4_prints_error_details(capsys: pytest.CaptureFixture[str]): +def test_conformance_checker_print_results_volume4_prints_error_details( + capsys: pytest.CaptureFixture[str], +): fail_tr = SimpleNamespace( checker_name="RuleZ", failure_count=2, passed=False, errors={"RuleZ": "detail"} ) cc = ConformanceChecker([]) - cc.results = [CheckResult("bad.py", tests_failed=1, all_passed=False, - test_results=[fail_tr])] # type: ignore + cc.results = [ + CheckResult("bad.py", tests_failed=1, all_passed=False, test_results=[fail_tr]) + ] # type: ignore cc.print_results(print_volume=4, quiet_pass=True) out = capsys.readouterr().out @@ -551,6 +588,7 @@ def test_create_external_runners_with_empty_commands(): assert checker.files_to_check == [Path("a.py"), Path("b.py")] assert checker.check_functions == {} + # Okay, c'mon CoPilot - where's the value in testing a tiny function to write a string # of characters from a runtime supplied length is the length you asked for ? def test_line_helpers_basic_shapes(): @@ -576,7 +614,9 @@ def test_process_arguments_defaults(monkeypatch: pytest.MonkeyPatch): assert args.file_types == {"Fortran"} -def test_process_arguments_verbose_and_filetype_group_expansion(monkeypatch: pytest.MonkeyPatch): +def test_process_arguments_verbose_and_filetype_group_expansion( + monkeypatch: pytest.MonkeyPatch, +): monkeypatch.setattr( sys, "argv", @@ -589,7 +629,9 @@ def test_process_arguments_verbose_and_filetype_group_expansion(monkeypatch: pyt assert args.file_types == {"Fortran", "Python", "Generic"} -def test_process_arguments_rejects_verbose_and_quiet_together(monkeypatch: pytest.MonkeyPatch): +def test_process_arguments_rejects_verbose_and_quiet_together( + monkeypatch: pytest.MonkeyPatch, +): monkeypatch.setattr(sys, "argv", ["umdp3_conformance.py", "-v", "-q"]) with pytest.raises(SystemExit): - process_arguments() \ No newline at end of file + process_arguments() From ffc3d86d76bdff2e4c3fb53d78883e7c476c1a2a Mon Sep 17 00:00:00 2001 From: r-sharp Date: Thu, 19 Mar 2026 17:08:49 +0000 Subject: [PATCH 14/15] move the type ignore comment which the autoformatter shifted to the next line where it no longer works... --- script_umdp3_checker/tests/test_umdp3_conformance.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/script_umdp3_checker/tests/test_umdp3_conformance.py b/script_umdp3_checker/tests/test_umdp3_conformance.py index 64fe35f6..c7d69b53 100644 --- a/script_umdp3_checker/tests/test_umdp3_conformance.py +++ b/script_umdp3_checker/tests/test_umdp3_conformance.py @@ -532,8 +532,8 @@ def test_conformance_checker_print_results_volume4_prints_error_details( cc = ConformanceChecker([]) cc.results = [ - CheckResult("bad.py", tests_failed=1, all_passed=False, test_results=[fail_tr]) - ] # type: ignore + CheckResult("bad.py", tests_failed=1, all_passed=False, test_results=[fail_tr]) # type: ignore + ] cc.print_results(print_volume=4, quiet_pass=True) out = capsys.readouterr().out From 13157c0c73a53d4dc409675a83b62e997bbd12fb Mon Sep 17 00:00:00 2001 From: r-sharp Date: Thu, 19 Mar 2026 17:12:46 +0000 Subject: [PATCH 15/15] Oh look ma - the linter didn't like the change made to appease the formatter, or was it the other way round... --- script_umdp3_checker/tests/test_umdp3_conformance.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script_umdp3_checker/tests/test_umdp3_conformance.py b/script_umdp3_checker/tests/test_umdp3_conformance.py index c7d69b53..51abd345 100644 --- a/script_umdp3_checker/tests/test_umdp3_conformance.py +++ b/script_umdp3_checker/tests/test_umdp3_conformance.py @@ -532,7 +532,7 @@ def test_conformance_checker_print_results_volume4_prints_error_details( cc = ConformanceChecker([]) cc.results = [ - CheckResult("bad.py", tests_failed=1, all_passed=False, test_results=[fail_tr]) # type: ignore + CheckResult("bad.py", tests_failed=1, all_passed=False, test_results=[fail_tr]) # type: ignore ] cc.print_results(print_volume=4, quiet_pass=True)