From d8d0119d0f5a6faa54fec9ace0f8475a656892c1 Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Thu, 27 Aug 2026 17:45:14 +0200 Subject: [PATCH] Add doctests to first-party Python scripts Substantially increase doctest coverage across tools/*.py and bundle/direct/tools/*.py (the files exercised by the test-doctest task), from ~20 to ~200 doctests. Coverage targets pure, deterministic functions (parsing, formatting, validation, data shaping); I/O-only scripts are left untouched. A few pure helpers were extracted from larger functions to make their logic testable without changing behavior. Co-authored-by: Isaac --- bundle/direct/tools/generate_apitypes.py | 18 ++- bundle/direct/tools/generate_resources.py | 72 +++++++++++- tools/bench_parse.py | 39 ++++++- tools/bump_mlops_stacks.py | 29 ++++- tools/check_deadcode.py | 42 ++++++- tools/gh_parse.py | 134 +++++++++++++++++++++- tools/gh_report.py | 46 ++++++++ tools/lintdiff.py | 36 +++++- tools/summarize_failed_tests.py | 120 ++++++++++++++++++- tools/update_github_links.py | 19 +++ tools/validate_nextchanges.py | 34 +++++- tools/validate_whitespace.py | 69 ++++++++++- 12 files changed, 637 insertions(+), 21 deletions(-) diff --git a/bundle/direct/tools/generate_apitypes.py b/bundle/direct/tools/generate_apitypes.py index 6d2dd202c2a..ce8ce8fe9fc 100644 --- a/bundle/direct/tools/generate_apitypes.py +++ b/bundle/direct/tools/generate_apitypes.py @@ -37,7 +37,23 @@ def parse_out_fields(path): def get_schema_fields(schemas): - """Get top-level field names for each schema type.""" + """Get top-level field names for each schema type. + + >>> get_schema_fields({}) + {} + >>> get_schema_fields({"TypeA": {}}) + {} + >>> get_schema_fields({"TypeA": {"fields": {"x": {}}}}) + {'TypeA': {'x'}} + >>> result = get_schema_fields({"A": {"fields": {"x": {}}}, "B": {"fields": {"y": {}, "z": {}}}}) + >>> result["A"] == {"x"} + True + >>> result["B"] == {"y", "z"} + True + >>> result = get_schema_fields({"TypeA": {"fields": {}}, "TypeB": {"fields": {"x": {}}}}) + >>> "TypeA" not in result and "TypeB" in result + True + """ schema_fields = {} for name, schema in schemas.items(): props = schema.get("fields", {}) diff --git a/bundle/direct/tools/generate_resources.py b/bundle/direct/tools/generate_resources.py index 45996b5fa27..4374ccdeaa0 100644 --- a/bundle/direct/tools/generate_resources.py +++ b/bundle/direct/tools/generate_resources.py @@ -111,7 +111,21 @@ def extract(schema, prefix, visited, depth, inherited): def find_inherited_behaviors(schemas, type_name): - """Find INPUT_ONLY/OUTPUT_ONLY behaviors from containers that reference type_name.""" + """Find INPUT_ONLY/OUTPUT_ONLY behaviors from containers that reference type_name. + + >>> find_inherited_behaviors({"A": {"fields": {"f": {"ref": "B", "behaviors": ["INPUT_ONLY"]}}}}, "B") + ['INPUT_ONLY'] + >>> find_inherited_behaviors({"A": {"fields": {"f": {"ref": "B", "behaviors": ["OUTPUT_ONLY"]}}}}, "B") + ['OUTPUT_ONLY'] + >>> find_inherited_behaviors({"A": {"fields": {"f": {"ref": "B", "behaviors": ["INPUT_ONLY", "OUTPUT_ONLY"]}}}}, "B") + ['INPUT_ONLY', 'OUTPUT_ONLY'] + >>> find_inherited_behaviors({"A": {"fields": {"f": {"ref": "B"}}}}, "B") + [] + >>> find_inherited_behaviors({}, "B") + [] + >>> find_inherited_behaviors({"A": {"fields": {}}}, "B") + [] + """ inherited = [] for container_schema in schemas.values(): for field_prop in container_schema.get("fields", {}).values(): @@ -126,7 +140,21 @@ def find_inherited_behaviors(schemas, type_name): def filter_prefixes(fields): - """Remove fields that are children of other fields in the list.""" + """Remove fields that are children of other fields in the list. + + >>> filter_prefixes([("a.b.c", "X"), ("a", "Y"), ("a.b", "Z")]) + [('a', 'Y')] + >>> filter_prefixes([("x", "A"), ("y", "B")]) + [('x', 'A'), ('y', 'B')] + >>> filter_prefixes([]) + [] + >>> filter_prefixes([("field", "IMMUTABLE")]) + [('field', 'IMMUTABLE')] + >>> filter_prefixes([("config", "A"), ("config.nested", "B"), ("config.nested.deep", "C")]) + [('config', 'A')] + >>> filter_prefixes([("a", "X"), ("b.c", "Y")]) + [('a', 'X'), ('b.c', 'Y')] + """ result = [] for field, behavior in sorted(fields): if not any(field.startswith(f + ".") for f, _ in result): @@ -135,7 +163,23 @@ def filter_prefixes(fields): def write_field_group(lines, header, fields): - """Write a group of fields with field and reason, grouped by behavior.""" + """Write a group of fields with field and reason, grouped by behavior. + + >>> lines = [] + >>> write_field_group(lines, "test_group", [("f1", "IMMUTABLE")]) + >>> len(lines) + 3 + >>> "test_group" in lines[0] + True + >>> lines = [] + >>> write_field_group(lines, "multi", [("a", "OUTPUT_ONLY"), ("b", "INPUT_ONLY")]) + >>> len(lines) + 6 + >>> any("spec:input_only" in line for line in lines) + True + >>> any("spec:output_only" in line for line in lines) + True + """ lines.append(f"\n {header}:") # Group by behavior by_behavior = {} @@ -153,7 +197,27 @@ def write_field_group(lines, header, fields): def generate(resource_behaviors): - """Generate resources.yml.""" + """Generate resources.yml. + + >>> result = generate({"res1": {"f1": ["OUTPUT_ONLY"]}}) + >>> "res1:" in result + True + >>> "ignore_remote_changes:" in result + True + >>> "spec:output_only" in result + True + >>> result = generate({}) + >>> "resources:" in result + True + >>> result = generate({"res": {}}) + >>> "no api field behaviors" in result + True + >>> result = generate({"res": {"field": ["IMMUTABLE", "OUTPUT_ONLY"]}}) + >>> "recreate_on_changes:" in result + True + >>> "ignore_remote_changes:" in result + True + """ lines = [ """# Generated, do not edit. API field behaviors from OpenAPI schema. # diff --git a/tools/bench_parse.py b/tools/bench_parse.py index cfaf6feaff2..467e4c237fe 100755 --- a/tools/bench_parse.py +++ b/tools/bench_parse.py @@ -14,6 +14,26 @@ def parse_key_values(text): >>> parse_key_values("wall=10.316 ru_utime=19.207 ru_stime=0.505 ru_maxrss=573079552") {'wall': 10.316, 'ru_utime': 19.207, 'ru_stime': 0.505, 'ru_maxrss': 573079552.0} + + Empty string returns empty dict: + + >>> parse_key_values("") + {} + + Single pair: + + >>> parse_key_values("key=42") + {'key': 42.0} + + Non-numeric values are kept as strings: + + >>> parse_key_values("name=test count=5") + {'name': 'test', 'count': 5.0} + + Pairs without equals sign are skipped: + + >>> parse_key_values("valid=1 invalid also_valid=2") + {'valid': 1.0, 'also_valid': 2.0} """ result = {} for kv_pair in text.split(): @@ -53,7 +73,24 @@ def parse_bench_output(file_path): def calculate_means(results): - """Calculate mean values for each metric.""" + """Calculate mean values for each metric. + + >>> calculate_means({"test1": {"wall": [10, 20]}}) + {'test1': {'wall': 15}} + + >>> calculate_means({"test1": {"wall": [1.5, 2.5, 3.0]}}) # doctest: +ELLIPSIS + {'test1': {'wall': 2.33...}} + + Multiple tests and metrics: + + >>> sorted(calculate_means({"t1": {"m1": [2, 4], "m2": [100]}, "t2": {"m1": [3]}}).items()) + [('t1', {'m1': 3, 'm2': 100}), ('t2', {'m1': 3})] + + Empty metrics list returns zero: + + >>> calculate_means({"test": {"metric": []}}) + {'test': {'metric': 0}} + """ means = {} for test_name, metrics in results.items(): means[test_name] = {metric: statistics.mean(values) if values else 0 for metric, values in metrics.items()} diff --git a/tools/bump_mlops_stacks.py b/tools/bump_mlops_stacks.py index 1f5a42c8464..2ffe327c5d7 100755 --- a/tools/bump_mlops_stacks.py +++ b/tools/bump_mlops_stacks.py @@ -38,7 +38,23 @@ def render_path(rel_path, config): - """Map a path under the upstream template/ dir to its path in the rendered project.""" + """Map a path under the upstream template/ dir to its path in the rendered project. + + >>> render_path(Path("template/{{.input_root_dir}}/job.yml"), {"input_root_dir": "src", "input_project_name": "myproj"}) + 'template/src/job.yml' + + >>> render_path(Path("template/{{.input_project_name}}/config.json.tmpl"), {"input_root_dir": "src", "input_project_name": "myproj"}) + 'template/myproj/config.json' + + Handles both vendor and upstream project directory patterns: + >>> cfg = {"input_root_dir": ".", "input_project_name": "proj"} + >>> render_path(Path("template/{{.input_root_dir}}/{{template `project_name_alphanumeric_underscore` .}}/file"), cfg) + 'template/./proj/file' + + Removes .tmpl suffix: + >>> render_path(Path("template/README.md.tmpl"), {"input_root_dir": ".", "input_project_name": "p"}) + 'template/README.md' + """ subs = { PROJECT_ROOT_SEG: config["input_root_dir"], VENDORED_PROJECT_DIR: config["input_project_name"], @@ -85,6 +101,17 @@ def keep_set(clone_dir, out_dir, config): def vendored_path(rel_path): + """Replace upstream project directory pattern with vendored pattern. + + >>> vendored_path(Path("template/{{template `project_name_alphanumeric_underscore` .}}/config")) + PosixPath('template/{{.input_project_name}}/config') + + >>> vendored_path(Path("library/helpers.py")) + PosixPath('library/helpers.py') + + >>> vendored_path(Path("a/{{template `project_name_alphanumeric_underscore` .}}/b/{{template `project_name_alphanumeric_underscore` .}}/c")) + PosixPath('a/{{.input_project_name}}/b/{{.input_project_name}}/c') + """ return Path(*[VENDORED_PROJECT_DIR if p == UPSTREAM_PROJECT_DIR else p for p in rel_path.parts]) diff --git a/tools/check_deadcode.py b/tools/check_deadcode.py index 5c37f1f4750..8d1bfbe6f39 100755 --- a/tools/check_deadcode.py +++ b/tools/check_deadcode.py @@ -47,6 +47,39 @@ ALLOW_COMMENT = "//deadcode:allow" +def should_exclude_line(line, excluded_dirs): + """Check if a deadcode output line refers to an excluded directory. + + >>> should_exclude_line("libs/gorules/myrule.go:10:5: func", ["libs/gorules/"]) + True + >>> should_exclude_line("bundle/internal/tf/schema/gen.go:10:5: func", ["bundle/internal/tf/schema/"]) + True + >>> should_exclude_line("cmd/bundle/deploy.go:10:5: func", ["libs/gorules/"]) + False + >>> should_exclude_line("bundle/internal/tf/schema/gen.go:10:5: func", ["libs/gorules/"]) + False + """ + return any(line.startswith(d) or ("/" + d) in line for d in excluded_dirs) + + +def parse_deadcode_line(line): + """Parse a deadcode output line into (filepath, lineno) or return None if unparseable. + + Typical deadcode format: path/to/file.go:123:45: message + >>> parse_deadcode_line("cmd/main.go:42:3: func Foo") + ('cmd/main.go', 42) + >>> parse_deadcode_line("libs/util/helper.go:1:0: func Helper") + ('libs/util/helper.go', 1) + >>> parse_deadcode_line("invalid line format") + + >>> parse_deadcode_line("") + """ + match = re.match(r"(.+?):(\d+):\d+:", line) + if not match: + return None + return (match.group(1), int(match.group(2))) + + def main(): result = subprocess.run( ["go", "tool", "-modfile=tools/go.mod", "deadcode", "-test", "./..."], @@ -67,16 +100,15 @@ def main(): violations = [] for line in lines: - if any(line.startswith(d) or ("/" + d) in line for d in EXCLUDED_DIRS): + if should_exclude_line(line, EXCLUDED_DIRS): continue - match = re.match(r"(.+?):(\d+):\d+:", line) - if not match: + parsed = parse_deadcode_line(line) + if not parsed: violations.append(line) continue - filepath = match.group(1) - lineno = int(match.group(2)) + filepath, lineno = parsed try: with open(filepath) as f: diff --git a/tools/gh_parse.py b/tools/gh_parse.py index b64f4dcddc0..13fde15a634 100755 --- a/tools/gh_parse.py +++ b/tools/gh_parse.py @@ -91,6 +91,28 @@ def matches(self, package_name, test_name): return test_name == self.test_pattern or self._matches_path_prefix(self.test_pattern, test_name) def _matches_path_prefix(self, s, pattern): + """ + Check if string s matches pattern as a path prefix. + + Matches if pattern is empty (wildcard), s equals pattern exactly, + or s starts with pattern followed by a "/". + + >>> rule = KnownFailuresRule("", "", False, False, "") + >>> rule._matches_path_prefix("bundle", "") + True + + >>> rule._matches_path_prefix("bundle", "bundle") + True + + >>> rule._matches_path_prefix("libs/auth", "libs") + True + + >>> rule._matches_path_prefix("libsother", "libs") + False + + >>> rule._matches_path_prefix("bundle", "bundle/subtest") + False + """ if pattern == "": return True if s == pattern: @@ -131,6 +153,24 @@ def parse_known_failures(content): def _parse_pattern(pattern): + """ + Parse a pattern string, extracting the base and a prefix flag. + + Returns (base, is_prefix) where is_prefix indicates if the pattern + ended with "/" (directory prefix match) or was "*" (wildcard). + + >>> _parse_pattern("bundle") + ('bundle', False) + + >>> _parse_pattern("*") + ('', True) + + >>> _parse_pattern("libs/") + ('libs', True) + + >>> _parse_pattern("foo/bar/") + ('foo/bar', True) + """ if pattern == "*": return "", True if pattern.endswith("/"): @@ -592,6 +632,21 @@ def key(column): def make_summary_message(table, summary): + """ + Create a summary message from test results. + + Formats as "N interesting tests: count1 info1, count2 info2, ..." + with results sorted by count (highest first). + + >>> make_summary_message([{}, {}, {}], {"failed": 2, "flaky": 1}) + '3 interesting tests: 2 failed, 1 flaky' + + >>> make_summary_message([{}], {"panic": 5}) + '1 interesting tests: 5 panic' + + >>> make_summary_message([], {}) + '0 interesting tests: ' + """ items = list(summary.items()) items.sort(key=lambda x: x[1], reverse=True) items = ", ".join(f"{count} {info}" for (info, count) in items) @@ -601,6 +656,25 @@ def make_summary_message(table, summary): # For test table, use shorter version of action. # We have full action name in env table, so that is used as agenda. def short_action(action): + """ + Return short version of action for table display. + + If the action has a zero-width space at position 1 (emoji format), + return first 3 characters (emoji + zero-width space + first letter). + Otherwise return the action unchanged. + + >>> short_action("\u274c\\u200bFAIL") + '\u274c\\u200bF' + + >>> short_action("\u2705\\u200bpass") + '\u2705\\u200bp' + + >>> short_action("regular_text") + 'regular_text' + + >>> short_action("AB") + 'AB' + """ if len(action) >= 4 and action[1] == "\u200b": # include first non-emoji letter in case emoji rendering is broken return action[:3] @@ -659,10 +733,45 @@ def format_table(table, columns=None, markdown=False): def fmt(cells, widths): + """ + Format cells as a single table row with specified column widths. + + Joins cells with two spaces, applying autojust to each cell. + + >>> fmt(["Name", "123", "Status"], [8, 5, 6]) + 'Name 123 Status' + + >>> fmt(["A", "B"], [3, 3]) + ' A B ' + + >>> fmt([], []) + '' + """ return " ".join(autojust(cell, w) for cell, w in zip(cells, widths, strict=False)) def autojust(value, width): + """ + Right-align numeric and short values, left-align longer text. + + For terminal display: numeric strings and values with 3 or fewer + characters are centered. Longer strings are left-justified. + + >>> autojust("123", 5) + ' 123 ' + + >>> autojust("test", 6) + 'test ' + + >>> autojust("AB", 4) + ' AB ' + + >>> autojust("", 3) + ' ' + + >>> autojust(0, 3) + ' 0 ' + """ # Note, this has no effect on how markdown is rendered, only relevant for terminal output value = str(value) if value.isdigit(): @@ -677,7 +786,30 @@ def wrap_in_details(txt, summary): def format_duration(seconds): - """Format duration from seconds to MM:SS format.""" + """ + Format duration from seconds to MM:SS format. + + Returns empty string for None input. Handles fractional seconds + by truncating to integers. + + >>> format_duration(65) + '1:05' + + >>> format_duration(3661) + '61:01' + + >>> format_duration(0) + '0:00' + + >>> format_duration(59) + '0:59' + + >>> format_duration(None) + '' + + >>> format_duration(3.7) + '0:03' + """ if seconds is None: return "" minutes = int(seconds // 60) diff --git a/tools/gh_report.py b/tools/gh_report.py index 455899529d4..7c08295d94e 100755 --- a/tools/gh_report.py +++ b/tools/gh_report.py @@ -44,6 +44,26 @@ def find_tables(lines): >>> find_tables(["intro", "| a |", "| - |", "| 1 |", "", "| b |"]) [(1, 4), (5, 6)] + + Empty input returns no tables: + >>> find_tables([]) + [] + + Single table spans entire input: + >>> find_tables(["| a |", "| - |", "| 1 |"]) + [(0, 3)] + + Table at start: + >>> find_tables(["| x |", "| - |", "text"]) + [(0, 2)] + + Table at end: + >>> find_tables(["text", "| y |", "| - |"]) + [(1, 3)] + + No tables returns empty: + >>> find_tables(["line1", "line2", "line3"]) + [] """ tables = [] start = None @@ -81,6 +101,32 @@ def trim_tables(text, limit=MAX_MARKDOWN_SIZE): | row 06 | outro (14 table rows omitted to keep the report under 180 bytes) + + Empty text fits any limit: + >>> trim_tables("", limit=10) + '' + + Text with no tables (below limit) is unchanged: + >>> trim_tables("just text", limit=100) + 'just text' + + Trimming prioritizes the largest table: + >>> big_table = "\n".join(["| row |"] * 15) + >>> small_table = "\n".join(["| x |"] * 2) + >>> report = big_table + "\n" + small_table + >>> result = trim_tables(report, limit=50) + >>> "table rows omitted" in result + True + >>> "| row |" in result # big table partially preserved + True + + Header and separator rows are always kept: + >>> table = "| Header |\n| --- |\n| row 1 |\n| row 2 |\n| row 3 |" + >>> result = trim_tables(table, limit=40) + >>> "| --- |" in result # separator always present + True + >>> "| Header |" in result # header always present + True """ size = len(text.encode()) if size <= limit: diff --git a/tools/lintdiff.py b/tools/lintdiff.py index 1225c36c20c..387821ed865 100755 --- a/tools/lintdiff.py +++ b/tools/lintdiff.py @@ -17,6 +17,36 @@ NESTED_MODULES = ("bundle/internal/tf/codegen", "tools") +def in_nested_module(path): + """Check if a path is under a nested module. + + >>> in_nested_module("tools") + True + + >>> in_nested_module("tools/task") + True + + >>> in_nested_module("tools/task/subtask.go") + True + + >>> in_nested_module("bundle/internal/tf/codegen") + True + + >>> in_nested_module("bundle/internal/tf/codegen/gen.go") + True + + >>> in_nested_module("cmd/bundle") + False + + >>> in_nested_module("cmd") + False + + >>> in_nested_module("toolz") + False + """ + return any(path == m or path.startswith(m + "/") for m in NESTED_MODULES) + + def parse_lines(cmd): # print("+ " + " ".join(cmd), file=sys.stderr, flush=True) result = subprocess.run(cmd, stdout=subprocess.PIPE, encoding="utf-8") @@ -59,10 +89,6 @@ def main(): filter_nested = "run" in cmd if changed is not None: - - def in_nested_module(path): - return filter_nested and any(path == m or path.startswith(m + "/") for m in NESTED_MODULES) - # We need to pass packages to golangci-lint, not individual files. # QQQ for lint we should also pass all dependent packages dirs = set() @@ -71,7 +97,7 @@ def in_nested_module(path): continue if filename.endswith(".go"): d = os.path.dirname(filename) - if in_nested_module(d): + if filter_nested and in_nested_module(d): continue dirs.add(d) diff --git a/tools/summarize_failed_tests.py b/tools/summarize_failed_tests.py index dfcbb564acf..940e9af4b15 100755 --- a/tools/summarize_failed_tests.py +++ b/tools/summarize_failed_tests.py @@ -32,7 +32,28 @@ def load_events(path): def last_action_by_key(events, key): - """Map each key to the Action of its last event.""" + """Map each key to the Action of its last event. + + >>> events = [ + ... {"Action": "run", "id": 1}, + ... {"Action": "pass", "id": 1}, + ... {"Action": "fail", "id": 2}, + ... ] + >>> last_action_by_key(events, lambda e: e["id"]) + {1: 'pass', 2: 'fail'} + + Empty events returns empty dict: + >>> last_action_by_key([], lambda e: e["id"]) + {} + + Later occurrences override earlier ones: + >>> events = [ + ... {"Action": "pass", "test": "A"}, + ... {"Action": "fail", "test": "A"}, + ... ] + >>> last_action_by_key(events, lambda e: e["test"]) + {'A': 'fail'} + """ last = {} for event in events: last[key(event)] = event["Action"] @@ -45,6 +66,37 @@ def failed_tests(events): A test can appear more than once because of --rerun-fails, so we group by package+test and keep only those whose last result was a failure (recovered flakes end on "pass"). + + >>> events = [ + ... {"Package": "pkg1", "Test": "TestA", "Action": "fail"}, + ... {"Package": "pkg1", "Test": "TestB", "Action": "pass"}, + ... ] + >>> failed_tests(events) + [('pkg1', 'TestA')] + + Flakes that recover are not reported: + >>> events = [ + ... {"Package": "pkg1", "Test": "TestFlake", "Action": "fail"}, + ... {"Package": "pkg1", "Test": "TestFlake", "Action": "pass"}, + ... ] + >>> failed_tests(events) + [] + + Multiple failures sorted by package, then test: + >>> events = [ + ... {"Package": "pkg2", "Test": "B", "Action": "fail"}, + ... {"Package": "pkg1", "Test": "A", "Action": "fail"}, + ... ] + >>> failed_tests(events) + [('pkg1', 'A'), ('pkg2', 'B')] + + Package-level events (Test=None) are ignored: + >>> events = [ + ... {"Package": "pkg1", "Test": None, "Action": "fail"}, + ... {"Package": "pkg1", "Test": "TestX", "Action": "fail"}, + ... ] + >>> failed_tests(events) + [('pkg1', 'TestX')] """ test_events = [e for e in events if e.get("Test") is not None] last = last_action_by_key(test_events, lambda e: (e["Package"], e["Test"])) @@ -59,6 +111,32 @@ def failed_packages_without_test(events, failed_test_packages): "fail" for every package that merely has a failing test, so exclude packages already reported via failed_tests; otherwise those get double-reported here as spurious build errors. + + >>> events = [ + ... {"Package": "pkg1", "Test": None, "Action": "fail"}, + ... {"Package": "pkg2", "Test": None, "Action": "fail"}, + ... ] + >>> failed_packages_without_test(events, set()) + ['pkg1', 'pkg2'] + + Packages with failing tests are excluded: + >>> events = [ + ... {"Package": "pkg1", "Test": None, "Action": "fail"}, + ... ] + >>> failed_packages_without_test(events, {"pkg1"}) + [] + + Passed packages are ignored: + >>> events = [ + ... {"Package": "pkg1", "Test": None, "Action": "pass"}, + ... {"Package": "pkg2", "Test": None, "Action": "fail"}, + ... ] + >>> failed_packages_without_test(events, set()) + ['pkg2'] + + Empty events returns empty: + >>> failed_packages_without_test([], set()) + [] """ package_events = [e for e in events if e.get("Test") is None] last = last_action_by_key(package_events, lambda e: e["Package"]) @@ -68,6 +146,46 @@ def failed_packages_without_test(events, failed_test_packages): def render(tests, packages): + """Render failed tests and build-error packages as markdown. + + >>> print(render([("pkg1", "TestA")], [])) + ## Failed tests + + | Package | Test | + | --- | --- | + | pkg1 | `TestA` | + + Build errors without tests: + >>> print(render([], ["pkg1", "pkg2"])) + ## Failed tests + + ### Packages that failed without a test (build error / panic) + + ``` + pkg1 + pkg2 + ``` + + Both tests and build errors: + >>> print(render([("pkg1", "T1")], ["pkg2"])) + ## Failed tests + + | Package | Test | + | --- | --- | + | pkg1 | `T1` | + + ### Packages that failed without a test (build error / panic) + + ``` + pkg2 + ``` + + No failures: + >>> print(render([], [])) + ## Failed tests + + No failed tests found in test-output.json (the failure may be outside the test run). + """ lines = ["## Failed tests"] if tests: lines += ["", "| Package | Test |", "| --- | --- |"] diff --git a/tools/update_github_links.py b/tools/update_github_links.py index 01f940d47d4..d030be44c75 100755 --- a/tools/update_github_links.py +++ b/tools/update_github_links.py @@ -58,6 +58,10 @@ def find_mismatched_links(text): [] >>> find_mismatched_links("([#1234](https://github.com/databricks/cli/pull/9999))") ['Converted link numbers differ: text #1234 vs URL #9999 — …([#1234](https://github.com/databricks/cli/pull/9999))…'] + >>> len(find_mismatched_links("([#1](https://github.com/databricks/cli/pull/2)) and ([#3](https://github.com/databricks/cli/pull/4))")) == 2 + True + >>> find_mismatched_links("") + [] """ mismatches = [] for m in CONVERTED_LINK_RE.finditer(text): @@ -96,6 +100,21 @@ def convert_raw_references(text): >>> t = "(#3456) and #7890" >>> convert_raw_references(convert_raw_references(t)) == convert_raw_references(t) True + + Multiple references in one string are all converted: + + >>> convert_raw_references("See #100, #200, and #300") + 'See ([#100](https://github.com/databricks/cli/pull/100)), ([#200](https://github.com/databricks/cli/pull/200)), and ([#300](https://github.com/databricks/cli/pull/300))' + + Empty string remains empty: + + >>> convert_raw_references("") + '' + + References with word boundaries are converted: + + >>> convert_raw_references("Issue #999 is fixed") + 'Issue ([#999](https://github.com/databricks/cli/pull/999)) is fixed' """ def _make_link(num): diff --git a/tools/validate_nextchanges.py b/tools/validate_nextchanges.py index a6755ea5d25..18301a705a5 100755 --- a/tools/validate_nextchanges.py +++ b/tools/validate_nextchanges.py @@ -39,6 +39,38 @@ NEXTVERSION_GO = "nextversion.go" +def is_valid_semver(version_str): + """Check if a string is a valid semantic version. + + Valid formats (bare or v-prefixed, with optional pre-release/metadata): + >>> is_valid_semver("1.0.0") + True + >>> is_valid_semver("v1.0.0") + True + >>> is_valid_semver("1.2.3-alpha") + True + >>> is_valid_semver("1.2.3+build") + True + >>> is_valid_semver("1.2.3-rc.1+build.123") + True + + Invalid formats: + >>> is_valid_semver("1.0") + False + >>> is_valid_semver("v1.0") + False + >>> is_valid_semver("not-a-version") + False + >>> is_valid_semver("") + False + + Whitespace is stripped, so "1.0.0" with leading/trailing whitespace is valid: + >>> is_valid_semver(" 1.0.0 ") + True + """ + return bool(SEMVER_RE.match(version_str.strip()) if version_str else False) + + def load_sections(root): """Return the section slugs from .codegen.json, in changelog order. @@ -91,7 +123,7 @@ def find_problems(changelog_dir, sections): version_path = changelog_dir / VERSION_FILE if not version_path.is_file(): problems.append((version_path, "missing; expected the next release version (e.g. 1.4.0)")) - elif not SEMVER_RE.match(version_path.read_text(encoding="utf-8").strip()): + elif not is_valid_semver(version_path.read_text(encoding="utf-8")): problems.append((version_path, "not a valid semver version (e.g. 1.4.0)")) return problems diff --git a/tools/validate_whitespace.py b/tools/validate_whitespace.py index 152e52c5e6c..5a566338104 100755 --- a/tools/validate_whitespace.py +++ b/tools/validate_whitespace.py @@ -31,11 +31,55 @@ def load_ignores(): def count_trailing_newlines(s): + """Count consecutive newlines at the end of a string. + + >>> count_trailing_newlines("hello") + 0 + >>> count_trailing_newlines("hello\\n") + 1 + >>> count_trailing_newlines("hello\\n\\n") + 2 + >>> count_trailing_newlines("\\n\\n\\n") + 3 + >>> count_trailing_newlines("") + 0 + """ match = re.search(r"(\n+)$", s) return len(match.group(1)) if match else 0 def validate_contents(data): + """Validate file contents and yield error messages for issues found. + + Returns empty for valid content (ends with single newline, no trailing spaces): + >>> msgs = list(validate_contents(b'hello\\nworld\\n')) + >>> len(msgs) + 0 + + Detects missing final newline: + >>> msgs = list(validate_contents(b'hello')) + >>> ' File does not end with a newline' in msgs + True + + Detects trailing whitespace: + >>> msgs = list(validate_contents(b'hello \\n')) + >>> any('Trailing whitespace' in m for m in msgs) + True + + Detects whitespace-only lines: + >>> msgs = list(validate_contents(b'hello\\n \\nworld\\n')) + >>> any('Whitespace-only line' in m for m in msgs) + True + + Detects multiple trailing newlines: + >>> msgs = list(validate_contents(b'hello\\n\\n\\n')) + >>> any('3 newlines at the end' in m for m in msgs) + True + + Empty data yields nothing: + >>> list(validate_contents(b'')) + [] + """ if not data: return try: @@ -63,7 +107,30 @@ def validate_contents(data): def fix_contents(data): - """Fix whitespace issues in file contents.""" + """Fix whitespace issues in file contents. + + Removes trailing whitespace and ensures exactly one final newline: + >>> result = fix_contents(b'hello \\nworld \\n\\n\\n') + >>> result == b'hello\\nworld\\n' + True + + Adds missing final newline: + >>> fix_contents(b'hello') == b'hello\\n' + True + + Handles whitespace-only lines by removing trailing spaces: + >>> result = fix_contents(b'hello\\n \\nworld\\n') + >>> result == b'hello\\n\\nworld\\n' + True + + Returns empty input as-is: + >>> fix_contents(b'') == b'' + True + + Preserves valid content unchanged: + >>> fix_contents(b'hello\\nworld\\n') == b'hello\\nworld\\n' + True + """ if not data: return data try: