test: integration tests for partial_match substring rejection (#193)#238
Merged
test: integration tests for partial_match substring rejection (#193)#238
Conversation
Lock the handler-level guarantee that a single-hit substring under --no-input routes through MatchResult::Ambiguous and errors before any state-changing HTTP call at three call sites identified in #193: - issue move --to "prog" (workflow.rs:129, exits 1 via bail!) - issue link --type "block" (links.rs:62, exits 1 via bail!) - issue list --status "prog" (list.rs:220, exits 64 via UserError) Each test mocks the candidate GET and mounts the state-changing POST with .expect(0) so wiremock fails the test if the short-circuit is ever bypassed. Complements the matcher's own unit tests in src/partial_match.rs; no production code changes. Scope: the three call sites named in #193. Additional uncovered callers (unlink, helpers team/user/asset, assets search/schema/tickets, queue view) filed as #236. Exit-code inconsistency between bail! (exit 1) and UserError (exit 64) filed as #237. Closes #193
There was a problem hiding this comment.
Pull request overview
Adds handler-level integration coverage to ensure single-hit substring inputs under --no-input are treated as ambiguous and fail before any downstream HTTP action, preventing regressions from the strict-matching rollout.
Changes:
- Add an integration test for
jr issue list --status <substring> --no-inputasserting exit code 64 and no JQL search call. - Add integration tests for
jr issue move <substring> --no-inputandjr issue link --type <substring> --no-inputasserting failure and no state-changing POST. - Pin current exit-code behavior for move/link (1) vs list (64) to make future normalization explicit.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/issue_list_errors.rs | Adds issue list --status ambiguous-substring rejection test and asserts no /search/jql call occurs. |
| tests/issue_commands.rs | Adds issue move and issue link ambiguous-substring rejection tests asserting no state-changing POST occurs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4 tasks
Zious11
added a commit
that referenced
this pull request
Apr 21, 2026
issue move, issue link, and issue unlink reported ambiguous-substring errors via anyhow::bail! which doesn't inject a JrError into the cause chain, so main.rs fell back to exit 1. issue list --status already used JrError::UserError (exit 64) for the same failure class, producing an inconsistency that broke scripts keying off $? == 64. Change the three Ambiguous + no_input branches to return JrError::UserError so all four handlers surface exit 64 uniformly. Error-message strings are byte-identical — no script-visible wording changes. Updates the two tests added in PR #238 to pin Some(64). Closes #237
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--no-inputroute throughMatchResult::Ambiguousand error before any state-changing HTTP call.issue move,issue link,issue list --status.How it works
Each test mocks the candidate-list GET and mounts the state-changing POST with
.expect(0). wiremock fails the test if the short-circuit is ever bypassed. Inputs are chosen sopartial_matchresolves to exactly one candidate (e.g.,"prog"→"In Progress"), exercising the Ambiguous-single-hit path rather than None or Ambiguous-multi-hit.Findings from local review (addressed / deferred)
partial_matchcallers (unlink, helpers team/user/asset, assets search/schema/tickets, queue view).bail!and exit 1, while list usesUserErrorand exits 64. All three are user-input errors and should surface as 64.Test plan
cargo fmt --all -- --checkcleancargo clippy --all-targets -- -D warningscleancargo test— 803 passed, 0 failed (baseline 782 + 21)test_move_single_substring_rejected_no_inputtest_link_single_substring_rejected_no_inputissue_list_status_single_substring_rejectedCloses #193