Skip to content

tests/test_adopt_plan_value_order.py: a bad id: or org: beside a --work-dir that is a file or a shared one, or beside a gone source, is the value's refusal in both check and execute, so the head ask is pinned against _work_root's other refusals and the source (#1179) - #1226

Draft
brettheap wants to merge 1 commit into
fix/1020-bad-id-first-problemfrom
fix/1179-value-order-mutant-rows

Conversation

@brettheap

@brettheap brettheap commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Test only: six rows in tests/test_adopt_plan_value_order.py that kill four surviving mutants (L1, L2, L4 and L11) of #1125's head ask. No change to adopt-project.py (its sha256 is 412edbda on #1125's head and on this one).

This PR lands after #1125 and stays a draft until then. Its base is fix/1020-bad-id-first-problem (#1125's head 9549006fe), not main, so the diff is this one commit. #1125 is a draft against main and has not landed. Once #1125 has landed, this branch is brought up to date with main and gated again on that head before it lands.

Closes #1179

What the rows pin

RV1125b's NIT-A on #1125 (for #1020): check and execute ask plan.names() and _checked_org_and_id(plan) right after the plan's keys and before _work_root. One row pinned that position against the work directory, and it used a directory made unwritable with chmod. _work_root refuses two more things before it makes anything, and both commands refuse a gone source, and no row put a bad id: or org: beside any of them.

Three tests, each run for id and for org, so six rows. Each is check and then execute --yes on one plan, in the shape of test_the_org_or_the_id_is_named_before_an_unwritable_work_dir:

Test The control: the plan unedited, beside the same file, directory or path With a bad id: or org: added
test_the_org_or_the_id_is_named_before_a_work_dir_that_is_a_file a --work-dir that is a file: both commands exit 2 on REFUSED adopt-work-dir-unwritable both exit 2 on the value's refusal, the same first line in each, and none of the three work-directory or source codes on either stderr
test_the_org_or_the_id_is_named_before_a_shared_work_dir (WINDOWS_SKIP) a --work-dir every user can write in, no sticky bit: adopt-work-dir-shared the same
test_the_org_or_the_id_is_named_before_a_source_that_is_gone a source.local_path that names nothing: plan-source-gone the same

The shared row is in because RV1125b's NIT-A names "a --work-dir that is a file or a shared one", and #1179 asked for it unless the lane ruled otherwise.

Mutants

Each mutant was made from #1179's text in the writer's own clone, run against the 85 old rows (the two order files of #1125, tests/test_adopt_plan_value_order.py and tests/test_adopt_check_plan_name.py) and against the six new ones, and adopt-project.py was restored after each with its sha256 checked (412edbda, the same after each one).

Mutant The 85 old rows The six new rows
L1: check asks plan.names() and _checked_org_and_id after _work_root 85 passed (survives) 4 failed: the file and shared rows, for id and for org
L2: check asks them after the source is opened 85 passed (survives) 6 failed: all of them
L4: execute asks them after _work_root 85 passed (survives) 4 failed: the file and shared rows
L11: check asks plan.names() at its head and _checked_org_and_id after _work_root 85 passed (survives) 4 failed: the file and shared rows

The plan-source-gone rows fail only on L2, as #1179's table says. On the head the six pass alone (6 passed, 40 deselected in 8.49s), the two order files together give 91 passed, and tests/test_adopt_plan_value_order.py alone is 46 rows, the 40 it had and the six.

Gates

One pytest run over 39 files each time: every file that greps for the codes and functions #1125 touches, the files this test file imports from, tests/test_repo_hygiene.py, tests/test_cli_reference.py and tests/test_agents_md_codes.py. The environment is the landing gate's own: a stub gh first on PATH, GH_TOKEN=bogus, an empty GH_CONFIG_DIR and an empty gitconfig, GIT_CONFIG_NOSYSTEM=1 and EMAIL set, and a wait for at least 17G free before each run.

Tree Result
The head, f61be9f59 3130 passed, 1 skipped in 41:00, exit 0
The scratch merge 1c1e6653c of 961640c68 (origin/main when it was gated) and the head, not pushed 3180 passed, 4 skipped in 40:15, exit 0
  • The one skip on the head is tests/test_adopt_plan.py:567, which needs SHAPE_MEDXEHR_CLONE, a read-only clone of MedxSoft/MedxEHR.
  • The 3 extra skips on the merge are main's new reftable rows in tests/test_adopt_root_symlinks.py, at lines 1926, 1981 and 2532. They need git 2.45 or later, and the bench has git 2.43.0.
  • The 50 extra passes are main's new rows.

origin/main has moved since: it is now 50ce8ab15, and #1119 (b7edd8816) changed adopt-project.py in check. Since 961640c68 main has changed adopt-project.py and tests/test_adopt_pins.py, two of the five files #1125 changes, and it has not changed this PR's file. The 39-file gate was not run again on it. The two order files, the 91 rows, were run on a scratch merge of 50ce8ab15 and the head (b0e2e0461, not pushed): 91 passed in 172.19s, exit 0.

Merge-trees (all clean, exit 0)

Against Tree
origin/main (50ce8ab15) and this head bede214aa0d551c1c0c2ce81ff4ce1452983d45c
#1125's head 9549006fe and this head 8c604453d4766e3eed40e34874f4a6acaaf0039d
#1198's head d38e4e36d and this head a1504e06d18f07f1f3c02d081ee6067159459180
origin/main and #1125's head (context) ad61940f1a55897592a519dee59b5f31f53c7a9a
#1198's head and #1125's head (context) 88530bb7b3d29bfb11c5bb8e1f28e0ab194261ae

Of the 45 open PRs, only #1125 and #1198 touch tests/test_adopt_plan_value_order.py.

See #1020, #1113 and #1125.

🤖 Generated with Claude Code
Lane: openreposhape-4 (openRepoShape-4)

Summary by Sourcery

Strengthen adoption plan value-order regression coverage for invalid IDs and organizations across validation and execution.

Enhancements:

  • Expand adoption plan ordering coverage to ensure invalid organization and ID values take precedence over additional work-directory and missing-source refusals in both validation and execution.

Tests:

  • Add parameterized tests covering file-backed work directories, shared work directories, and missing sources for both invalid IDs and organizations.

…`--work-dir` that is a file or a shared one, or beside a gone source, is the value's refusal in both `check` and `execute`, so the head ask is pinned against `_work_root`'s other refusals and the source (#1179)

Six rows, with the plan unedited beside the same file, directory or path as the control. Test only: no change to adopt-project.py. Stacked on the head of #1125 (9549006).

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap brettheap added this to the v1.2.1 milestone Oct 23, 2026
@sourcery-ai

sourcery-ai Bot commented Oct 23, 2026

Copy link
Copy Markdown

Reviewer's Guide

Adds six parameterized tests that pin id:/org: validation ahead of file, shared, or missing-source refusals in both adoption commands, with shared helpers asserting exact refusal ordering and no competing diagnostics; production code is unchanged.

File-Level Changes

Change Details Files
Added parameterized regression coverage proving bad plan values are validated before additional work-directory and source refusals.
  • Cover bad id: and org: values beside a work-directory path that is a file.
  • Cover bad values beside a shared work directory, skipped on Windows.
  • Cover bad values beside a missing local source.
  • Run and compare both check and execute --yes, including refusal ordering, exit status, and absence of unrelated refusal codes.
  • Factor shared assertions and test setup for the six new cases.
tests/test_adopt_plan_value_order.py
Expanded test dependencies and documentation to describe the required validation ordering.
  • Import filesystem/platform helpers and shared fixtures/utilities for path editing and platform skips.
  • Document the ordering guarantees covered by the new rows and the refusal codes that must not appear.
tests/test_adopt_plan_value_order.py

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sonarqubecloud

Copy link
Copy Markdown

@brettheap

Copy link
Copy Markdown
Contributor Author

RV1226: independent review of #1226 at f61be9f5: READY

Independent review for lane openRepoShape-4 of #1226 (for #1179), stacked on #1125 (9549006fe). I did not write this code. No MUST and no SHOULD; two NITs, both follow-ups.

What I measured

Mutants

Each was applied to adopt-project.py in my own worktree at the PR head and restored after each run. The sha256 was 412edbda after every one. L1, L2, L4 and L11 follow #1179's text and ran against the two order files (91 rows). The others are my own and ran against the six new rows.

Mutant The 85 old rows The six new rows
none (head) 85 passed 6 passed (91 passed together, 205 s)
L1: check asks plan.names() and _checked_org_and_id after _work_root 85 passed (survives) 4 failed: file and shared, id and org
L2: check asks them after the source is opened 85 passed (survives) 6 failed
L4: execute asks them after _work_root 85 passed (survives) 4 failed: file and shared
L11: check asks plan.names() at its head, _checked_org_and_id after _work_root 85 passed (survives) 4 failed: file and shared
L5: execute asks them after _refuse_an_unwritable_work_dir not run 4 failed: file and shared
L6: execute asks them after the source is opened not run 6 failed
L8: BOTH ask them after _work_root (both commands agree, so RV1125b's probe let it live) not run 4 failed: file and shared, at startswith(refused)
L14: BOTH ask them after the source is opened not run 6 failed
C1 (control): _work_root no longer refuses a shared --work-dir not run 2 failed: the shared rows' control (check exit 0; execute ran and made remotes)
C2 (control, code): a gone source is refused as plan-source-vanished not run 2 failed: the gone rows' control line
R1 (code): execute alone refuses a bad id or org as plan-bad-value not run 6 failed at named_first(executed) == named_first(checked)
R2 (order, stderr): execute asks _work_root first and prints its refusal as a NOTE line, then asks the values not run 4 failed, at the NOT_SAID_BESIDE_A_VALUE loop alone: the first line was still the value's
R3 (exit code): execute exits 1 on a bad id or org not run 6 failed at executed.returncode == 2

L8 is new here: the 85 old rows and RV1125b's agree-only probe both let it live, and the file and shared rows kill it.

Gate on a scratch merge of current main

Scratch merge b079615ea of origin/main 50ce8ab15 and f61be9f59 (tree bede214aa, not pushed). Against main it differs only in #1125's five files, this file among them.

The environment was the landing gate's:

  • a stub gh first on PATH, GH_TOKEN=bogus and an empty GH_CONFIG_DIR;
  • an empty GIT_CONFIG_GLOBAL and GIT_CONFIG_NOSYSTEM=1;
  • GIT_ALLOW_PROTOCOL and GIT_SSH_COMMAND unset, and EMAIL set;
  • -p no:cacheprovider -o tmp_path_retention_policy=failed;
  • each run waited for at least 12G and 2M inodes free.
Run Files Result
G1 test_adopt_plan_value_order.py, test_adopt_check_plan_name.py, and the two modules the file imports from, test_adopt_root_symlinks.py and test_adopt_template_values.py, in one run 375 passed, 3 skipped in 816.56s, exit 0. The 3 skips are main's reftable rows in test_adopt_root_symlinks.py, which need git 2.45; the bench has git 2.43.0
G2 test_adopt_execute_refusals.py, test_adopt_pins.py, test_adopt_work_dir_refusal.py, test_adopt_shape_pin_and_work_dir.py, tests/test_repo_hygiene.py 507 passed in 786.55s, exit 0

Merge-trees (git merge-tree --write-tree, all exit 0)

Against Tree
origin/main 50ce8ab15 and f61be9f5 bede214aa0d5, the same as the body's
#1125's head 9549006fe and f61be9f5 8c604453d476, this head's own tree
#1198's head d38e4e36d and f61be9f5 a1504e06d18f, the same as the body's
my scratch merge b079615ea and #1198's head fb54228545ca, clean

Of the open PRs, only #1125, #1198 and this one touch tests/test_adopt_plan_value_order.py.

Findings

NIT-1: a second sys.path insert for an import that is already loaded. The new lines add sys.path.insert(0, str(REPO / "scripts")) and from repo_shape import double_quoted. But test_adopt_template_values, imported on the line above, already does both at its import (its lines 57 and 59). So scripts/ lands on sys.path twice.
Fix: from test_adopt_template_values import double_quoted, with_line. Then drop the sys import, the REPO import and the insert.

NIT-2: the module docstring undercounts _work_root. It says "_work_root refuses two more things before it makes anything". _work_root also refuses:

  • a symlink loop;
  • a symlink to something that is no directory;
  • a directory another user owns;
  • a shared directory above the --work-dir.

The rows rightly cover two of them.
Fix: "refuses more before it makes anything, among them a --work-dir that is a file and one every user can write in". Or leave it.

Landing note, not a finding: GitHub reads the body's closing line only once the PR is retargeted to main after #1125 lands. closingIssuesReferences is empty now. Check that it reads #1179 before landing.

Lane: openreposhape-4 (openRepoShape-4)

This branch has not been deployed

No deployments
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