Consolidate ask_user E2E snapshots into single canonical folder#1311
Merged
Conversation
Three sibling folders existed for the same scenario in test/snapshots/: - ask_user/ (canonical, targeted by all five SDK harnesses) - ask-user/ (unused legacy duplicate) - askuser/ (unused legacy duplicate) All five SDK E2E harnesses (Node, Python, Go, .NET, Rust) resolve their snapshot path to ask_user/. Removed the two duplicate folders and three stale files inside ask_user/ that were left over from before the test names were normalized to use a should_ prefix (PR #304). Only the three canonical should_*.yaml files remain. Fixes #314 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Consolidates duplicate ask_user E2E snapshot folders into the single canonical test/snapshots/ask_user/ directory that all five SDK harnesses actually resolve to, eliminating dead snapshots and snapshot drift.
Changes:
- Deletes the unused
test/snapshots/ask-user/folder (3should_*.yamlfiles). - Deletes the unused
test/snapshots/askuser/folder (3should_*.yamlfiles). - Removes 3 stale pre-
should_-prefix files fromtest/snapshots/ask_user/, keeping only the canonicalshould_*.yamlsnapshots.
Show a summary per file
| File | Description |
|---|---|
| test/snapshots/ask-user/should_receive_choices_in_user_input_request.yaml | Removes unused duplicate snapshot. |
| test/snapshots/ask-user/should_invoke_user_input_handler_when_model_uses_ask_user_tool.yaml | Removes unused duplicate snapshot. |
| test/snapshots/ask-user/should_handle_freeform_user_input_response.yaml | Removes unused duplicate snapshot. |
| test/snapshots/askuser/should_receive_choices_in_user_input_request.yaml | Removes unused duplicate snapshot. |
| test/snapshots/askuser/should_invoke_user_input_handler_when_model_uses_ask_user_tool.yaml | Removes unused duplicate snapshot. |
| test/snapshots/askuser/should_handle_freeform_user_input_response.yaml | Removes unused duplicate snapshot. |
| test/snapshots/ask_user/receive_choices_in_user_input_request.yaml | Removes stale pre-should_-prefix snapshot. |
| test/snapshots/ask_user/invoke_user_input_handler_when_model_uses_ask_user_tool.yaml | Removes stale pre-should_-prefix snapshot. |
| test/snapshots/ask_user/handle_freeform_user_input_response.yaml | Removes stale pre-should_-prefix snapshot. |
Copilot's findings
- Files reviewed: 9/9 changed files
- Comments generated: 0
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.
Fixes #314
test/snapshots/had three sibling folders for the same scenario (ask-user/,ask_user/,askuser/), which caused confusion and snapshot drift across SDKs. This PR consolidates them.What's actually used
All five SDK E2E harnesses already resolve their snapshot path to
ask_user/:sdkTestContext.tsstrips.test.ts/.e2eand replaces-with_, soask_user.e2e.test.ts->ask_userconftest.pystripstest_and_e2e, sotest_ask_user_e2e.py->ask_usertestharness/context.gostrips_test.go/_e2e, soask_user_e2e_test.go->ask_userAskUserE2ETests.cspasses"ask_user"explicitlytests/e2e/ask_user.rspasses"ask_user"explicitlyNo harness has any fallback / alternative-spelling logic, and the replaying CAPI proxy uses the exact path the harness gives it. So
ask-user/andaskuser/were dead snapshots that only existed because earlier captures were regenerated under different folder names.The change
Deletions only, no source/test/harness code changes:
test/snapshots/ask-user/(3should_*.yamlfiles)test/snapshots/askuser/(3should_*.yamlfiles)test/snapshots/ask_user/left over from before PR Fail CI if snapshots aren't present #304 normalized test names to use theshould_prefix:handle_freeform_user_input_response.yamlinvoke_user_input_handler_when_model_uses_ask_user_tool.yamlreceive_choices_in_user_input_request.yamlKept the 3 canonical
ask_user/should_*.yamlfiles that every SDK's tests actually replay against.Validation
All
ask_userE2E suites pass in snapshot-replay mode (GITHUB_ACTIONS=true) against the consolidated folder: Node.js 3/3, Python 3/3, Go 3/3, .NET 3/3, Rust 3/3.Scope
This is scoped to
ask_userper #314. The remainingmcp_and_agents/mcp-and-agentssibling pair is tracked separately as the related issue #315 and is intentionally left alone here.