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