From 9833c2f3738a0ba54d7613eab41001776aeeb4bc Mon Sep 17 00:00:00 2001 From: Miguel Dalberto Pedro Date: Mon, 5 Oct 2026 16:04:42 -0300 Subject: [PATCH 1/4] Publish a metrics payload even when no design produced metrics uploadMetadata.py used to skip the publish when no design had a metadata.json, so builds that generated no metrics never reached the QoR dashboard. It now publishes the pipeline payload with an empty designs list through the normal path, and a failed publish still exits 1. Three optional arguments describe the build on the pipeline payload: --buildTime (build_time, defaulting to now in genMetrics.py's format and emitted on every pipeline payload), --expectedDesigns (design_count) and --noMetricsReason (no_metrics_reason, only on an empty payload). v1 fallback messages are unchanged, since the backend reads their unknown root keys as metrics. Part of The-OpenROAD-Project-private/lembas#209. Signed-off-by: Miguel Dalberto Pedro --- flow/test/test_uploadMetadata.py | 261 +++++++++++++++++++++++++++++++ flow/util/uploadMetadata.py | 59 ++++++- 2 files changed, 317 insertions(+), 3 deletions(-) create mode 100755 flow/test/test_uploadMetadata.py diff --git a/flow/test/test_uploadMetadata.py b/flow/test/test_uploadMetadata.py new file mode 100755 index 0000000000..329e9adbbd --- /dev/null +++ b/flow/test/test_uploadMetadata.py @@ -0,0 +1,261 @@ +#!/usr/bin/env python3 + +"""Tests for the Pub/Sub payload that uploadMetadata.py publishes. + +uploadMetadata.py does its work at import time (it parses arguments, changes +to flow/ and walks reports/), so it runs here as a subprocess. The script is +copied into a temporary util/ directory next to a temporary reports/ tree, +which makes that tree the one it walks, and stub google.cloud modules on +PYTHONPATH record every published message instead of sending it. +""" + +import json +import os +import re +import shutil +import subprocess +import sys +import tempfile +import unittest + +SCRIPT = os.path.join( + os.path.dirname(os.path.abspath(__file__)), "..", "util", "uploadMetadata.py" +) + +STUB_PUBSUB = """ +import json +import os + + +class _Future: + def __init__(self, message_id): + self._message_id = message_id + + def result(self): + if os.environ.get("STUB_PUBSUB_FAIL"): + raise RuntimeError("stub publish failure") + return self._message_id + + +class PublisherClient: + def __init__(self, credentials=None): + pass + + def topic_path(self, project, topic): + return f"projects/{project}/topics/{topic}" + + def publish(self, topic_path, data, **attributes): + with open(os.environ["STUB_PUBSUB_OUT"], "a") as f: + record = {"data": json.loads(data), "attributes": attributes} + f.write(json.dumps(record) + "\\n") + return _Future("stub-id") +""" + +STUB_SERVICE_ACCOUNT = """ +class Credentials: + @staticmethod + def from_service_account_file(path): + return None +""" + +BUILD_TIME_FORMAT = re.compile(r"^\d{4}-\d{2}-\d{2} \d{2}:\d{2}$") + + +class TestUploadMetadata(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + root = self.tmp.name + self.flow = os.path.join(root, "flow") + os.makedirs(os.path.join(self.flow, "util")) + os.makedirs(os.path.join(self.flow, "reports")) + shutil.copy(SCRIPT, os.path.join(self.flow, "util")) + + stubs = os.path.join(root, "stubs") + files = { + "google/__init__.py": "", + "google/cloud/__init__.py": "", + "google/cloud/pubsub_v1.py": STUB_PUBSUB, + "google/oauth2/__init__.py": "", + "google/oauth2/service_account.py": STUB_SERVICE_ACCOUNT, + } + for rel, content in files.items(): + path = os.path.join(stubs, rel) + os.makedirs(os.path.dirname(path), exist_ok=True) + with open(path, "w") as f: + f.write(content) + self.stubs = stubs + self.out = os.path.join(root, "published.jsonl") + + def tearDown(self): + self.tmp.cleanup() + + def add_design(self, platform, design, variant="base", metrics=None): + report_dir = os.path.join(self.flow, "reports", platform, design, variant) + os.makedirs(report_dir) + with open(os.path.join(report_dir, "metadata.json"), "w") as f: + json.dump(metrics or {"finish:area": 1.0}, f) + + def run_script(self, *extra, fail=False): + env = dict(os.environ) + env["PYTHONPATH"] = self.stubs + env["STUB_PUBSUB_OUT"] = self.out + if fail: + env["STUB_PUBSUB_FAIL"] = "1" + cmd = [ + sys.executable, + os.path.join(self.flow, "util", "uploadMetadata.py"), + "--buildID", + "42", + "--branchName", + "master", + "--pipelineID", + "jenkins-ORFS-master-42", + "--commitSHA", + "abc123", + "--jenkinsURL", + "http://jenkins/job/ORFS/42/", + "--jenkinsEnv", + "public", + "--pubsubProjectID", + "test-project", + *extra, + ] + result = subprocess.run(cmd, env=env, capture_output=True, text=True) + messages = [] + if os.path.exists(self.out): + with open(self.out) as f: + messages = [json.loads(line) for line in f] + return result, messages + + def test_empty_designs_publishes_report(self): + result, messages = self.run_script( + "--jobName", + "ORFS", + "--buildTime", + "2026-10-05 12:34", + "--expectedDesigns", + "12", + "--noMetricsReason", + " flow failed before metrics ", + ) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(len(messages), 1) + payload = messages[0]["data"] + self.assertEqual(payload["designs"], []) + self.assertEqual(payload["payload_schema_version"], 3) + self.assertEqual(payload["build_time"], "2026-10-05 12:34") + self.assertEqual(payload["design_count"], 12) + self.assertEqual(payload["no_metrics_reason"], "flow failed before metrics") + self.assertEqual(messages[0]["attributes"]["payload_schema_version"], "3") + self.assertIn("empty (no-metrics)", result.stdout) + + def test_empty_designs_without_new_flags(self): + result, messages = self.run_script() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(len(messages), 1) + payload = messages[0]["data"] + self.assertEqual(payload["designs"], []) + self.assertEqual(payload["payload_schema_version"], 2) + self.assertRegex(payload["build_time"], BUILD_TIME_FORMAT) + self.assertNotIn("design_count", payload) + self.assertNotIn("no_metrics_reason", payload) + + def test_blank_reason_is_omitted(self): + result, messages = self.run_script("--noMetricsReason", " ") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertNotIn("no_metrics_reason", messages[0]["data"]) + + def test_empty_designs_failed_publish_exits_nonzero(self): + result, _ = self.run_script(fail=True) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("[ERROR] Pub/Sub publish failed", result.stdout) + + def test_designs_payload_gains_build_time_only(self): + self.add_design("nangate45", "gcd", metrics={"finish:area": 5.0}) + result, messages = self.run_script( + "--jobName", "ORFS", "--noMetricsReason", "ignored" + ) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(len(messages), 1) + payload = messages[0]["data"] + self.assertRegex(payload.pop("build_time"), BUILD_TIME_FORMAT) + self.assertEqual( + payload, + { + "payload_schema_version": 3, + "jenkins_env": "public", + "build_id": "42", + "branch_name": "master", + "pipeline_id": "jenkins-ORFS-master-42", + "change_branch": None, + "commit_sha": "abc123", + "jenkins_url": "http://jenkins/job/ORFS/42/", + "job_name": "ORFS", + "designs": [ + { + "platform": "nangate45", + "design": "gcd", + "variant": "base", + "metrics": {"finish__area": 5.0}, + } + ], + }, + ) + self.assertNotIn("empty (no-metrics)", result.stdout) + + def test_design_count_with_designs(self): + self.add_design("nangate45", "gcd") + result, messages = self.run_script("--expectedDesigns", "3") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + payload = messages[0]["data"] + self.assertEqual(payload["design_count"], 3) + self.assertEqual(len(payload["designs"]), 1) + + def test_empty_designs_invalid_provenance_falls_back_to_v3(self): + bad = os.path.join(self.tmp.name, "provenance.json") + with open(bad, "w") as f: + f.write("[]") + result, messages = self.run_script("--jobName", "ORFS", "--provenanceFile", bad) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + payload = messages[0]["data"] + self.assertEqual(payload["payload_schema_version"], 3) + self.assertEqual(payload["designs"], []) + self.assertNotIn("submodules", payload) + + def test_empty_designs_valid_provenance_is_v4(self): + good = os.path.join(self.tmp.name, "provenance.json") + with open(good, "w") as f: + json.dump( + { + "jenkins_library": {"ref": "v1.0.0"}, + "submodules": [ + {"path": "tools/OpenROAD", "sha": "def456", "overridden": False} + ], + }, + f, + ) + result, messages = self.run_script( + "--jobName", "ORFS", "--provenanceFile", good + ) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + payload = messages[0]["data"] + self.assertEqual(payload["payload_schema_version"], 4) + self.assertEqual(payload["designs"], []) + self.assertEqual(payload["jenkins_library"], {"ref": "v1.0.0"}) + + def test_no_publisher_publishes_nothing(self): + env = dict(os.environ) + env["PYTHONPATH"] = self.stubs + env["STUB_PUBSUB_OUT"] = self.out + result = subprocess.run( + [sys.executable, os.path.join(self.flow, "util", "uploadMetadata.py")], + env=env, + capture_output=True, + text=True, + ) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertFalse(os.path.exists(self.out)) + + +if __name__ == "__main__": + unittest.main() diff --git a/flow/util/uploadMetadata.py b/flow/util/uploadMetadata.py index b7a49c27f3..4aa647bd4c 100755 --- a/flow/util/uploadMetadata.py +++ b/flow/util/uploadMetadata.py @@ -4,6 +4,7 @@ import argparse import os import sys +from datetime import datetime # --- PUBSUB --- from google.cloud import pubsub_v1 @@ -52,6 +53,29 @@ "pinned commit rather than the built one. When supplied alongside " "--jobName the payload is emitted as schema v4.", ) +parser.add_argument( + "--buildTime", + type=str, + default=None, + help="When the build ran, emitted as build_time on the pipeline payload. " + "Defaults to now, in the format genMetrics.py uses for " + "run__flow__generate_date (%%Y-%%m-%%d %%H:%%M).", +) +parser.add_argument( + "--expectedDesigns", + type=int, + default=None, + help="Number of designs the pipeline set out to build. When set, emitted " + "as design_count on the pipeline payload so the backend can tell a build " + "that produced fewer (or no) metrics from one that built fewer designs.", +) +parser.add_argument( + "--noMetricsReason", + type=str, + default=None, + help="Why no design produced metrics. Emitted as no_metrics_reason only " + "on a payload with no designs.", +) # --- PUBSUB args --- parser.add_argument( @@ -194,6 +218,13 @@ def resolve_schema_version(args, provenance): return 4 if provenance else 3 +def resolve_build_time(args): + """The build time to report, defaulting to now in genMetrics.py's format.""" + if args.buildTime and args.buildTime.strip(): + return args.buildTime.strip() + return datetime.now().strftime("%Y-%m-%d %H:%M") + + def build_pipeline_payload(design_records, args, provenance=None): """Return the pipeline-level payload dict. @@ -211,10 +242,18 @@ def build_pipeline_payload(design_records, args, provenance=None): "change_branch": args.changeBranch, "commit_sha": args.commitSHA, "jenkins_url": args.jenkinsURL, + "build_time": resolve_build_time(args), "designs": design_records, } if job_name: payload["job_name"] = job_name + if args.expectedDesigns is not None: + payload["design_count"] = args.expectedDesigns + # A reason only describes a build that produced nothing; on a payload with + # designs it would contradict the metrics it carries. + reason = args.noMetricsReason.strip() if args.noMetricsReason else "" + if not design_records and reason: + payload["no_metrics_reason"] = reason # Only a v4 payload carries provenance. Provenance without a job name cannot # be v4 (the backend requires job_name from v3 up), and attaching v4 keys to # a message labelled v2 would make payload_schema_version stop describing @@ -260,6 +299,11 @@ def publish_v1_per_design(publisher, topic_path, design_records, args, provenanc requirement: a v1 message declares no schema version, so there is nothing for the extra keys to contradict, and the backend reads provenance here by key presence rather than by version. + + build_time, design_count and no_metrics_reason are left off: the backend + reads every v1 root key it does not list as metadata as a metric, so new + root keys here would surface as bogus metrics. An empty build never gets + here anyway; its payload is far below the size cap. """ futures = [] failed = 0 @@ -361,7 +405,16 @@ def publish_v1_per_design(publisher, topic_path, design_records, args, provenanc # A failed publish exits non-zero. A warning alone let CI builds pass while the # QoR dashboard stopped receiving results, which froze the baseline that later # builds are compared against. -if publisher and design_records: +# +# A build where no design produced metrics still publishes, with an empty +# designs list, so the dashboard lists it as a build without metrics instead of +# leaving it out. +if publisher: + if not design_records: + print( + "[INFO] No design records were collected. Publishing an empty " + "(no-metrics) pipeline report." + ) provenance = load_provenance(args.provenanceFile) payload = build_pipeline_payload(design_records, args, provenance) message_data = json.dumps(payload, default=str).encode("utf-8") @@ -393,6 +446,6 @@ def publish_v1_per_design(publisher, topic_path, design_records, args, provenanc except Exception as e: print(f"[ERROR] Pub/Sub publish failed for pipeline report: {e}") sys.exit(1) -elif publisher and not design_records: - print("[WARN] Pub/Sub publisher initialized but no design records were collected.") + if not design_records: + print("[INFO] Published an empty (no-metrics) pipeline report.") # --- END PUBSUB --- From e315077d4e910f5d4818c761fe961f870500b38c Mon Sep 17 00:00:00 2001 From: Miguel Dalberto Pedro Date: Tue, 6 Oct 2026 11:52:40 -0300 Subject: [PATCH 2/4] Default the reported build time to UTC palantir reads build_time as UTC, but the default came from local time, so a build published from a machine outside UTC was dated off by its offset. A beta test from a UTC-3 machine showed the build three hours in the past. Signed-off-by: Miguel Dalberto Pedro --- flow/test/test_uploadMetadata.py | 16 +++++++++++++++- flow/util/uploadMetadata.py | 15 ++++++++++----- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/flow/test/test_uploadMetadata.py b/flow/test/test_uploadMetadata.py index 329e9adbbd..59f4ee57b1 100755 --- a/flow/test/test_uploadMetadata.py +++ b/flow/test/test_uploadMetadata.py @@ -17,6 +17,7 @@ import sys import tempfile import unittest +from datetime import datetime, timezone SCRIPT = os.path.join( os.path.dirname(os.path.abspath(__file__)), "..", "util", "uploadMetadata.py" @@ -95,8 +96,10 @@ def add_design(self, platform, design, variant="base", metrics=None): with open(os.path.join(report_dir, "metadata.json"), "w") as f: json.dump(metrics or {"finish:area": 1.0}, f) - def run_script(self, *extra, fail=False): + def run_script(self, *extra, fail=False, tz=None): env = dict(os.environ) + if tz: + env["TZ"] = tz env["PYTHONPATH"] = self.stubs env["STUB_PUBSUB_OUT"] = self.out if fail: @@ -170,6 +173,17 @@ def test_empty_designs_failed_publish_exits_nonzero(self): self.assertEqual(result.returncode, 1, result.stdout + result.stderr) self.assertIn("[ERROR] Pub/Sub publish failed", result.stdout) + def test_default_build_time_is_utc(self): + # Far from UTC, so a local-time default could not pass by accident. + before = datetime.now(timezone.utc).replace(second=0, microsecond=0) + result, messages = self.run_script(tz="Pacific/Kiritimati") + after = datetime.now(timezone.utc) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + build_time = datetime.strptime( + messages[0]["data"]["build_time"], "%Y-%m-%d %H:%M" + ).replace(tzinfo=timezone.utc) + self.assertTrue(before <= build_time <= after, build_time) + def test_designs_payload_gains_build_time_only(self): self.add_design("nangate45", "gcd", metrics={"finish:area": 5.0}) result, messages = self.run_script( diff --git a/flow/util/uploadMetadata.py b/flow/util/uploadMetadata.py index 4aa647bd4c..28f358c3e6 100755 --- a/flow/util/uploadMetadata.py +++ b/flow/util/uploadMetadata.py @@ -4,7 +4,7 @@ import argparse import os import sys -from datetime import datetime +from datetime import datetime, timezone # --- PUBSUB --- from google.cloud import pubsub_v1 @@ -58,8 +58,9 @@ type=str, default=None, help="When the build ran, emitted as build_time on the pipeline payload. " - "Defaults to now, in the format genMetrics.py uses for " - "run__flow__generate_date (%%Y-%%m-%%d %%H:%%M).", + "Defaults to now in UTC, in the format genMetrics.py uses for " + "run__flow__generate_date (%%Y-%%m-%%d %%H:%%M). The backend reads it " + "as UTC.", ) parser.add_argument( "--expectedDesigns", @@ -219,10 +220,14 @@ def resolve_schema_version(args, provenance): def resolve_build_time(args): - """The build time to report, defaulting to now in genMetrics.py's format.""" + """The build time to report, defaulting to now in genMetrics.py's format. + + The default is UTC because the backend reads the value as UTC; local time + would shift the build by the agent's offset wherever that is not UTC. + """ if args.buildTime and args.buildTime.strip(): return args.buildTime.strip() - return datetime.now().strftime("%Y-%m-%d %H:%M") + return datetime.now(timezone.utc).strftime("%Y-%m-%d %H:%M") def build_pipeline_payload(design_records, args, provenance=None): From ec5b59ca727b779d22c9bfdc29a676276528bf7f Mon Sep 17 00:00:00 2001 From: Miguel Dalberto Pedro Date: Tue, 13 Oct 2026 12:36:21 -0300 Subject: [PATCH 3/4] Validate --buildTime and --expectedDesigns, warn on an ignored reason Reject a --buildTime that does not match YYYY-MM-DD HH:MM and a negative or non-integer --expectedDesigns at argument parsing, before anything is published. A blank --buildTime still defaults to now in UTC. Warn when --noMetricsReason is given but design records were collected, since the reason is then dropped. Signed-off-by: Miguel Dalberto Pedro --- flow/test/test_uploadMetadata.py | 18 ++++++++++++++ flow/util/uploadMetadata.py | 42 ++++++++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/flow/test/test_uploadMetadata.py b/flow/test/test_uploadMetadata.py index 59f4ee57b1..ff9b5ea2b3 100755 --- a/flow/test/test_uploadMetadata.py +++ b/flow/test/test_uploadMetadata.py @@ -216,6 +216,24 @@ def test_designs_payload_gains_build_time_only(self): }, ) self.assertNotIn("empty (no-metrics)", result.stdout) + self.assertIn("[WARN] --noMetricsReason was given", result.stdout) + + def test_blank_build_time_defaults_to_now(self): + result, messages = self.run_script("--buildTime", " ") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertRegex(messages[0]["data"]["build_time"], BUILD_TIME_FORMAT) + + def test_malformed_build_time_is_rejected(self): + result, messages = self.run_script("--buildTime", "2026-10-05T12:34:56Z") + self.assertEqual(result.returncode, 2, result.stdout + result.stderr) + self.assertIn("expected format", result.stderr) + self.assertEqual(messages, []) + + def test_negative_expected_designs_is_rejected(self): + result, messages = self.run_script("--expectedDesigns", "-1") + self.assertEqual(result.returncode, 2, result.stdout + result.stderr) + self.assertIn("is negative", result.stderr) + self.assertEqual(messages, []) def test_design_count_with_designs(self): self.add_design("nangate45", "gcd") diff --git a/flow/util/uploadMetadata.py b/flow/util/uploadMetadata.py index 28f358c3e6..dbcdd37c20 100755 --- a/flow/util/uploadMetadata.py +++ b/flow/util/uploadMetadata.py @@ -19,6 +19,33 @@ # make sure the working dir is flow/ os.chdir(os.path.join(os.path.dirname(os.path.abspath(__file__)), "..")) +BUILD_TIME_FORMAT = "%Y-%m-%d %H:%M" + + +def _build_time(val): + """Validate --buildTime against BUILD_TIME_FORMAT; blank means unset.""" + val = val.strip() + if not val: + return None + try: + datetime.strptime(val, BUILD_TIME_FORMAT) + except ValueError: + raise argparse.ArgumentTypeError( + f"'{val}' does not match the expected format 'YYYY-MM-DD HH:MM'" + ) + return val + + +def _non_negative_int(val): + try: + number = int(val) + except ValueError: + raise argparse.ArgumentTypeError(f"'{val}' is not an integer") + if number < 0: + raise argparse.ArgumentTypeError(f"'{val}' is negative") + return number + + # Create the argument parser parser = argparse.ArgumentParser(description="Process some integers.") @@ -55,7 +82,7 @@ ) parser.add_argument( "--buildTime", - type=str, + type=_build_time, default=None, help="When the build ran, emitted as build_time on the pipeline payload. " "Defaults to now in UTC, in the format genMetrics.py uses for " @@ -64,7 +91,7 @@ ) parser.add_argument( "--expectedDesigns", - type=int, + type=_non_negative_int, default=None, help="Number of designs the pipeline set out to build. When set, emitted " "as design_count on the pipeline payload so the backend can tell a build " @@ -225,9 +252,9 @@ def resolve_build_time(args): The default is UTC because the backend reads the value as UTC; local time would shift the build by the agent's offset wherever that is not UTC. """ - if args.buildTime and args.buildTime.strip(): - return args.buildTime.strip() - return datetime.now(timezone.utc).strftime("%Y-%m-%d %H:%M") + if args.buildTime: + return args.buildTime + return datetime.now(timezone.utc).strftime(BUILD_TIME_FORMAT) def build_pipeline_payload(design_records, args, provenance=None): @@ -259,6 +286,11 @@ def build_pipeline_payload(design_records, args, provenance=None): reason = args.noMetricsReason.strip() if args.noMetricsReason else "" if not design_records and reason: payload["no_metrics_reason"] = reason + elif reason: + print( + "[WARN] --noMetricsReason was given but design records were " + "collected. Ignoring the reason." + ) # Only a v4 payload carries provenance. Provenance without a job name cannot # be v4 (the backend requires job_name from v3 up), and attaching v4 keys to # a message labelled v2 would make payload_schema_version stop describing From a337ac690864ac3e4c0a666acc57df83b1ef83a4 Mon Sep 17 00:00:00 2001 From: Miguel Dalberto Pedro Date: Wed, 21 Oct 2026 10:20:50 -0300 Subject: [PATCH 4/4] Tighten no-metrics upload validation and logging Normalize --buildTime to zero-padded fields, leave a zero --expectedDesigns off the payload since it carries no completeness claim, and drop a --noMetricsReason longer than 100 characters with a warning instead of sending a value that would be discarded. Warnings about an ignored reason now print at the call site, a build with no records logs at WARN again, and the v1 fallback warns when it drops --expectedDesigns. Correct the v1 fallback docstring: only design_count would be read as a metric there. Add tests for report directories without metadata.json, builds with only skipped platforms, and the v1 fallback warning. Signed-off-by: Miguel Dalberto Pedro --- flow/test/test_uploadMetadata.py | 72 +++++++++++++++++++++++++++- flow/util/uploadMetadata.py | 81 ++++++++++++++++++++++---------- 2 files changed, 126 insertions(+), 27 deletions(-) diff --git a/flow/test/test_uploadMetadata.py b/flow/test/test_uploadMetadata.py index ff9b5ea2b3..ec234f6194 100755 --- a/flow/test/test_uploadMetadata.py +++ b/flow/test/test_uploadMetadata.py @@ -150,7 +150,7 @@ def test_empty_designs_publishes_report(self): self.assertEqual(payload["design_count"], 12) self.assertEqual(payload["no_metrics_reason"], "flow failed before metrics") self.assertEqual(messages[0]["attributes"]["payload_schema_version"], "3") - self.assertIn("empty (no-metrics)", result.stdout) + self.assertIn("[WARN] No design records were collected", result.stdout) def test_empty_designs_without_new_flags(self): result, messages = self.run_script() @@ -216,7 +216,10 @@ def test_designs_payload_gains_build_time_only(self): }, ) self.assertNotIn("empty (no-metrics)", result.stdout) - self.assertIn("[WARN] --noMetricsReason was given", result.stdout) + self.assertIn( + "[WARN] Ignoring --noMetricsReason because design records were collected", + result.stdout, + ) def test_blank_build_time_defaults_to_now(self): result, messages = self.run_script("--buildTime", " ") @@ -235,6 +238,71 @@ def test_negative_expected_designs_is_rejected(self): self.assertIn("is negative", result.stderr) self.assertEqual(messages, []) + def test_zero_expected_designs_is_not_sent(self): + result, messages = self.run_script("--expectedDesigns", "0") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertNotIn("design_count", messages[0]["data"]) + + def test_unpadded_build_time_is_normalized(self): + result, messages = self.run_script("--buildTime", "2026-1-5 1:3") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(messages[0]["data"]["build_time"], "2026-01-05 01:03") + + def test_reason_at_length_limit_is_kept(self): + reason = "x" * 100 + result, messages = self.run_script("--noMetricsReason", reason) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(messages[0]["data"]["no_metrics_reason"], reason) + + def test_overlong_reason_is_dropped_with_warning(self): + result, messages = self.run_script("--noMetricsReason", "x" * 101) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertNotIn("no_metrics_reason", messages[0]["data"]) + self.assertIn("longer than 100 characters", result.stdout) + + def test_report_dir_without_data_file_is_skipped(self): + self.add_design("nangate45", "gcd") + os.makedirs(os.path.join(self.flow, "reports", "nangate45", "aes", "base")) + result, messages = self.run_script("--expectedDesigns", "2") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + payload = messages[0]["data"] + self.assertEqual([d["design"] for d in payload["designs"]], ["gcd"]) + self.assertEqual(payload["design_count"], 2) + self.assertIn("[WARN] No data file for nangate45 aes base.", result.stdout) + + def test_failed_flow_without_data_files_publishes_empty_report(self): + os.makedirs(os.path.join(self.flow, "reports", "nangate45", "gcd", "base")) + result, messages = self.run_script( + "--expectedDesigns", "1", "--noMetricsReason", "no_reports_found" + ) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(len(messages), 1) + payload = messages[0]["data"] + self.assertEqual(payload["designs"], []) + self.assertEqual(payload["no_metrics_reason"], "no_reports_found") + + def test_only_skipped_platforms_publishes_empty_report(self): + self.add_design("sky130hd_fakestack", "gcd") + self.add_design("src", "gcd") + result, messages = self.run_script() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(len(messages), 1) + self.assertEqual(messages[0]["data"]["designs"], []) + self.assertIn( + "[WARN] Skiping upload sky130hd_fakestack gcd base.", result.stdout + ) + self.assertIn("[WARN] No design records were collected", result.stdout) + + def test_v1_fallback_warns_that_design_count_is_dropped(self): + # Large enough to push the pipeline payload over the 8 MB cap. + self.add_design("nangate45", "gcd", metrics={"blob": "x" * (9 * 1024 * 1024)}) + result, messages = self.run_script("--expectedDesigns", "3") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(len(messages), 1) + self.assertNotIn("payload_schema_version", messages[0]["data"]) + self.assertNotIn("design_count", messages[0]["data"]) + self.assertIn("[WARN] --expectedDesigns is not sent", result.stdout) + def test_design_count_with_designs(self): self.add_design("nangate45", "gcd") result, messages = self.run_script("--expectedDesigns", "3") diff --git a/flow/util/uploadMetadata.py b/flow/util/uploadMetadata.py index dbcdd37c20..bf74fac9e0 100755 --- a/flow/util/uploadMetadata.py +++ b/flow/util/uploadMetadata.py @@ -20,6 +20,8 @@ os.chdir(os.path.join(os.path.dirname(os.path.abspath(__file__)), "..")) BUILD_TIME_FORMAT = "%Y-%m-%d %H:%M" +# The backend drops a longer no_metrics_reason without an error. +MAX_NO_METRICS_REASON = 100 def _build_time(val): @@ -28,12 +30,13 @@ def _build_time(val): if not val: return None try: - datetime.strptime(val, BUILD_TIME_FORMAT) + parsed = datetime.strptime(val, BUILD_TIME_FORMAT) except ValueError: raise argparse.ArgumentTypeError( f"'{val}' does not match the expected format 'YYYY-MM-DD HH:MM'" ) - return val + # strptime accepts unpadded fields such as "2026-1-5 1:3". + return parsed.strftime(BUILD_TIME_FORMAT) def _non_negative_int(val): @@ -93,16 +96,20 @@ def _non_negative_int(val): "--expectedDesigns", type=_non_negative_int, default=None, - help="Number of designs the pipeline set out to build. When set, emitted " - "as design_count on the pipeline payload so the backend can tell a build " - "that produced fewer (or no) metrics from one that built fewer designs.", + help="Number of designs the pipeline set out to build. When positive, " + "emitted as design_count on the pipeline payload so the backend can tell " + "a build that produced fewer (or no) metrics from one that built fewer " + "designs. 0 is accepted but not sent, since the backend ignores it. Not " + "sent on the per-design fallback for oversized payloads.", ) parser.add_argument( "--noMetricsReason", type=str, default=None, - help="Why no design produced metrics. Emitted as no_metrics_reason only " - "on a payload with no designs.", + help="Why no design produced metrics, as a short code such as " + "no_reports_found (at most 100 characters; a longer value is dropped with " + "a warning). Emitted as no_metrics_reason only on a payload with no " + "designs.", ) # --- PUBSUB args --- @@ -257,7 +264,27 @@ def resolve_build_time(args): return datetime.now(timezone.utc).strftime(BUILD_TIME_FORMAT) -def build_pipeline_payload(design_records, args, provenance=None): +def resolve_no_metrics_reason(args, design_records): + """The no_metrics_reason to send, and why a given reason is not sent. + + Returns ``(reason, problem)``: ``reason`` is ``None`` unless it goes on the + payload, and ``problem`` describes a non-blank reason that was dropped. + """ + reason = args.noMetricsReason.strip() if args.noMetricsReason else "" + if not reason: + return None, None + # A reason only describes a build that produced nothing; on a payload with + # designs it would contradict the metrics it carries. + if design_records: + return None, "design records were collected" + if len(reason) > MAX_NO_METRICS_REASON: + return None, f"it is longer than {MAX_NO_METRICS_REASON} characters" + return reason, None + + +def build_pipeline_payload( + design_records, args, provenance=None, no_metrics_reason=None +): """Return the pipeline-level payload dict. Emits schema v2 (no job name), v3 (job name) or v4 (job name plus build @@ -279,18 +306,10 @@ def build_pipeline_payload(design_records, args, provenance=None): } if job_name: payload["job_name"] = job_name - if args.expectedDesigns is not None: + if args.expectedDesigns: payload["design_count"] = args.expectedDesigns - # A reason only describes a build that produced nothing; on a payload with - # designs it would contradict the metrics it carries. - reason = args.noMetricsReason.strip() if args.noMetricsReason else "" - if not design_records and reason: - payload["no_metrics_reason"] = reason - elif reason: - print( - "[WARN] --noMetricsReason was given but design records were " - "collected. Ignoring the reason." - ) + if no_metrics_reason: + payload["no_metrics_reason"] = no_metrics_reason # Only a v4 payload carries provenance. Provenance without a job name cannot # be v4 (the backend requires job_name from v3 up), and attaching v4 keys to # a message labelled v2 would make payload_schema_version stop describing @@ -337,10 +356,12 @@ def publish_v1_per_design(publisher, topic_path, design_records, args, provenanc for the extra keys to contradict, and the backend reads provenance here by key presence rather than by version. - build_time, design_count and no_metrics_reason are left off: the backend - reads every v1 root key it does not list as metadata as a metric, so new - root keys here would surface as bogus metrics. An empty build never gets - here anyway; its payload is far below the size cap. + design_count is left off: the backend reads every v1 root key it does not + list as metadata as a metric, and design_count is not on that list, so it + would surface as a bogus metric. build_time and no_metrics_reason are + metadata there but not needed: each design's metrics carry + run__flow__generate_date, which the backend prefers, and a reason only + applies to an empty build, whose payload is far below the size cap. """ futures = [] failed = 0 @@ -449,11 +470,16 @@ def publish_v1_per_design(publisher, topic_path, design_records, args, provenanc if publisher: if not design_records: print( - "[INFO] No design records were collected. Publishing an empty " + "[WARN] No design records were collected. Publishing an empty " "(no-metrics) pipeline report." ) + no_metrics_reason, reason_problem = resolve_no_metrics_reason(args, design_records) + if reason_problem: + print(f"[WARN] Ignoring --noMetricsReason because {reason_problem}.") provenance = load_provenance(args.provenanceFile) - payload = build_pipeline_payload(design_records, args, provenance) + payload = build_pipeline_payload( + design_records, args, provenance, no_metrics_reason + ) message_data = json.dumps(payload, default=str).encode("utf-8") if len(message_data) > MAX_PUBSUB_BYTES: @@ -462,6 +488,11 @@ def publish_v1_per_design(publisher, topic_path, design_records, args, provenanc f"{MAX_PUBSUB_BYTES // 1024} KB cap. Falling back to v1 per-design publish " f"({len(design_records)} messages)." ) + if args.expectedDesigns: + print( + "[WARN] --expectedDesigns is not sent on the v1 fallback; the " + "backend will report this build's completeness as unknown." + ) failed = publish_v1_per_design( publisher, topic_path, design_records, args, provenance )