test: repair pre-existing test failures + stale docstring fix (extracted from #259) - #266
Merged
Merged
Conversation
All 15 failures are stale test fixtures — unrelated to the Windows fixes on this branch. Verified identical failures on `origin/main` via `git stash` baseline A/B. No product bugs discovered; all causes are fixture-maintenance issues: - 11: stale mock target (commit 5b8e995 renamed process_runtime_mentions) - 2: stale prompt assertions (fork-skill load_skill form changed) - 1: unanswered provider-add credential-collision prompt - 1: stale session config dict assertion (agents key now present) Before: 15 failed, 1271 passed After: 1286 passed, 0 failed Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…solver The class docstring claimed a 6-layer resolution strategy copied from StandardModuleSourceResolver's comment, with step 4 listed as a removed legacy module pattern and step 5 skipped entirely, jumping to step 6 (installed package). The actual code here only ever walks 5 steps, and step 4 is the source hint pulled from bundle config -- not a legacy pattern at all. Fixed the docstring to describe what the resolver actually does instead of a stale description carried over from a different resolver's comment.
6885b2c weakened test_subprocess_param_routes_to_subprocess from an exact dict-equality assertion to two looser ones, citing an "agents" key that "now" appeared in the forwarded config. That was a mocking artifact: the fixture's coordinator.config was correctly stubbed as a plain dict even before that commit, but the default fixture value carries no "agents" key, so spawn_sub_session's issue #233 live-registry propagation has nothing to add here. Verified empirically on this branch: with the exact-equality assertion restored, the full test_session_spawner_subprocess.py suite (including the sibling test that exercises a populated live registry) still passes. Restores assert call_kwargs.kwargs["config"] == {"session": {}} and documents why exact-equality is the correct, intentional assertion for this specific (empty-registry) parent fixture. Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Salil Das (sadlilas)
marked this pull request as ready for review
August 18, 2026 01:31
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
3 participants
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.
What this is
Janitorial extraction from #259 (
fix/gap-003-020-023-027-021). Two pre-existing commits, cherry-picked clean ontomain, plus one follow-up fix:6885b2c— test: repair all 15 pre-existing test failures (tests/test_provider_commands.py,tests/test_session_spawner_subprocess.py)30ff56b— fix: correct stale resolution-order docstring inFoundationSettingsResolver(amplifier_app_cli/lib/bundle_loader/resolvers.py, docstring only, no behaviour change)6885b2chad weakened (see below)No product/behaviour changes. This PR can be reviewed and merged completely independently of #259, #263, #264, and #265.
The 15 repairs (per
6885b2c's own message)process_runtime_mentions)load_skillform changed)provider-addcredential-collision promptagentskey)The weakened-assertion question (flagged by reviewer)
6885b2cchangedtest_subprocess_param_routes_to_subprocessfrom an exactassert call_kwargs.kwargs["config"] == {"session": {}}to two looser checks,with a comment blaming a now-present
"agents"key. The reviewer suspectedthis was a mocking artifact (
coordinator.configas an unstubbedMagicMockbeing truthy) rather than real behaviour, and that stubbing it properly would
have kept the strong assertion.
Investigated and confirmed: the fixture (
_make_parent_session) alreadystubs
coordinator.configas a real dict, and with the default (empty)fixture value there is no
"agents"key to produce, sospawn_sub_session's issue-#233 live-registry propagation has nothing tomerge in. Restored the exact-equality assertion in a follow-up commit and
verified the full
test_session_spawner_subprocess.pyfile — including thesibling test that legitimately exercises a populated live registry — still
passes. The strong assertion is correct and ships in this PR.
Verification
git diff origin/main...HEAD --stat— touches only the 3 expected files.uv run pytest -q— full suite green (1301 passed, 1 skipped, 13 deselected, 1 xfailed).uv run ruff checkclean on all touched lines (one pre-existing, unrelatedF821intest_provider_commands.pyat a line this PR does not touch —present on
origin/maintoo).Note: in this environment, running the plain
origin/mainbaseline alreadyshows the full suite green (no repro of the original 15 failures) — most
likely because the failures were specific to the full original branch
context (stacked GAP-003/SIGINT behaviour changes, or an author-machine
environment difference) rather than anything reproducible against a clean
main. The test-file changes here are still correct, minimal, and neteither neutral or strictly better (see assertion-strength fix above) on a
clean
main, so they're included as instructed.🤖 Generated with Amplifier