Skip to content

test: fix vacuous is_err() assertions in add_pr_comment and update_pr#1574

Merged
jamesadevine merged 1 commit into
mainfrom
test-reducer/add-pr-comment-update-pr-2026-07-19-02680347df2dd62b
Jul 21, 2026
Merged

test: fix vacuous is_err() assertions in add_pr_comment and update_pr#1574
jamesadevine merged 1 commit into
mainfrom
test-reducer/add-pr-comment-update-pr-2026-07-19-02680347df2dd62b

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Test Suite Reduction: src/safe_outputs/add_pr_comment.rs + src/safe_outputs/update_pr.rs

What was wrong

Five validation-rejection tests used bare is_err() without inspecting the error message. A silent change to the validation logic — e.g. the wrong field being checked, or a different code path reaching the error return — would have let these tests pass while the feature regressed unnoticed.

Test Problem
add_pr_comment::test_validation_rejects_zero_pr_id assert!(result.is_err()) — doesn't verify the guard that fired
add_pr_comment::test_validation_rejects_short_content assert!(result.is_err()) — doesn't verify the content-length message
add_pr_comment::test_validation_rejects_repository_pipeline_command assert!(result.is_err()) — doesn't verify the injection-detection message
add_pr_comment::test_validation_rejects_line_without_file_path assert!(result.is_err()) — doesn't verify the cross-field constraint message
update_pr::test_validation_rejects_invalid_operation assert!(result.is_err()) — doesn't verify the allowed-operations message

Changes

Each test now calls unwrap_err().to_string() and asserts the error string contains the key phrase that identifies which guard fired:

Test Action Asserted phrase
test_validation_rejects_zero_pr_id Rewritten "pull_request_id must be positive"
test_validation_rejects_short_content Rewritten "content must be at least 10 characters"
test_validation_rejects_repository_pipeline_command Rewritten "pipeline command"
test_validation_rejects_line_without_file_path Rewritten "line requires file_path"
test_validation_rejects_invalid_operation Rewritten "operation must be one of"

Verification

  • cargo test: all tests pass ✅
  • cargo clippy --all-targets --all-features: no errors ✅

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • spsprodeus21.vssps.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "spsprodeus21.vssps.visualstudio.com"

See Network Configuration for more information.

Generated by Test Reducer · 238.6 AIC · ⌖ 12.1 AIC · ⊞ 8.7K ·

Strengthen 5 validation rejection tests that previously only checked
is_err() without verifying the error message content.  A silent change
to the validation logic (e.g. wrong field guarded, wrong error path
reached) would have let these tests pass unnoticed.

- add_pr_comment::test_validation_rejects_zero_pr_id
- add_pr_comment::test_validation_rejects_short_content
- add_pr_comment::test_validation_rejects_repository_pipeline_command
- add_pr_comment::test_validation_rejects_line_without_file_path
- update_pr::test_validation_rejects_invalid_operation

Each test now calls unwrap_err() and asserts the error string contains
the key phrase that identifies the guard that fired.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jamesadevine
jamesadevine marked this pull request as ready for review July 21, 2026 13:44
@jamesadevine
jamesadevine merged commit 74db9b7 into main Jul 21, 2026
1 of 4 checks passed
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.

1 participant