Skip to content

Commit 58fa29e

Browse files
r-sharpPierre-siddallericaneiningerjennyhicksonjames-bruten-mo
authored
Clarify output (#181)
* Tweaks to get VSCode's internal linters to quit whinging and make the ToDo plugin work nicely. * Touch O Tidying and ToDo commenting. * Adding abilty to force a check on all files. * Tidying up output a bit - round 1 * Ticking off a TODO * Tweaks to get VSCode's internal linters to quit whinging and make the… (#173) * Tweaks to get VSCode's internal linters to quit whinging and make the ToDo plugin work nicely. * Touch O Tidying and ToDo commenting. * reviewer request to Update script_umdp3_checker/umdp3_conformance.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Update script_umdp3_checker/umdp3_checker_rules.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Update script_umdp3_checker/umdp3_checker_rules.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Update script_umdp3_checker/umdp3_checker_rules.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Update script_umdp3_checker/umdp3_checker_rules.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Update script_umdp3_checker/umdp3_checker_rules.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Rreviwer requested changes that I was unable to commit via GitHub's web interface --------- Co-authored-by: Pierre Siddall <43399998+Pierre-siddall@users.noreply.github.com> Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Tweaks to get VSCode's internal linters to quit whinging and make the… (#173) * Tweaks to get VSCode's internal linters to quit whinging and make the ToDo plugin work nicely. * Touch O Tidying and ToDo commenting. * reviewer request to Update script_umdp3_checker/umdp3_conformance.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Update script_umdp3_checker/umdp3_checker_rules.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Update script_umdp3_checker/umdp3_checker_rules.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Update script_umdp3_checker/umdp3_checker_rules.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Update script_umdp3_checker/umdp3_checker_rules.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Update script_umdp3_checker/umdp3_checker_rules.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Rreviwer requested changes that I was unable to commit via GitHub's web interface --------- Co-authored-by: Pierre Siddall <43399998+Pierre-siddall@users.noreply.github.com> Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Tweaks to get VSCode's internal linters to quit whinging and make the… (#173) * Tweaks to get VSCode's internal linters to quit whinging and make the ToDo plugin work nicely. * Touch O Tidying and ToDo commenting. * reviewer request to Update script_umdp3_checker/umdp3_conformance.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Update script_umdp3_checker/umdp3_checker_rules.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Update script_umdp3_checker/umdp3_checker_rules.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Update script_umdp3_checker/umdp3_checker_rules.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Update script_umdp3_checker/umdp3_checker_rules.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Update script_umdp3_checker/umdp3_checker_rules.py Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Rreviwer requested changes that I was unable to commit via GitHub's web interface --------- Co-authored-by: Pierre Siddall <43399998+Pierre-siddall@users.noreply.github.com> Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> * Switch to safe exit when branch==main detected. * Update script_umdp3_checker/umdp3_conformance.py Co-authored-by: Pierre Siddall <43399998+Pierre-siddall@users.noreply.github.com> * remove redundent line Co-authored-by: Pierre Siddall <43399998+Pierre-siddall@users.noreply.github.com> * Reviewer suggestion : improving use of Path object handling Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --------- Co-authored-by: Pierre Siddall <43399998+Pierre-siddall@users.noreply.github.com> Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com> Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> Co-authored-by: James Bruten <109733895+james-bruten-mo@users.noreply.github.com>
1 parent daddfe4 commit 58fa29e

2 files changed

Lines changed: 105 additions & 49 deletions

File tree

‎script_umdp3_checker/umdp3_checker_rules.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ def openmp_sentinels_in_column_one(self, lines: List[str]) -> TestResult:
236236
)
237237
output = f"Checked {count+1} lines, found {failures} failures."
238238
return TestResult(
239-
checker_name="Capitalised Keywords",
239+
checker_name="OpenMP sentinels not in column one",
240240
failure_count=failures,
241241
passed=(failures == 0),
242242
output=output,
@@ -252,7 +252,8 @@ def unseparated_keywords(self, lines: List[str]) -> TestResult:
252252
if line.lstrip(" ").startswith("!"):
253253
continue
254254
clean_line = self.remove_quoted(line)
255-
for pattern in [f"\\b{kw}\\b" for kw in unseparated_keywords_list]:
255+
for pattern in [f"\\b{kw}\\b" for kw in
256+
unseparated_keywords_list]:
256257
if re.search(pattern, clean_line, re.IGNORECASE):
257258
self.add_extra_error(f"unseparated keyword in line: "
258259
f"{line.strip()}")

‎script_umdp3_checker/umdp3_conformance.py‎

Lines changed: 102 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ def __init__(
180180
file_extensions: Set[str],
181181
check_functions: Dict[str, Callable],
182182
changed_files: List[Path] = [],
183+
print_volume: int = 3,
183184
):
184185
self.name = name
185186
self.file_extensions = file_extensions or set()
@@ -189,12 +190,12 @@ def __init__(
189190
if changed_files
190191
else []
191192
)
192-
# Should wrap the following in some kind of verbosity control
193-
# print(f"UMDP3_checker initialized :\n"
194-
# f" Name : {self.name}\n"
195-
# f" Has {len(self.check_functions)} check functions\n"
196-
# f" Using {len(self.file_extensions)} file extensions\n"
197-
# f" Gives {len(self.files_to_check)} files to check.")
193+
if print_volume >= 5:
194+
print(f"UMDP3_checker initialized :\n"
195+
f" Name : {self.name}\n"
196+
f" Has {len(self.check_functions)} check functions\n"
197+
f" Using {len(self.file_extensions)} file extensions\n"
198+
f" Gives {len(self.files_to_check)} files to check.")
198199

199200
def get_name(self) -> str:
200201
return self.name
@@ -233,6 +234,7 @@ def __init__(
233234
file_extensions: Set[str],
234235
check_functions: Dict[str, List[str]],
235236
changed_files: List[Path],
237+
print_volume: int = 3,
236238
):
237239
self.name = name
238240
self.file_extensions = file_extensions or set()
@@ -242,12 +244,12 @@ def __init__(
242244
if changed_files
243245
else []
244246
)
245-
# Should wrap the following in some kind of verbosity control
246-
# print(f"ExternalChecker initialized :\n"
247-
# f" Name : {self.name}\n"
248-
# f" Has {len(self.check_commands)} check commands\n"
249-
# f" Using {len(self.file_extensions)} file extensions\n"
250-
# f" Gives {len(self.files_to_check)} files to check.")
247+
if print_volume >= 5:
248+
print(f"ExternalChecker initialized :\n"
249+
f" Name : {self.name}\n"
250+
f" Has {len(self.check_commands)} check commands\n"
251+
f" Using {len(self.file_extensions)} file extensions\n"
252+
f" Gives {len(self.files_to_check)} files to check.")
251253

252254
def get_name(self) -> str:
253255
return self.name
@@ -360,7 +362,8 @@ def check_files(self) -> None:
360362
self.results = results
361363
return
362364

363-
def print_results(self, print_volume: int = 3) -> bool:
365+
def print_results(self, print_volume: int = 3,
366+
quiet_pass: bool = True) -> bool:
364367
"""Print results and return True if all checks passed.
365368
========================================================"""
366369
"""
@@ -374,35 +377,31 @@ def print_results(self, print_volume: int = 3) -> bool:
374377
# Lousy variable names here: 'result' is the CheckResult for a file
375378
# which had multiple tests, so result.all_passed is for that file.
376379
all_passed = all_passed and result.all_passed
377-
if print_volume >= 2:
378-
print(f"{file_status:7s} file : {result.file_path:50s}")
379-
if print_volume < 4 and result.all_passed:
380+
# verbosity level 4 overides quiet_pass for file summary.
381+
if quiet_pass and result.all_passed and print_volume < 4:
380382
continue
383+
print(f"{file_status:7s} file : {result.file_path:50s}")
384+
if print_volume >= 3 and not result.all_passed:
385+
print(" " * 4 + line_2(86))
381386
for test_result in result.test_results:
382-
# TODO : The output logic here is a bit of a mess.
383387
if print_volume < 5 and test_result.passed:
384388
continue
385-
if print_volume >= 4:
386-
print(
387-
" " * 5
388-
+ "-" * 50
389-
+ " " * 5
390-
+ f"\n {test_result.checker_name} Output :\n"
391-
+ " " * 5
392-
+ f"{test_result.output}\n"
393-
+ " " * 5
394-
+ "-" * 50
395-
)
396-
if test_result.errors:
397-
print(" " * 5 + "-=-" * 30)
398-
print(" " * 5 + " Std Error :")
389+
if print_volume >= 3 and not test_result.passed:
390+
plural = "" if test_result.failure_count == 1 else "s"
391+
print(f" {test_result.checker_name:60s} : Found "
392+
+ f"{test_result.failure_count:3} failure{plural}.")
393+
if test_result.errors and print_volume >= 4:
394+
print(" " * 8 + line_2(82))
399395
for count, (title, info) in enumerate(
400396
test_result.errors.items()
401397
):
402-
print(f" {count:2} : {title} : {info}")
403-
print(" " * 5 + "-=-" * 30)
404-
elif print_volume > 2:
405-
print(f" {test_result.checker_name:60s} : ✗ FAIL")
398+
print(" " * 8 +
399+
f"{count + 1:2} : {title} : {info}")
400+
print(" " * 8 + line_2(82))
401+
elif print_volume >= 3:
402+
print(f" {test_result.checker_name:60s} : ✓ PASS")
403+
if print_volume >= 3 and not result.all_passed:
404+
print(" " * 4 + line_2(86))
406405
return all_passed
407406

408407

@@ -439,10 +438,16 @@ def process_arguments():
439438
"the repository"
440439
)
441440
parser.add_argument(
441+
"--printpass", action="store_true",
442+
help="Print details of passed checks as well as failed ones.\n"
443+
"By default, only failed checks are printed in detail."
444+
)
445+
group = parser.add_mutually_exclusive_group()
446+
group.add_argument(
442447
"-v", "--verbose", action="count", default=0,
443448
help="Increase output verbosity"
444449
)
445-
parser.add_argument(
450+
group.add_argument(
446451
"-q", "--quiet", action="count", default=0,
447452
help="Decrease output verbosity"
448453
)
@@ -461,6 +466,24 @@ def process_arguments():
461466
return args
462467

463468

469+
def line_1(length: int = 80) -> str:
470+
"""Helper function to print a line for separating output sections."""
471+
repeats = length // 3
472+
pads = length % 3
473+
line = ""
474+
if pads > 1:
475+
line += "="
476+
line += "-=-" * repeats
477+
if pads > 0:
478+
line += "="
479+
return line
480+
481+
482+
def line_2(length: int = 80) -> str:
483+
"""Helper function to print a line for separating output sections."""
484+
return "-" * length
485+
486+
464487
def which_cms_is_it(path: str, print_volume: int = 3) -> CMSSystem:
465488
"""Determine which CMS is in use based on the presence of certain files."""
466489
repo_path = Path(path)
@@ -475,25 +498,37 @@ def which_cms_is_it(path: str, print_volume: int = 3) -> CMSSystem:
475498
raise RuntimeError("Unknown CMS type at path: " + str(path))
476499
branch_name = cms.get_branch_name()
477500
if not cms.is_branch():
501+
# TODO : This /might/ be better as a raise ValueError to allow
502+
# printing the help message, but for now just print and exit.
478503
print(
479504
f"The path {path} is not a branch."
480505
f"\nReported branch name is : {branch_name}"
481506
"\nThe meaning of differences is unclear, and so"
482507
" checking is aborted.\n"
483508
f"Please try switching on the full check option"
484509
)
485-
exit(1)
510+
# Soft exit mainly so nightly testing on main doesn't flag failure.
511+
exit(0)
486512
else:
487-
print(f"The branch, {branch_name}, at path {path} is a branch.")
488-
if print_volume >= 5:
513+
if print_volume >= 2:
514+
print(f"Found branch, {branch_name}, at path {path}.")
515+
if print_volume >= 4:
489516
print("The files changed on this branch are:")
490-
for changed_file in cms.get_changed_files():
491-
print(f" {changed_file}")
517+
changed_files = cms.get_changed_files()
518+
no_of_changed_files = len(changed_files)
519+
extras = no_of_changed_files - 10
520+
if no_of_changed_files > 10:
521+
changed_files = changed_files[:10]
522+
for changed_file in changed_files:
523+
print(f" {changed_file}")
524+
if no_of_changed_files > 10:
525+
print(f" ... and {extras} more changed files.")
492526
return cms
493527

494528

495529
def create_style_checkers(
496-
file_types: List[str], changed_files: List[Path]
530+
file_types: List[str], changed_files: List[Path],
531+
print_volume: int = 3
497532
) -> List[StyleChecker]:
498533
"""Create style checkers based on requested file types."""
499534
dispatch_tables = CheckerDispatchTables()
@@ -504,7 +539,8 @@ def create_style_checkers(
504539
fortran_diff_table = dispatch_tables.get_diff_dispatch_table_fortran()
505540
fortran_file_table = dispatch_tables.get_file_dispatch_table_fortran()
506541
generic_file_table = dispatch_tables.get_file_dispatch_table_all()
507-
print("Configuring Fortran checkers:")
542+
if print_volume >= 3:
543+
print("Configuring Fortran checkers:")
508544
combined_checkers = fortran_diff_table | fortran_file_table | \
509545
generic_file_table
510546
fortran_file_checker = UMDP3_checker.from_full_list(
@@ -513,7 +549,8 @@ def create_style_checkers(
513549
)
514550
checkers.append(fortran_file_checker)
515551
if "Python" in file_types:
516-
print("Setting up Python external checkers.")
552+
if print_volume >= 3:
553+
print("Configuring External Python checkers:")
517554
file_extensions = {".py"}
518555
python_checkers = {
519556
# "flake 8": ["flake8", "-q"],
@@ -522,11 +559,13 @@ def create_style_checkers(
522559
"ruff": ["ruff", "check"],
523560
}
524561
python_file_checker = ExternalChecker(
525-
"Python External Checkers", file_extensions,
562+
"External Python Checkers", file_extensions,
526563
python_checkers, changed_files
527564
)
528565
checkers.append(python_file_checker)
529566
if "Generic" in file_types or file_types == []:
567+
if print_volume >= 3:
568+
print("Configuring Generic File Checkers:")
530569
all_file_dispatch_table = dispatch_tables.get_file_dispatch_table_all()
531570
generic_checker = UMDP3_checker(
532571
"Generic File Checker", set(), all_file_dispatch_table,
@@ -557,11 +596,15 @@ def get_files_to_check(path: str, full_check: bool,
557596
if full_check: # Override to check all files present.
558597
repo_path = Path(path)
559598
all_files = [f for f in repo_path.rglob("*") if f.is_file()]
599+
if print_volume >= 1:
600+
print("Full check override enabled.")
560601
if print_volume >= 3:
561-
print(f"Full check requested. Found {len(all_files)} files to "
602+
print(f" Found {len(all_files)} files to "
562603
f"check in repository at path: {path}")
563604
return all_files
564605
else: # Configure CMS, and check we've been passed a branch
606+
if print_volume >= 1:
607+
print("Using a CMS to determine changed files.")
565608
cms = which_cms_is_it(path, print_volume)
566609
changed_files = cms.get_changed_files()
567610
return changed_files
@@ -572,6 +615,7 @@ def get_files_to_check(path: str, full_check: bool,
572615
args = process_arguments()
573616

574617
log_volume = args.volume
618+
quiet_pass = not args.printpass
575619

576620
file_paths = get_files_to_check(args.path, args.fullcheck, log_volume)
577621
full_file_paths = [Path(args.path) / f for f in file_paths]
@@ -597,7 +641,18 @@ def get_files_to_check(path: str, full_check: bool,
597641

598642
checker.check_files()
599643

600-
all_passed = checker.print_results(print_volume=log_volume)
644+
if log_volume >= 3:
645+
print(line_1(81))
646+
print("## Results :" + " "*67 + "##")
647+
print(line_1(81) + "\n")
648+
else:
649+
print("Results :")
650+
all_passed = checker.print_results(print_volume=log_volume,
651+
quiet_pass=quiet_pass)
652+
if log_volume >= 4:
653+
print("\n" + line_1(81))
654+
print("## Summary :" + " "*67 + "##")
655+
print(line_1(81))
601656
print(f"Total files checked: {len(checker.results)}")
602657
print(f"Total files failed: "
603658
f"{sum(1 for r in checker.results if not r.all_passed)}")

0 commit comments

Comments
 (0)