test(integration): fix 3 semantic-merge test breakages in merge queue#1834
Merged
Conversation
Three independent stale-test failures surfaced in the merge queue once recently-merged PRs combined on main: - test_wave2_adapters_coverage: patched GeminiClientAdapter._get_gemini_dir, which #1770 renamed to _get_config_dir. Update the patch target. - test_wave7 test_unmanaged_file_deny: asserted exactly one detail line, but #1793 appends a single trailing next-action hint to any non-empty unmanaged-files report. Assert on the flagged-file line instead of the raw detail count. - test_commands_deep_coverage (outdated/compile): these tests passed obj={"cwd": ...}, which the commands never read -- they resolve the project via Path.cwd(). They only passed because pytest's cwd was the repo root (a valid APM project); under CI xdist a sibling test leaking os.chdir flipped their exit codes. Pin cwd with monkeypatch.chdir and assert the commands' real behavior (no-lockfile and no-content exit 1). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes stale integration tests that broke in the merge queue due to upstream behavior changes landing in separate PRs, bringing the integration suite back to green without changing src/ behavior.
Changes:
- Update a Gemini adapter test to patch the renamed helper (
_get_gemini_dir->_get_config_dir). - Make unmanaged-files policy test resilient to an added trailing “next action” hint by asserting on the per-file detail line.
- Make
outdated/compiledeep-coverage tests deterministic under xdist by pinning process CWD viamonkeypatch.chdir(...)and aligning assertions with actual command behavior.
Show a summary per file
| File | Description |
|---|---|
tests/integration/test_wave7_policy_registry_coverage.py |
Avoids brittle details count assertion; checks the unmanaged-file detail line content instead. |
tests/integration/test_wave2_adapters_coverage.py |
Patches the correct renamed method on GeminiClientAdapter to prevent AttributeError. |
tests/integration/test_commands_deep_coverage.py |
Pins Path.cwd()-based project resolution via monkeypatch.chdir and updates exit-code/output expectations to match real command behavior. |
Copilot's findings
- Files reviewed: 3/3 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.
TL;DR
Six integration tests started failing in the merge queue (run 27680546218) across shards 1 and 4. None are flaky in the usual sense — they are stale tests broken by semantic merge conflicts: three recently-merged PRs each passed in isolation, but their combination on
mainleft assertions/patches pointing at old behavior. This PR fixes all six.Root causes & fixes
1.
AttributeError: ... does not have the attribute '_get_gemini_dir'tests/integration/test_wave2_adapters_coverage.pyPR #1770 renamed
GeminiClientAdapter._get_gemini_dir()→_get_config_dir(). PR #1788 added a test that still patches the old name. Updated thepatch.objecttarget.2.
assert 2 == 1on unmanaged-files detailstests/integration/test_wave7_policy_registry_coverage.pyPR #1793 now appends a single trailing next-action hint (
"Next: run 'apm install <ref>' ...") to any non-empty unmanaged-files report.test_unmanaged_file_denyasserted exactly one detail line. Changed it to assert on the flagged-file line itself ("not tracked in apm.lock.yaml"+stray.md) rather than the raw count, so it stays correct regardless of the hint.3. Four
SystemExit(1)failures inoutdated/compiletests/integration/test_commands_deep_coverage.pyThese tests passed
obj={"cwd": ...}— but no command ever reads that key (verified: zeroobj["cwd"]reads insrc/).outdatedandcompileresolve the project viaPath.cwd()(get_apm_dir(PROJECT)returnsPath.cwd()). The tests only passed because pytest's process CWD was the repo root (a valid APM project), never the fixture dir. Under CI's parallel xdist (-n 2 --dist loadgroup), a sibling test leakingos.chdirpolluted the shared worker CWD and flipped these exit codes.Fix: pin CWD deterministically with
monkeypatch.chdir(fixture)(auto-restoring, so the tests can no longer be polluted or pollute others), and correct three assertions that never actually matched the commands' real behavior:test_outdated_no_lockfileNo lockfile foundtest_outdated_no_dependenciesNo locked dependencies(now deterministic)test_compile_no_apm_ymlNot an APM projecttest_compile_no_agents_mdNo APM content foundValidation
-n 2 --dist loadgroup.os.chdir(the exact CI failure mode).ruff check+ruff format --checkclean on all edited files.src/changes — test-only fix matching real, intended command behavior.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com