Skip to content

test: add build diagnostic metadata tests (0 bounty) #26

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gaomingyi779-del wants to merge 1 commit into
base: main
Choose a base branch
from
Open

test: add build diagnostic metadata tests (0 bounty) #26

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions tests/test_diagnostic_metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
#!/usr/bin/env python3
"""Tests for build diagnostic report metadata ($30 bounty, issue #3).

Validates commit ID tracking, module result summaries, logd path handling,
and deterministic output from build_diagnostic_report().
"""
from __future__ import annotations

import sys
import unittest
from pathlib import Path

sys.path.insert(0, str(Path(__file__).resolve().parents[1]))

import build # noqa: E402


class TestDiagnosticMetadata(unittest.TestCase):
"""Validates build_diagnostic_report output structure and determinism."""

# ── report metadata ─────────────────────────────────────────────

def test_report_includes_commit_id(self):
report = build.build_diagnostic_report([], "abc123")
self.assertIn("commit", report)
self.assertEqual(report["commit"], "abc123")

def test_report_includes_module_result_summaries(self):
results = [
("mod_a", True, 1.2, "output A", None),
("mod_b", False, 0.8, "output B", None),
]
report = build.build_diagnostic_report(results, "commit-1")
self.assertEqual(report["total_modules"], 2)
self.assertEqual(report["passed"], 1)
self.assertEqual(report["failed"], 1)
self.assertEqual(len(report["modules"]), 2)
# Module entries have expected keys
for mod in report["modules"]:
self.assertIn("name", mod)
self.assertIn("status", mod)
self.assertIn("elapsed_seconds", mod)

def test_module_status_pass_fail(self):
results = [
("pass_mod", True, 0.5, "ok", None),
("fail_mod", False, 1.0, "error", "artifact.bin"),
]
report = build.build_diagnostic_report(results, "c2")
mods = {m["name"]: m for m in report["modules"]}
self.assertEqual(mods["pass_mod"]["status"], "PASS")
self.assertEqual(mods["fail_mod"]["status"], "FAIL")
self.assertEqual(mods["fail_mod"]["artifact"], "artifact.bin")

# ── logd path handling ─────────────────────────────────────────

def test_report_includes_diagnostic_logd_key(self):
report = build.build_diagnostic_report([], "test")
self.assertIn("diagnostic_logd", report)

def test_logd_relpaths_appear_in_pr_note(self):
report = build.build_diagnostic_report(
[], "c3", logd_relpaths=["diagnostic/build-abc.logd"]
)
self.assertIn("build-abc.logd", report["pr_note"])

Comment on lines +55 to +66

@coderabbitai coderabbitai Bot Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Missing required coverage for failure and multi-file/chunked .logd contract

Line 55 onward only validates single-path note text. It does not cover diagnostic_logd_error behavior on generation failure, or multi-file/chunked outputs (diagnostic_logd as list, chunked, chunk_size_bytes) required by the linked objective and exposed by build.py.

Suggested test additions 
+    def test_logd_failure_sets_error_without_archive_claim(self):
+        report = build.build_diagnostic_report(
+            [], "c-fail", logd_relpaths=None, logd_error="encrypt failed"
+        )
+        self.assertEqual(report["diagnostic_logd_error"], "encrypt failed")
+        self.assertIsNone(report["diagnostic_logd"])
+        self.assertIn("was not created", report["pr_note"])
+
+    def test_multi_file_logd_and_chunked_metadata(self):
+        relpaths = ["diagnostic/build-c.logd.part1", "diagnostic/build-c.logd.part2"]
+        report = build.build_diagnostic_report([], "c-chunk", logd_relpaths=relpaths, chunked=True)
+        self.assertEqual(report["diagnostic_logd"], relpaths)
+        self.assertTrue(report["chunked"])
+        self.assertIsNotNone(report["chunk_size_bytes"])
+        self.assertIn(relpaths[0], report["pr_note"])
+        self.assertIn(relpaths[1], report["pr_note"])
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# ── logd path handling ─────────────────────────────────────────
def test_report_includes_diagnostic_logd_key(self):
report = build.build_diagnostic_report([], "test")
self.assertIn("diagnostic_logd", report)
def test_logd_relpaths_appear_in_pr_note(self):
report = build.build_diagnostic_report(
[], "c3", logd_relpaths=["diagnostic/build-abc.logd"]
)
self.assertIn("build-abc.logd", report["pr_note"])
# ── logd path handling ─────────────────────────────────────────
def test_report_includes_diagnostic_logd_key(self):
report = build.build_diagnostic_report([], "test")
self.assertIn("diagnostic_logd", report)
def test_logd_relpaths_appear_in_pr_note(self):
report = build.build_diagnostic_report(
[], "c3", logd_relpaths=["diagnostic/build-abc.logd"]
)
self.assertIn("build-abc.logd", report["pr_note"])
def test_logd_failure_sets_error_without_archive_claim(self):
report = build.build_diagnostic_report(
[], "c-fail", logd_relpaths=None, logd_error="encrypt failed"
)
self.assertEqual(report["diagnostic_logd_error"], "encrypt failed")
self.assertIsNone(report["diagnostic_logd"])
self.assertIn("was not created", report["pr_note"])
def test_multi_file_logd_and_chunked_metadata(self):
relpaths = ["diagnostic/build-c.logd.part1", "diagnostic/build-c.logd.part2"]
report = build.build_diagnostic_report([], "c-chunk", logd_relpaths=relpaths, chunked=True)
self.assertEqual(report["diagnostic_logd"], relpaths)
self.assertTrue(report["chunked"])
self.assertIsNotNone(report["chunk_size_bytes"])
self.assertIn(relpaths[0], report["pr_note"])
self.assertIn(relpaths[1], report["pr_note"])
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_diagnostic_metadata.py` around lines 55 - 66, Add missing tests in
the diagnostic metadata suite to cover the `.logd` contract beyond the existing
single-path PR note assertion. Extend the checks around
build.build_diagnostic_report and related metadata handling to verify
diagnostic_logd_error is populated when report generation fails, and add
multi-file/chunked coverage that asserts diagnostic_logd can be a list and that
chunked/chunk_size_bytes metadata is preserved. Use the existing
build_diagnostic_report and diagnostic_logd symbols to locate the logic and
ensure the new assertions match the build.py behavior.

# ── determinism ─────────────────────────────────────────────────

def test_empty_results_are_deterministic(self):
report1 = build.build_diagnostic_report([], "same-commit")
report2 = build.build_diagnostic_report([], "same-commit")
self.assertEqual(report1["total_modules"], report2["total_modules"])
self.assertEqual(report1["passed"], report2["passed"])
self.assertEqual(report1["commit"], report2["commit"])

def test_same_inputs_produce_same_output(self):
results = [("m", True, 0.1, "out", None)]
r1 = build.build_diagnostic_report(results, "c4")
r2 = build.build_diagnostic_report(results, "c4")
self.assertEqual(r1["passed"], r2["passed"])
self.assertEqual(r1["total_modules"], r2["total_modules"])

Comment on lines +69 to +82

@coderabbitai coderabbitai Bot Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Determinism tests are partial despite full-output naming

At Line 76 (test_same_inputs_produce_same_output), assertions only compare counters, so regressions in other stable fields can slip through. Either rename to reflect partial checks or assert equality on normalized reports (excluding generated_at).

Suggested tightening 
     def test_same_inputs_produce_same_output(self):
         results = [("m", True, 0.1, "out", None)]
         r1 = build.build_diagnostic_report(results, "c4")
         r2 = build.build_diagnostic_report(results, "c4")
-        self.assertEqual(r1["passed"], r2["passed"])
-        self.assertEqual(r1["total_modules"], r2["total_modules"])
+        self.assertEqual(
+            {k: v for k, v in r1.items() if k != "generated_at"},
+            {k: v for k, v in r2.items() if k != "generated_at"},
+        )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_empty_results_are_deterministic(self):
report1 = build.build_diagnostic_report([], "same-commit")
report2 = build.build_diagnostic_report([], "same-commit")
self.assertEqual(report1["total_modules"], report2["total_modules"])
self.assertEqual(report1["passed"], report2["passed"])
self.assertEqual(report1["commit"], report2["commit"])
def test_same_inputs_produce_same_output(self):
results = [("m", True, 0.1, "out", None)]
r1 = build.build_diagnostic_report(results, "c4")
r2 = build.build_diagnostic_report(results, "c4")
self.assertEqual(r1["passed"], r2["passed"])
self.assertEqual(r1["total_modules"], r2["total_modules"])
def test_empty_results_are_deterministic(self):
report1 = build.build_diagnostic_report([], "same-commit")
report2 = build.build_diagnostic_report([], "same-commit")
self.assertEqual(report1["total_modules"], report2["total_modules"])
self.assertEqual(report1["passed"], report2["passed"])
self.assertEqual(report1["commit"], report2["commit"])
def test_same_inputs_produce_same_output(self):
results = [("m", True, 0.1, "out", None)]
r1 = build.build_diagnostic_report(results, "c4")
r2 = build.build_diagnostic_report(results, "c4")
self.assertEqual(
{k: v for k, v in r1.items() if k != "generated_at"},
{k: v for k, v in r2.items() if k != "generated_at"},
)
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_diagnostic_metadata.py` around lines 69 - 82, The determinism test
named test_same_inputs_produce_same_output only checks a couple of counters, so
it can miss regressions in other stable fields from
build.build_diagnostic_report. Update this test to compare the full report
content in a normalized way, excluding non-deterministic fields such as
generated_at, or rename the test so it clearly reflects that it only validates
partial equality. Use build.build_diagnostic_report and the existing report
dictionaries to ensure all stable fields are covered.

# ── no external dependencies ────────────────────────────────────

def test_build_diagnostic_report_no_network(self):
"""Report generation does not require network access."""
report = build.build_diagnostic_report(
[("isolated", True, 0.0, "", None)],
"no-net-commit",
)
self.assertEqual(report["total_modules"], 1)

def test_build_diagnostic_report_no_file_io(self):
"""Report generation is pure in-memory (no disk writes)."""
import tempfile
report = build.build_diagnostic_report([], "mem-only")
self.assertIsInstance(report, dict)
self.assertIn("generated_at", report)


if __name__ == "__main__":
unittest.main()