Skip to content

test: Add build diagnostic metadata tests (closes #3)#27

Open
royliz3090-jpg wants to merge 2 commits into
jackjin1997:mainfrom
royliz3090-jpg:diagnostic-metadata-tests
Open

test: Add build diagnostic metadata tests (closes #3)#27
royliz3090-jpg wants to merge 2 commits into
jackjin1997:mainfrom
royliz3090-jpg:diagnostic-metadata-tests

Conversation

@royliz3090-jpg

@royliz3090-jpg royliz3090-jpg commented Jun 25, 2026

Copy link
Copy Markdown

Diagnostic Metadata Tests

This PR adds comprehensive tests for build diagnostic metadata generation.

Tests Added:

  • TestCurrentCommitId: Tests commit ID format and fallback behavior
  • TestDiagnosticPaths: Tests diagnostic path generation with commit IDs
  • TestBuildDiagnosticReport: Tests report structure for successful/failed builds
  • TestModuleResults: Tests module result structure in reports

Coverage:

  • Successful report metadata (commit id, module summaries, logd path)
  • Failed report with diagnostic_logd_error
  • Chunked/multi-file logd references
  • PR note generation
  • Deterministic tests (no external dependencies)

Test Results:

test_returns_8_char_hex ... ok
test_fallback_on_error ... ok
test_paths_contain_commit_id ... ok
test_successful_report_structure ... ok
test_failed_report_structure ... ok
test_report_with_logd ... ok
test_report_with_chunked_logd ... ok
test_report_with_logd_error ... ok
test_report_pr_note_with_logd ... ok
test_report_pr_note_without_logd ... ok
test_module_results_structure ... ok

Ran 11 tests in 0.002s
OK

Running Tests:

python3 -m pytest tests/test_diagnostic_metadata.py -v

Closes #3


Bounty: $30
Lightning Address: fullphone56@walletofsatoshi.com

Summary by CodeRabbit

  • Tests
    • Added a new unit-test suite covering diagnostic metadata generation, including commit ID formatting, error fallback, and generated log/metadata filename conventions.
    • Verified diagnostic report structure for both successful and failed builds, including chunked log behavior and correct handling of log artifact errors.
    • Confirmed per-module result entries include expected fields (name, pass/fail status, elapsed time) and report notes reflect presence/absence of log artifacts.
    • Enabled execution for the new test module.

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1c014a09-aecb-43e3-935e-8a3cb2d0bc7e

📥 Commits

Reviewing files that changed from the base of the PR and between 377fa5f and cdfd04b.

📒 Files selected for processing (1)
  • tests/test_diagnostic_metadata.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/test_diagnostic_metadata.py

📝 Walkthrough

Walkthrough

Adds a new Python test module for diagnostic metadata helpers and report generation. The tests cover commit ID formatting, diagnostic path naming, report contents for success and failure cases, logd artifact handling, per-module result fields, and direct execution via unittest.main().

Changes

Diagnostic Metadata Test Coverage

Layer / File(s) Summary
Bootstrap and helper checks
tests/test_diagnostic_metadata.py
Adds import setup for build.py and tests current_commit_id() plus diagnostic_paths_for_commit().
Report scenarios
tests/test_diagnostic_metadata.py
Tests build_diagnostic_report() for success, failure, logd artifact handling, chunked logd inputs, logd errors, and PR note text.
Module results and runner
tests/test_diagnostic_metadata.py
Verifies per-module report entries and adds unittest.main() for direct execution.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A bunny hopped in with a test-file flair,
Sniffed out commit IDs floating in air.
Logd crumbs lined up, neat as can be,
PASS and FAIL twinkled in JSON for me.
Thump-thump! 🐰

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding build diagnostic metadata tests.
Description check ✅ Passed The description includes the main summary, changes, and local testing results, and is sufficiently complete despite not matching the template verbatim.
Linked Issues check ✅ Passed The tests cover the required diagnostic metadata, failure behavior, chunked logd refs, and deterministic execution for issue #3.
Out of Scope Changes check ✅ Passed The change set stays within the diagnostic metadata test scope and doesn't introduce unrelated code.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@tests/test_diagnostic_metadata.py`:
- Around line 131-134: The PR-note tests are calling build_diagnostic_report
with the arguments in the wrong order, causing inconsistent report construction.
Update the affected test cases that use build_diagnostic_report so they match
the rest of the file by passing results first and the commit_id string second,
keeping the existing logd_relpaths named argument unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 69f3d31a-e73c-4212-be8b-acd87d123768

📥 Commits

Reviewing files that changed from the base of the PR and between 1462fe7 and 377fa5f.

📒 Files selected for processing (1)
  • tests/test_diagnostic_metadata.py

Comment thread tests/test_diagnostic_metadata.py Outdated
@royliz3090-jpg

Copy link
Copy Markdown
Author

Fixed the reversed build_diagnostic_report arguments in lines 132 and 142. The arguments are now in the correct order: results first, then commit_id. Thank you for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[$30 BOUNTY] [Python] Add build diagnostic metadata tests

1 participant