diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml index 035b82fec..e8ec7aa13 100644 --- a/.github/workflows/linting.yml +++ b/.github/workflows/linting.yml @@ -53,17 +53,17 @@ jobs: - name: Check documentation id: documentation-fabric run: | - python3 tools/check_documentation.py --show-diffs modules fast blueprints + python3 tools/check_documentation.py --show-diffs --no-show-summary modules fast blueprints - name: Check documentation links id: documentation-links-fabric run: | - python3 tools/check_links.py . + python3 tools/check_links.py --no-show-summary . - name: Check name length (fast) id: name-length-fast run: | - python3 tools/check_names.py --prefix-length=10 fast/stages + python3 tools/check_names.py --prefix-length=10 --failed-only fast/stages - name: Check python formatting id: yapf @@ -76,4 +76,4 @@ jobs: - name: Check blueprint metadata id: metadata run: | - python tools/validate_metadata.py -v blueprints + python tools/validate_metadata.py -v --failed-only blueprints diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7396f875b..3d997e948 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -669,6 +669,8 @@ Options: --help Show this message and exit. ``` +As a convenience, we provide a script that runs the same set of checks as our GitHub workflow. Before submitting a PR, run `tools/lint.sh` and fix any errors. You can use the tools described above to find out more about the failures. + The test workflow runs test suites in parallel. Refer to the next section for more details on running and writing tests. ## Using and writing tests diff --git a/tools/check_documentation.py b/tools/check_documentation.py index 438fa1b24..992e44849 100755 --- a/tools/check_documentation.py +++ b/tools/check_documentation.py @@ -72,7 +72,7 @@ def _check_dir(dir_name, exclude_files=None, files=False, show_extra=False): diff = None readme = readme_path.read_text() - mod_name = str(readme_path.relative_to(dir_path).parent) + readme_rel = str(readme_path.relative_to(BASEDIR)) current_doc = tfdoc.get_tfref_parts(readme) current_toc = tfdoc.get_toc_parts(readme) if current_doc or current_toc: @@ -88,14 +88,14 @@ def _check_dir(dir_name, exclude_files=None, files=False, show_extra=False): if current_doc and new_doc.content != current_doc['doc']: state = State.FAIL_STALE_README - header = f'----- {mod_name} diff -----\n' + header = f'----- {readme_rel} diff -----\n' ndiff = difflib.ndiff(current_doc['doc'].splitlines(keepends=True), new_doc.content.splitlines(keepends=True)) diff = ''.join([header] + [x for x in ndiff if x[0] != ' ']) elif current_toc and new_toc != current_toc['toc']: state = State.FAIL_STALE_TOC - header = f'----- {mod_name} diff -----\n' + header = f'----- {readme_rel} diff -----\n' ndiff = difflib.ndiff(current_toc['toc'].splitlines(keepends=True), new_toc.splitlines(keepends=True)) diff = ''.join([header] + [x for x in ndiff if x[0] != ' ']) @@ -103,21 +103,21 @@ def _check_dir(dir_name, exclude_files=None, files=False, show_extra=False): elif empty := [v.name for v in newvars if not v.description]: state = state.FAIL_VARIABLE_DESCRIPTION diff = "\n".join([ - f'----- {mod_name} variables missing description -----', + f'----- {readme_rel} variables missing description -----', ', '.join(empty), ]) elif empty := [o.name for o in newouts if not o.description]: state = state.FAIL_VARIABLE_DESCRIPTION diff = "\n".join([ - f'----- {mod_name} outputs missing description -----', + f'----- {readme_rel} outputs missing description -----', ', '.join(empty), ]) elif variables != sorted(variables): state = state.FAIL_UNSORTED_VARS diff = "\n".join([ - f'----- {mod_name} variables -----', + f'----- {readme_rel} variables -----', f'variables should be in this order: ', ', '.join(sorted(variables)), ]) @@ -125,7 +125,7 @@ def _check_dir(dir_name, exclude_files=None, files=False, show_extra=False): elif outputs != sorted(outputs): state = state.FAIL_UNSORTED_OUTPUTS diff = "\n".join([ - f'----- {mod_name} outputs -----', + f'----- {readme_rel} outputs -----', f'outputs should be in this order: ', ', '.join(sorted(outputs)), ]) @@ -133,18 +133,18 @@ def _check_dir(dir_name, exclude_files=None, files=False, show_extra=False): elif nc := [v.name for v in newvars if not v.description.endswith('.')]: state = state.FAIL_VARIABLE_PERIOD diff = "\n".join([ - f'----- {mod_name} variable descriptions missing ending period -----', + f'----- {readme_rel} variable descriptions missing ending period -----', ', '.join(nc), ]) elif nc := [o.name for o in newouts if not o.description.endswith('.')]: state = state.FAIL_OUTPUT_PERIOD diff = "\n".join([ - f'----- {mod_name} output descriptions missing ending period -----', + f'----- {readme_rel} output descriptions missing ending period -----', ', '.join(nc), ]) - yield mod_name, state, diff + yield readme_rel, state, diff @click.command() @@ -153,18 +153,19 @@ def _check_dir(dir_name, exclude_files=None, files=False, show_extra=False): @click.option('--files/--no-files', default=False) @click.option('--show-diffs/--no-show-diffs', default=False) @click.option('--show-extra/--no-show-extra', default=False) +@click.option('--show-summary/--no-show-summary', default=True) def main(dirs, exclude_file=None, files=False, show_diffs=False, - show_extra=False): + show_extra=False, show_summary=True): 'Cycle through modules and ensure READMEs are up-to-date.' - print(f'files: {files}, extra: {show_extra}, diffs: {show_diffs}\n') + # print(f'files: {files}, extra: {show_extra}, diffs: {show_diffs}\n') errors = [] for dir_name in dirs: - print(f'----- {dir_name} -----') result = _check_dir(dir_name, exclude_file, files, show_extra) - for mod_name, state, diff in result: + for readme_path, state, diff in result: if state.failed: - errors.append((mod_name, diff)) - print(f'[{state.label}] {mod_name}') + errors.append((readme_path, diff)) + if show_summary: + print(f'[{state.label}] {readme_path}') if errors: if show_diffs: @@ -173,6 +174,7 @@ def main(dirs, exclude_file=None, files=False, show_diffs=False, else: print('Errored modules:') print('\n'.join([e[0] for e in errors])) + raise SystemExit('Errors found.') diff --git a/tools/check_links.py b/tools/check_links.py index 1e2759dfb..202bc6401 100755 --- a/tools/check_links.py +++ b/tools/check_links.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 -# Copyright 2022 Google LLC +# Copyright 2023 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -77,20 +77,24 @@ def check_docs(dir_name, external=False): @click.argument('dirs', type=str, nargs=-1) @click.option('-e', '--external', is_flag=True, default=False, help='Whether to test external links.') -def main(dirs, external): +@click.option('--show-summary/--no-show-summary', default=True) +def main(dirs, external, show_summary=True): 'Checks links in Markdown files contained in dirs.' errors = [] for dir_name in dirs: - print(f'----- {dir_name} -----') + if show_summary: + print(f'----- {dir_name} -----') for doc in check_docs(dir_name, external): state = '✓' if all(l.valid for l in doc.links) else '✗' - print(f'[{state}] {doc.relpath} ({len(doc.links)})') + if show_summary: + print(f'[{state}] {doc.relpath} ({len(doc.links)})') if state == '✗': error = [f'{dir_name}/{doc.relpath}'] for l in doc.links: if not l.valid: error.append(f' - {l.dest}') - print(f' {l.dest}') + if show_summary: + print(f' {l.dest}') errors.append('\n'.join(error)) if errors: raise SystemExit('Errors found:\n{}'.format('\n'.join(errors))) diff --git a/tools/check_names.py b/tools/check_names.py index e3fcf88f1..eb9b395df 100755 --- a/tools/check_names.py +++ b/tools/check_names.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright 2022 Google LLC +# Copyright 2023 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -72,7 +72,8 @@ def get_names(dir_name): @click.command() @click.argument('dirs', type=str, nargs=-1) @click.option('--prefix-length', default=7, type=int) -def main(dirs, prefix_length=None): +@click.option('--failed-only', is_flag=True, default=False) +def main(dirs, prefix_length=None, failed_only=False): 'Parse names in dirs.' import json logging.basicConfig(level=logging.INFO) @@ -87,18 +88,21 @@ def main(dirs, prefix_length=None): errors = [] for name in names: name_length = name.length + prefix_length + do_print = True if name_length >= MOD_LIMITS[name.source]: flag = "✗" errors += [f"{name.source}:{name.name}:{name_length}"] else: flag = "✓" + do_print = not failed_only - print(f"[{flag}] {name.source.ljust(source_just)} " - f"{name.name.ljust(name_just)} " - f"{name.value.ljust(value_just)} " - f"({name_length})") - if errors: - raise ValueError(errors) + if do_print: + print(f"[{flag}] {name.source.ljust(source_just)} " + f"{name.name.ljust(name_just)} " + f"{name.value.ljust(value_just)} " + f"({name_length})") + + return 0 if not errors else 1 if __name__ == '__main__': diff --git a/tools/validate_metadata.py b/tools/validate_metadata.py index 08e76ea5b..2e5f55cc3 100755 --- a/tools/validate_metadata.py +++ b/tools/validate_metadata.py @@ -59,7 +59,8 @@ def _validate(path: Path, validator) -> ValidationResult: @click.argument('dirs', type=click.Path(exists=True, file_okay=False), nargs=-1) @click.option('-v', '--verbose', is_flag=True, default=False, help='Print additional validation details.') -def main(dirs: list[str], verbose: bool) -> int: +@click.option('--failed-only', is_flag=True, default=False) +def main(dirs: list[str], verbose: bool, failed_only=False) -> int: instances = set() for dir_name in dirs: instances |= set(Path(dir_name).glob("**/metadata.yaml")) @@ -72,7 +73,8 @@ def main(dirs: list[str], verbose: bool) -> int: for instance in instances: result = _validate(instance, validator) if result.state == State.OK: - print(f'[✓] {instance}') + if not failed_only: + print(f'[✓] {instance}') else: print(f'[✗] {instance}') failed_files[instance] = result.errors