Skip to content

test: close 11 test gaps and fix @{ rule in validate_git_ref_name#367

Merged
jamesadevine merged 2 commits into
mainfrom
copilot/test-gap-analysis-fixes
Apr 30, 2026
Merged

test: close 11 test gaps and fix @{ rule in validate_git_ref_name#367
jamesadevine merged 2 commits into
mainfrom
copilot/test-gap-analysis-fixes

Conversation

Copilot AI commented Apr 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Adds 31 unit tests + 1 integration test addressing every gap from the test gap analysis: validate_git_ref_name (security-critical, all 10+ rules), the standalone compile/common.rs generators (generate_setup_job, generate_teardown_job, generate_agentic_depends_on, generate_finalize_steps, generate_checkout_steps, generate_repositories), agent_stats::sanitize_for_markdown ##[ branch, missing create_branch validation paths, and create_pr::truncate_error_body Unicode boundary.

While writing the at-brace test I uncovered a real bug:

  • src/safeoutputs/mod.rs — validate_git_ref_name checked the literal string "@{{" (3 chars) instead of "@{", so the @{0} reflog-injection rule never fired. One-character fix; test_validate_git_ref_name_rejects_at_brace confirms branch@{0} is now rejected. The format-string '@{{' in the ensure! message already renders to '@{' for users, so no message change.
// before — literal "@{{" (3 chars), never matches a real ref
ensure!(!name.contains("@{{"), "{label} must not contain '@{{'");
// after — correctly matches "@{"
ensure!(!name.contains("@{"),  "{label} must not contain '@{{'");

Test assertions in the issue used hypothetical names (SetupJob, dependsOn: ProcessSafeOutputs, 3-arg generators with display names); I aligned them to what the code actually emits today (Setup/Teardown, dependsOn: Execution/Setup, 2-arg signatures).

Test plan

  • cargo test — 1049 tests pass (was 1018; +31 unit, +1 integration)
  • cargo clippy --all-targets --all-features — no new warnings
  • New integration test compiles complete-agent.md and asserts - job: Setup, - job: Teardown, dependsOn: Setup, and the literal setup/teardown step content all appear in the output

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • spsprodeus21.vssps.visualstudio.com
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 -m elf_x86_64 --hash-style=gnu --as-needed -dynamic-linker /lib64/ld-linux-x86-64.so.2 -pie -z now -z relro pi6s�� krw7k4z8nw604v1xndo.0g8kkum.rcgu.o e8ty8n7mb6ejfu5i5do.0g8kkum.rcgu.o lib/rustlib/x86_--crate-type lib/rustlib/x86_/tmp/check lib/rustlib/x86_--emit=dep-info,metadata,link lib/rustlib/x86_-C lib/rustlib/x86_embed-bitcode=no (dns block)
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 nt79chsxk0t8dkruebs.0g8kkum.rcgu.o hqwtedbizwfardrdckl.0g8kkum.rcgu.o 8kp4p7l5gve4ov6i9zn.0g8kkum.rcgu.o pi6s�� krw7k4z8nw604v1xndo.0g8kkum.rcgu.o 8atel5l3op9z678oy5n.02mhfpe.rcgu.o xx36woxfl7gremo3ua9.02mhfpe.rcgu.o ilk8m5xuz4zwrxvetrr.02mhfpe.rcgu.o so7d4u0hp112ow3jv3x.02mhfpe.rcgu.o coq2x4gd01bhkez88r1.02mhfpe.rcgu.o dn9sguvfbwe9cthd0xu.02mhfpe.rcgu.o qzxtm92rf11xgqb3iuy.02mhfpe.rcgu.o wi2ps31hgwqos9b990j.02mhfpe.rcgu.o b6gih0ja96gef7h0e2j.02mhfpe.rcgu.o p9ztercifwqae6vafgh.02mhfpe.rcgu.o uj498wtd9txhz7jddu8.02mhfpe.rcgu.o 5vvi9jr1qp5uuejei6i.02mhfpe.rcgu.o hbl8ezlwmnw18avaft9.02mhfpe.rcgu.o 8tyyyneef8n1tctu6lc.02mhfpe.rcgu.o (dns block)
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 2line-3367f26bd4remote d cgu.0.rcgu.o r/wo�� .rlib 6b2b26a06.rlib 327.rlib 653.rlib 86b29d.rlib bd.rlib ad3.rlib 9c0c�� ba3e0fd.rlib 766fa6.rlib alloc-76b5fe9328c1063f.rlib 6e1dc71b.rlib ccc.rlib target/debug/depget-url target/debug/deporigin (dns block)
  • spsprodweu4.vssps.visualstudio.com
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 -m elf_x86_64 --hash-style=gnu --as-needed -dynamic-linker /lib64/ld-linux-x86-64.so.2 -pie -z now -z relro pi6s�� krw7k4z8nw604v1xndo.0g8kkum.rcgu.o e8ty8n7mb6ejfu5i5do.0g8kkum.rcgu.o lib/rustlib/x86_--crate-type lib/rustlib/x86_/tmp/check lib/rustlib/x86_--emit=dep-info,metadata,link lib/rustlib/x86_-C lib/rustlib/x86_embed-bitcode=no (dns block)
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 nt79chsxk0t8dkruebs.0g8kkum.rcgu.o hqwtedbizwfardrdckl.0g8kkum.rcgu.o 8kp4p7l5gve4ov6i9zn.0g8kkum.rcgu.o pi6s�� krw7k4z8nw604v1xndo.0g8kkum.rcgu.o 8atel5l3op9z678oy5n.02mhfpe.rcgu.o xx36woxfl7gremo3ua9.02mhfpe.rcgu.o ilk8m5xuz4zwrxvetrr.02mhfpe.rcgu.o so7d4u0hp112ow3jv3x.02mhfpe.rcgu.o coq2x4gd01bhkez88r1.02mhfpe.rcgu.o dn9sguvfbwe9cthd0xu.02mhfpe.rcgu.o qzxtm92rf11xgqb3iuy.02mhfpe.rcgu.o wi2ps31hgwqos9b990j.02mhfpe.rcgu.o b6gih0ja96gef7h0e2j.02mhfpe.rcgu.o p9ztercifwqae6vafgh.02mhfpe.rcgu.o uj498wtd9txhz7jddu8.02mhfpe.rcgu.o 5vvi9jr1qp5uuejei6i.02mhfpe.rcgu.o hbl8ezlwmnw18avaft9.02mhfpe.rcgu.o 8tyyyneef8n1tctu6lc.02mhfpe.rcgu.o (dns block)
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 2line-3367f26bd4remote d cgu.0.rcgu.o r/wo�� .rlib 6b2b26a06.rlib 327.rlib 653.rlib 86b29d.rlib bd.rlib ad3.rlib 9c0c�� ba3e0fd.rlib 766fa6.rlib alloc-76b5fe9328c1063f.rlib 6e1dc71b.rlib ccc.rlib target/debug/depget-url target/debug/deporigin (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI linked an issue Apr 30, 2026 that may be closed by this pull request
🧪 Test gap analysis — 11 gaps found in standalone generators, git ref validation, and create_pr helpers #242
Closed
@jamesadevine
test: add 31 unit tests covering 11 gaps; fix @{ rule in validate_git…
1cc8cb2 
…_ref_name

Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/26a33f11-50ca-4f4f-aa31-1e90fe06bcbe

Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Copilot AI changed the title [WIP] Add tests for gap analysis in generator modules test: close 11 test gaps and fix @{ rule in validate_git_ref_name Apr 30, 2026
Copilot AI requested a review from jamesadevine April 30, 2026 13:31
@jamesadevine

jamesadevine commented Apr 30, 2026

Copy link
Copy Markdown
Collaborator

/rust-review

@github-actions

github-actions Bot commented Apr 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

✅ Rust PR Reviewer completed successfully!

@github-actions

github-actions Bot commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

🔍 Rust PR Review

Summary: Looks good — the bug fix is correct and critical, and the tests are high quality with no issues found.

Findings

✅ What Looks Good

  • Bug fix is correct (src/safeoutputs/mod.rs:218): The original "@\{\{" is the three-character Rust string @\{\{ (literal double-brace), which would never match a real git reflog expression like branch@{0}. The fix to "@{" correctly catches the two-character sequence that git uses for reflog syntax. The ensure! message '@\{\{' was already correct (renders as '@{' via format string escaping) — only the check string needed fixing. High-impact security fix.

  • truncate_error_body Unicode test (src/safeoutputs/create_pr.rs): The test_truncate_error_body_multibyte_boundary test correctly validates the char_indices().nth(max_len) implementation — "héllo" with max_len=3 yields "hél" (3 chars, 4 bytes), proving no byte-boundary splitting occurs on multi-byte characters.

  • validate_git_ref_name test coverage: The 10 new tests exhaustively cover all rules in the function — empty, .., @{, trailing ., .lock, \\, //, special chars ~^:?*[, dot-prefixed components, and a positive acceptance case. Each assertion maps directly to a corresponding ensure! in the implementation.

  • generate_* generator tests: Unit tests correctly match what the functions actually emit (e.g., dependsOn: Execution for teardown, dependsOn: Setup for the agentic job). The integration test in tests/compiler_tests.rs uses the complete-agent.md fixture which genuinely has setup: and teardown: sections, so the assertions are grounded.

  • create_branch validation tests: The four new paths (dash-prefix, spaces, >200 chars, .. in source_branch) all call try_into() which goes through the full validation pipeline, making these true end-to-end unit tests.

  • All 1049 tests pass locally (cargo test).

Generated by Rust PR Reviewer for issue #367 · ● 537.2K · ◷

@jamesadevine jamesadevine marked this pull request as ready for review April 30, 2026 17:18
@jamesadevine jamesadevine merged commit 701674a into main Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jamesadevine jamesadevine Awaiting requested review from jamesadevine

Assignees

@jamesadevine jamesadevine

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

🧪 Test gap analysis — 11 gaps found in standalone generators, git ref validation, and create_pr helpers

2 participants

@jamesadevine