Skip to content

Commit 701674a

Browse files
test: close 11 test gaps and fix @{ rule in validate_git_ref_name (#367)
* Initial plan * test: add 31 unit tests covering 11 gaps; fix @{ rule in validate_git_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> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
1 parent 10a8537 commit 701674a

6 files changed

Lines changed: 287 additions & 1 deletion

File tree

src/agent_stats.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,14 @@ mod tests {
355355
);
356356
}
357357

358+
#[test]
359+
fn test_sanitize_for_markdown_strips_shorthand_pipeline_command() {
360+
assert_eq!(
361+
sanitize_for_markdown("##[error]Something bad"),
362+
"[filtered][error]Something bad"
363+
);
364+
}
365+
358366
#[test]
359367
fn test_internal_tools_excluded_from_count() {
360368
let entries = vec![

src/compile/common.rs

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4773,4 +4773,116 @@ mod tests {
47734773
let warnings = validate::warn_potential_secrets("my-mcp", &env, &headers);
47744774
assert!(warnings.is_empty(), "non-secret env var should not produce warnings");
47754775
}
4776+
4777+
// ─── standalone setup/teardown/finalize/checkout/repositories generators ───
4778+
4779+
#[test]
4780+
fn test_generate_setup_job_empty_returns_empty() {
4781+
assert!(generate_setup_job(&[], "MyPool").is_empty());
4782+
}
4783+
4784+
#[test]
4785+
fn test_generate_setup_job_with_steps() {
4786+
let step: serde_yaml::Value = serde_yaml::from_str("bash: echo setup").unwrap();
4787+
let out = generate_setup_job(&[step], "MyPool");
4788+
assert!(out.contains("- job: Setup"), "out: {out}");
4789+
assert!(out.contains("displayName: \"Setup\""), "out: {out}");
4790+
assert!(out.contains("name: MyPool"), "out: {out}");
4791+
assert!(out.contains("- checkout: self"), "out: {out}");
4792+
assert!(out.contains("echo setup"), "out: {out}");
4793+
}
4794+
4795+
#[test]
4796+
fn test_generate_teardown_job_empty_returns_empty() {
4797+
assert!(generate_teardown_job(&[], "MyPool").is_empty());
4798+
}
4799+
4800+
#[test]
4801+
fn test_generate_teardown_job_with_steps() {
4802+
let step: serde_yaml::Value = serde_yaml::from_str("bash: echo td").unwrap();
4803+
let out = generate_teardown_job(&[step], "MyPool");
4804+
assert!(out.contains("- job: Teardown"), "out: {out}");
4805+
assert!(out.contains("dependsOn: Execution"), "out: {out}");
4806+
assert!(out.contains("name: MyPool"), "out: {out}");
4807+
assert!(out.contains("echo td"), "out: {out}");
4808+
}
4809+
4810+
#[test]
4811+
fn test_generate_agentic_depends_on_empty_steps() {
4812+
assert!(generate_agentic_depends_on(&[]).is_empty());
4813+
}
4814+
4815+
#[test]
4816+
fn test_generate_agentic_depends_on_with_steps() {
4817+
let step: serde_yaml::Value = serde_yaml::from_str("bash: x").unwrap();
4818+
assert_eq!(generate_agentic_depends_on(&[step]), "dependsOn: Setup");
4819+
}
4820+
4821+
#[test]
4822+
fn test_generate_finalize_steps_empty() {
4823+
assert!(generate_finalize_steps(&[]).is_empty());
4824+
}
4825+
4826+
#[test]
4827+
fn test_generate_finalize_steps_with_step() {
4828+
let step: serde_yaml::Value = serde_yaml::from_str("bash: echo done").unwrap();
4829+
let out = generate_finalize_steps(&[step]);
4830+
assert!(out.contains("echo done"), "out: {out}");
4831+
}
4832+
4833+
#[test]
4834+
fn test_generate_checkout_steps_empty() {
4835+
assert!(generate_checkout_steps(&[]).is_empty());
4836+
}
4837+
4838+
#[test]
4839+
fn test_generate_checkout_steps_multiple() {
4840+
let aliases = vec!["repo-a".to_string(), "repo-b".to_string()];
4841+
let out = generate_checkout_steps(&aliases);
4842+
assert!(out.contains("- checkout: repo-a"), "out: {out}");
4843+
assert!(out.contains("- checkout: repo-b"), "out: {out}");
4844+
}
4845+
4846+
#[test]
4847+
fn test_generate_repositories_empty() {
4848+
assert!(generate_repositories(&[]).is_empty());
4849+
}
4850+
4851+
#[test]
4852+
fn test_generate_repositories_single() {
4853+
let repos = vec![Repository {
4854+
repository: "my-repo".to_string(),
4855+
repo_type: "git".to_string(),
4856+
name: "org/my-repo".to_string(),
4857+
repo_ref: "refs/heads/main".to_string(),
4858+
}];
4859+
let out = generate_repositories(&repos);
4860+
assert!(out.contains("- repository: my-repo"), "out: {out}");
4861+
assert!(out.contains("type: git"), "out: {out}");
4862+
assert!(out.contains("name: org/my-repo"), "out: {out}");
4863+
assert!(out.contains("ref: refs/heads/main"), "out: {out}");
4864+
}
4865+
4866+
#[test]
4867+
fn test_generate_repositories_multiple() {
4868+
let repos = vec![
4869+
Repository {
4870+
repository: "repo-a".to_string(),
4871+
repo_type: "git".to_string(),
4872+
name: "org/repo-a".to_string(),
4873+
repo_ref: "refs/heads/main".to_string(),
4874+
},
4875+
Repository {
4876+
repository: "repo-b".to_string(),
4877+
repo_type: "git".to_string(),
4878+
name: "org/repo-b".to_string(),
4879+
repo_ref: "refs/heads/develop".to_string(),
4880+
},
4881+
];
4882+
let out = generate_repositories(&repos);
4883+
assert!(out.contains("- repository: repo-a"), "out: {out}");
4884+
assert!(out.contains("- repository: repo-b"), "out: {out}");
4885+
assert!(out.contains("name: org/repo-a"), "out: {out}");
4886+
assert!(out.contains("ref: refs/heads/develop"), "out: {out}");
4887+
}
47764888
}

src/safeoutputs/create_branch.rs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,54 @@ mod tests {
460460
assert!(result.is_err());
461461
}
462462

463+
#[test]
464+
fn test_validation_rejects_branch_starting_with_dash() {
465+
let params = CreateBranchParams {
466+
branch_name: "-bad".to_string(),
467+
source_branch: None,
468+
source_commit: None,
469+
repository: None,
470+
};
471+
let result: Result<CreateBranchResult, _> = params.try_into();
472+
assert!(result.is_err(), "branch starting with '-' should be rejected");
473+
}
474+
475+
#[test]
476+
fn test_validation_rejects_branch_with_spaces() {
477+
let params = CreateBranchParams {
478+
branch_name: "my branch".to_string(),
479+
source_branch: None,
480+
source_commit: None,
481+
repository: None,
482+
};
483+
let result: Result<CreateBranchResult, _> = params.try_into();
484+
assert!(result.is_err(), "branch with spaces should be rejected");
485+
}
486+
487+
#[test]
488+
fn test_validation_rejects_branch_over_200_chars() {
489+
let params = CreateBranchParams {
490+
branch_name: "a".repeat(201),
491+
source_branch: None,
492+
source_commit: None,
493+
repository: None,
494+
};
495+
let result: Result<CreateBranchResult, _> = params.try_into();
496+
assert!(result.is_err(), "branch >200 chars should be rejected");
497+
}
498+
499+
#[test]
500+
fn test_validation_rejects_source_branch_with_traversal() {
501+
let params = CreateBranchParams {
502+
branch_name: "feature/valid".to_string(),
503+
source_branch: Some("../evil".to_string()),
504+
source_commit: None,
505+
repository: None,
506+
};
507+
let result: Result<CreateBranchResult, _> = params.try_into();
508+
assert!(result.is_err(), "source_branch with '..' should be rejected");
509+
}
510+
463511
#[test]
464512
fn test_result_serializes_correctly() {
465513
let params = CreateBranchParams {

src/safeoutputs/create_pr.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2367,4 +2367,34 @@ new file mode 100755
23672367
assert!(config.auto_complete);
23682368
}
23692369

2370+
// ─── truncate_error_body ────────────────────────────────────────────────
2371+
2372+
#[test]
2373+
fn test_truncate_error_body_shorter_than_max() {
2374+
assert_eq!(truncate_error_body("hello", 100), "hello");
2375+
}
2376+
2377+
#[test]
2378+
fn test_truncate_error_body_exact_max() {
2379+
assert_eq!(truncate_error_body("hello", 5), "hello");
2380+
}
2381+
2382+
#[test]
2383+
fn test_truncate_error_body_longer_than_max() {
2384+
assert_eq!(truncate_error_body("hello world", 5), "hello");
2385+
}
2386+
2387+
#[test]
2388+
fn test_truncate_error_body_multibyte_boundary() {
2389+
// "héllo" — é is 2 bytes; truncation must not split a multi-byte char.
2390+
// max_len is char count: 3 chars = "hél".
2391+
let s = "héllo";
2392+
let result = truncate_error_body(s, 3);
2393+
assert_eq!(result, "hél");
2394+
}
2395+
2396+
#[test]
2397+
fn test_truncate_error_body_empty_input() {
2398+
assert_eq!(truncate_error_body("", 5), "");
2399+
}
23702400
}

src/safeoutputs/mod.rs

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ pub(crate) fn validate_git_ref_name(name: &str, label: &str) -> anyhow::Result<(
215215

216216
ensure!(!name.is_empty(), "{label} must not be empty");
217217
ensure!(!name.contains(".."), "{label} must not contain '..'");
218-
ensure!(!name.contains("@{{"), "{label} must not contain '@{{'");
218+
ensure!(!name.contains("@{"), "{label} must not contain '@{{'");
219219
ensure!(!name.ends_with('.'), "{label} must not end with '.'");
220220
ensure!(!name.ends_with(".lock"), "{label} must not end with '.lock'");
221221
ensure!(
@@ -391,4 +391,64 @@ mod tests {
391391
"ALL_KNOWN_SAFE_OUTPUTS should be the union of write + diagnostic + non-MCP lists"
392392
);
393393
}
394+
395+
// ─── validate_git_ref_name ──────────────────────────────────────────────
396+
397+
#[test]
398+
fn test_validate_git_ref_name_rejects_at_brace() {
399+
assert!(validate_git_ref_name("branch@{0}", "b").is_err());
400+
}
401+
402+
#[test]
403+
fn test_validate_git_ref_name_rejects_dotlock_suffix() {
404+
assert!(validate_git_ref_name("my-branch.lock", "b").is_err());
405+
}
406+
407+
#[test]
408+
fn test_validate_git_ref_name_rejects_consecutive_slashes() {
409+
assert!(validate_git_ref_name("feat//thing", "b").is_err());
410+
}
411+
412+
#[test]
413+
fn test_validate_git_ref_name_rejects_backslash() {
414+
assert!(validate_git_ref_name("feat\\evil", "b").is_err());
415+
}
416+
417+
#[test]
418+
fn test_validate_git_ref_name_rejects_special_chars() {
419+
for ch in ['~', '^', ':', '?', '*', '['] {
420+
let name = format!("feat{ch}bad");
421+
assert!(
422+
validate_git_ref_name(&name, "b").is_err(),
423+
"should reject '{ch}'"
424+
);
425+
}
426+
}
427+
428+
#[test]
429+
fn test_validate_git_ref_name_rejects_component_starting_with_dot() {
430+
assert!(validate_git_ref_name("feat/.hidden", "b").is_err());
431+
}
432+
433+
#[test]
434+
fn test_validate_git_ref_name_rejects_trailing_dot() {
435+
assert!(validate_git_ref_name("my-branch.", "b").is_err());
436+
}
437+
438+
#[test]
439+
fn test_validate_git_ref_name_rejects_double_dot() {
440+
assert!(validate_git_ref_name("foo..bar", "b").is_err());
441+
}
442+
443+
#[test]
444+
fn test_validate_git_ref_name_rejects_empty() {
445+
assert!(validate_git_ref_name("", "b").is_err());
446+
}
447+
448+
#[test]
449+
fn test_validate_git_ref_name_accepts_valid_refs() {
450+
assert!(validate_git_ref_name("feature/add-login", "b").is_ok());
451+
assert!(validate_git_ref_name("v1.2.3", "b").is_ok());
452+
assert!(validate_git_ref_name("release/2026-04-17", "b").is_ok());
453+
}
394454
}

tests/compiler_tests.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3110,6 +3110,34 @@ fn test_standalone_complete_compiled_output_is_valid_yaml() {
31103110
assert_valid_yaml(&compiled, "complete-agent.md");
31113111
}
31123112

3113+
/// Test that the complete standalone fixture emits Setup/Teardown jobs and
3114+
/// that the agentic task waits on Setup. The fixture has `setup:`,
3115+
/// `teardown:`, and `post-steps:` sections so all three should appear.
3116+
#[test]
3117+
fn test_standalone_complete_agent_has_setup_and_teardown_jobs() {
3118+
let compiled = compile_fixture("complete-agent.md");
3119+
assert!(
3120+
compiled.contains("- job: Setup"),
3121+
"Should generate Setup job: {compiled}"
3122+
);
3123+
assert!(
3124+
compiled.contains("- job: Teardown"),
3125+
"Should generate Teardown job"
3126+
);
3127+
assert!(
3128+
compiled.contains("dependsOn: Setup"),
3129+
"Agentic task should depend on Setup job"
3130+
);
3131+
assert!(
3132+
compiled.contains("echo \"Setup step\"") || compiled.contains("echo 'Setup step'"),
3133+
"Should include setup step content"
3134+
);
3135+
assert!(
3136+
compiled.contains("echo \"Teardown step\"") || compiled.contains("echo 'Teardown step'"),
3137+
"Should include teardown step content"
3138+
);
3139+
}
3140+
31133141
/// Test that the pipeline-trigger fixture produces valid YAML
31143142
#[test]
31153143
fn test_standalone_pipeline_trigger_compiled_output_is_valid_yaml() {

0 commit comments

Comments
 (0)