Skip to content

test(ag2-sparrow): split the claude-code adapter suite so each file runs well under CI's 240 s cap - #5943

Merged
sonichi merged 3 commits into
mainfrom
fix/5940-split-claude-adapter-suite
Oct 27, 2026
Merged

sonichi merged 3 commits into
mainfrom
fix/5940-split-claude-adapter-suite

Conversation

@sonichi

@sonichi sonichi commented Oct 27, 2026 •

Copy link
Copy Markdown
Owner

Fixes #5940

What

tests/ag2-sparrow-claude-code-adapter.test.py timed out at CI's 240 s per-file cap on main. It is now three files that share one fixtures module. Every test is kept, and no production code or default changes.

  • Where the time went. I measured per-test durations at main. About 50 s came from one fixture. The stand-in launcher that the fake manager starts waited out its 5 s admission bound when the test stopped it, because it only read stdin once it held the root. Its stop is now recorded as a signal, which ends the wait in admission at once, as a stop does in production. The rest is the sheer size of the suite (321 tests).

  • The split (packages/ag2-sparrow/tests/):

    File Tests Covers
    test_claude_code_adapter.py 141 the mod, the verifier, the inbox CLI, the launcher, the cards, the pane reads
    test_claude_code_connect.py 99 connect, the agent's root and bridge, verify
    test_claude_code_install.py 81 install, uninstall, hold, the locks, the source pins
    claude_code_fixtures.py — the fake manager, the stand-in launchers, the backend, the base test cases

    The set of test ids is identical before and after (321).

  • CI pickup. Each file has its own tests/ag2-sparrow-claude-code-*.test.py wrapper, which scripts/discover-python-tests.sh finds. Each also has an entry in tests/python-suite-costs.txt; those are estimates until the table is regenerated. The source pins that scan the suite now scan every file of it. Each new file isolates the environment at module top, which hermetic-env-structural requires.

  • FILE_TIMEOUT is unchanged.

Durations

Measured locally under coverage as CI runs it (coverage run --rcfile=.coveragerc, SUTANDO_TEST_SUBPROCESS_COVERAGE=1), at nice -n 19 with env -i. The figure is unittest's own "Ran N tests in" time.

File Before, local Before, CI After, local After, CI
ag2-sparrow-claude-code-adapter.test.py 321 tests, 252.9 s timed out at 240 s on main (run 40425521828) 141 tests, 54.6 s 79 s
ag2-sparrow-claude-code-connect.test.py — — 99 tests, 62.1 s 62 s
ag2-sparrow-claude-code-install.test.py — — 81 tests, 49.5 s 59 s

CI's figures are the per-file times the standalone legs print (run 40432383223).

Without coverage: 207.7 s in one file before, then 156.4 s after the launcher fix alone, then 50.4 / 61.4 / 48.5 s after the split.

Tests

  • The three files pass: 141, 99 and 81 tests, OK.
  • These guards pass: python-ci-legs-partition (every discovered suite runs once across the 9 legs), ci-covers-every-python-test, hermetic-env-structural, events-plane-boundary, test_adapter_boundaries.
  • ruff is clean.

🤖 Generated with Claude Code

sonichi and others added 3 commits October 26, 2026 18:11
…es, and the test launcher leaves admission on its stop

test_claude_code_adapter.py had grown to 321 tests (about 210 s locally) and timed out at
CI's 240 s per-file cap. It is now three files over one fixtures module, every test kept:

- test_claude_code_adapter.py: the mod, the verifier, the inbox CLI, the launcher, the cards
  and the pane reads (141 tests)
- test_claude_code_connect.py: connect, the agent's root and bridge, and verify (99)
- test_claude_code_install.py: install, uninstall, hold, the locks and the source pins (81)
- claude_code_fixtures.py: the fake manager, the stand-in launchers, the backend and the
  base test cases

Each runs through its own tests/ wrapper, which CI discovers, and has an entry in the
suite-cost table. The source pins that scan the suite now scan every file of it.

The stand-in launcher's stop (stdin closed) is now recorded as a signal. That ends its wait
in admission at once, as a stop does in production, instead of after its 5 s bound; this
alone took about 50 s off the suite.

Fixes #5940

Stand: Echo Act IV Pro
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stand: Echo Act IV Pro
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…k cap

Stand: Echo Act IV Pro
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 94%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • packages/ag2-sparrow/tests/claude_code_fixtures.py (96.6%): Missing lines 301,309,361-364,574,676,722-723,725-726,734-735,739,760,847
  • packages/ag2-sparrow/tests/test_claude_code_adapter.py (100%)
  • packages/ag2-sparrow/tests/test_claude_code_connect.py (99.1%): Missing lines 419,469-471,887-888,909-913,1040-1041
  • packages/ag2-sparrow/tests/test_claude_code_install.py (99.8%): Missing lines 1666,1690,1774

Summary

  • Total: 3493 lines
  • Missing: 33 lines
  • Coverage: 99%

packages/ag2-sparrow/tests/claude_code_fixtures.py

Lines 297-305

  297             os.waitpid(pid, os.WNOHANG)     # a reparented, already-dead child may need reaping by us
  298         except ChildProcessError:
  299             pass
  300         time.sleep(0.05)
! 301     return not alive(pid)
  302 
  303 
  304 def oplock_free():
  305     import fcntl

Lines 305-313

  305     import fcntl
  306     fd = os.open(cm.oplock_path(L), os.O_RDWR | os.O_CREAT, 0o600)
  307     try:
  308         fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
! 309         return True
  310     except BlockingIOError:
  311         return False
  312     finally:
  313         os.close(fd)

Lines 357-368

  357     def reap(self):
  358         """The test's own cleanup: every launcher it spawned, and its process group, gone."""
  359         for spawned in self.spawned:
  360             if self.real and spawned.poll() is None:
! 361                 try:
! 362                     os.killpg(spawned.pid, signal.SIGKILL)
! 363                 except ProcessLookupError:
! 364                     pass
  365         self.stop_session()
  366 
  367     def end_session_later(self):
  368         session, self.session = self.session, None

Lines 570-578

  570             out = "".join(words[k] + "\n" for k in cmd[cmd.index("-p") + 1].split(","))
  571         elif verb == "print":
  572             if not self.registered:
  573                 return subprocess.CompletedProcess(cmd, 113, "", 'Could not find service "%s" in domain\n' % cli.BRIDGE.agent_label)
! 574             return subprocess.CompletedProcess(cmd, 0, "\tstate = running\n" if self.active else "\tstate = waiting\n", "")
  575         elif verb in ("enable", "disable"):
  576             self.enabled = verb == "enable"
  577         elif verb in ("restart", "bootstrap"):
  578             self.active = self.registered = True

Lines 672-680

  672         got = predicate()
  673         if got:
  674             return got
  675         time.sleep(step)
! 676     return predicate()
  677 
  678 
  679 CONTRACT_TS = MOD / "tests" / "cli-contract.ts"

Lines 718-730

  718 def fd_path(fd):
  719     """The path an open fd names (macOS F_GETPATH, Linux /proc), or '' when it cannot be read."""
  720     try:
  721         if sys.platform == "darwin":
! 722             import fcntl
! 723             return os.path.realpath(fcntl.fcntl(fd, 50, b"\0" * 1024).split(b"\0", 1)[0].decode())
  724         return os.path.realpath(os.readlink("/proc/self/fd/%d" % fd))
! 725     except OSError:
! 726         return ""
  727 
  728 
  729 def complete_safe(path):
  730     """Whether the ledger still opens (a corrupted one is reset so later property cases stay independent)."""

Lines 730-743

  730     """Whether the ledger still opens (a corrupted one is reset so later property cases stay independent)."""
  731     try:
  732         inbox.lifecycle.SettledIds(path)
  733         return True
! 734     except inbox.lifecycle.LedgerError:
! 735         return False
  736 
  737 
  738 def real_ids(path):
! 739     return [line for line in Path(path).read_text(errors="replace").splitlines()
  740             if inbox.lifecycle.local_task_protocol.valid_task_id(line)]
  741 
  742 
  743 CANARY = ("AG2_TEST_CANARY", "canary-must-not-reach-the-server")

Lines 756-764

  756 def tree_bytes(root):
  757     """Every file under `root` (lock files aside) with its bytes: the store compared before and after."""
  758     root = Path(root)
  759     if not root.exists():
! 760         return {}
  761     return {str(p.relative_to(root)): p.read_bytes() for p in sorted(root.rglob("*")) if p.is_file() and p.name != ".lock"}
  762 
  763 
  764 class CardBase(ConnectBase):

Lines 843-851

  843     def has_session(self, name=None):
  844         return self.present
  845 
  846     def capture(self, pane_id):
! 847         return None
  848 
  849 
  850 # The pane of a fresh install, as the live witness captured it from Claude Code 2.1.303.
  851 TRUST_PANE = """────────────────────────────────────────────────────────────────────────────────

packages/ag2-sparrow/tests/test_claude_code_connect.py

Lines 415-423

  415                                  stdin=subprocess.PIPE, stdout=subprocess.PIPE, text=True)
  416             successor.append(p)
  417             self.assertEqual(p.stdout.readline().strip(), "held")
  418             if select.select([p.stdout], [], [], 3)[0]:     # the older writer's seat lock holds it off
! 419                 early.append(p.stdout.readline().strip())
  420             return real(owner, root)
  421         try:
  422             with mock.patch.object(cm, "root_is_held", side_effect=checked):
  423                 rc, out, err = self.connect(Backend())

Lines 465-475

  465         t = threading.Thread(target=lambda: got.append(self.connect(Backend())), daemon=True)
  466         t.start()
  467         t.join(10)
  468         if t.is_alive():
! 469             os.close(os.open(token, os.O_WRONLY))     # end the read connect is stuck in
! 470             t.join(30)
! 471             self.fail("connect hung reading the FIFO seat token")
  472         rc, out, err = got[0]
  473         self.assertEqual(rc, cli.EXIT_UNUSABLE, out + err)
  474         self.assertNotIn("connected:", out)
  475         self.assertIn("token is not a regular file", err)

Lines 883-892

  883             self.assertEqual(cm.start_supervisor(L, restart=False), "")
  884             self.assertIn("launchctl bootstrap failed", cm.start_supervisor(L))
  885 
  886     def _main_with(self, fake, *argv):
! 887         with mock.patch.object(sm, "_run", side_effect=fake):
! 888             return self.main(*argv)
  889 
  890     def launch_with(self, shape, gap=None):
  891         """The production launcher sequence on the root, `gap()` run just before it takes the root."""
  892         real, seen = task_io.acquire_lock, []

Lines 905-917

  905     def other_thread_takes_the_agent_lock(self):
  906         got = []
  907 
  908         def take():
! 909             try:
! 910                 with cm.agent_lock(str(self.root), wait_s=0.3):
! 911                     got.append("taken")
! 912             except cm.Busy:
! 913                 got.append("busy")
  914         t = threading.Thread(target=take)
  915         t.start()
  916         t.join(10)
  917         return got[0]

Lines 1036-1045

  1036                     time.sleep(0.1)
  1037                 acted, held = Path(r.acted).exists(), task_io.lock_held(str(self.root))
  1038                 for launcher in r.spawned:                  # cleanup, so one shape cannot leak into the next
  1039                     if launcher.poll() is None:
! 1040                         os.killpg(launcher.pid, signal.SIGKILL)
! 1041                         launcher.wait(timeout=30)
  1042                 self.assertFalse(acted, "the late launcher never held or acted on the root")
  1043                 self.assertEqual(held, "free")
  1044                 self.assertNotEqual(rc, 0, out + err)
  1045                 self.assertNotIn("next:", out)

packages/ag2-sparrow/tests/test_claude_code_install.py

Lines 1662-1670

  1662             self.assertIsNone(self.session(), "claude never started")
  1663             self.assertEqual(task_io.occupancy(str(self.work)), "free", "the dead owner with no consumer frees the root")
  1664         finally:
  1665             if proc.poll() is None:
! 1666                 os.kill(proc.pid, signal.SIGKILL)
  1667             proc.communicate()
  1668 
  1669     def test_kill_9_hold_after_the_record_and_before_the_go_byte_leaves_a_consumer_that_never_started(self):
  1670         from ag2_sparrow import task_io

Lines 1686-1694

  1686             self.assertEqual(wait_for(lambda: task_io.occupancy(str(self.work)) == "free" and "free", 10), "free",
  1687                              "a verified-dead consumer frees the root; while it lived the root read held")
  1688         finally:
  1689             if proc.poll() is None:
! 1690                 os.kill(proc.pid, signal.SIGKILL)
  1691             proc.communicate()
  1692 
  1693     def test_a_failed_attach_reaps_the_shim_and_hold_exits_without_a_consumer(self):
  1694         from ag2_sparrow import task_io

Lines 1770-1778

  1770             loop.wait(timeout=30)
  1771             loop.stdin.close()
  1772             loop.stdout.close()
  1773             if proc.poll() is None:
! 1774                 os.kill(proc.pid, signal.SIGKILL)
  1775             proc.communicate()
  1776 
  1777     def test_hold_recovers_the_dead_generations_claims_under_its_flock_and_leaves_a_live_one(self):
  1778         """Recovery runs after acquire and publish, before the spawn; only claims of a

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified at head 48ffcda8f; not approving yet, because nothing else has approved at this head (the bar here is one expert approval plus verification, or three known approvals).

What I checked:

  • Test-only. The diff touches packages/ag2-sparrow/tests/ (three suite files plus claude_code_fixtures.py), two new tests/ag2-sparrow-claude-code-*.test.py wrappers, tests/python-suite-costs.txt, and one paragraph of the adapter README. Nothing under ag2_sparrow/ or src/ changes.
  • The 321 test ids are kept. I parsed main's single test_claude_code_adapter.py (at 32093d40b) and the three new files plus fixtures with ast. Both sides have 321 Class.test_* ids, the same set, with none missing, added or duplicated. All 321 test bodies match main exactly except two ConnectSourceRules source pins. In those two, Path(__file__) became *SUITE_FILES, the glob of every test_claude_code_*.py plus the fixtures. That widens what they scan, and their assertions are unchanged. The Runner, the helpers and the non-test methods all match main.
  • The stand-in launcher. Only the LAUNCHED test double changes. Its stdin EOF now sets state['sig'] on a thread, and it passes state to the real cm.launch_admission(root, state, wait_s=5). That is the production parameter: connect_common.py:839 returns at once when state["sig"] is set, which is how a SIGTERM reaches it through stop_handlers. The dedicated stop test, test_a_stop_reaches_a_launcher_waiting_in_admission_at_once, runs the real hold_and_supervise through REAL_LAUNCHER, which this PR does not change. So the stop and admission behaviour is still tested against the real stop path, and neither test is vacuous.
  • Discovery. scripts/discover-python-tests.sh lists all three wrappers.
  • Local run at the head (python3.13, env -i, scratch HOME, no coverage). connect: 99 tests OK in 60.7 s. install: 81 tests OK in 49.4 s. adapter: 129 of 141 pass in 32.5 s. The 12 failures are all LauncherTmux, which I ran with tmux deliberately off PATH (FileNotFoundError: 'tmux'), so this run cannot see that class. CI runs it, and it is green there.
  • CI and threads. All 9 standalone shards and every other check pass. license/cla is success (1 context). There are no unresolved threads.

Missing: an approval from someone else at this head. With one, this is ready to approve.

— Sutando (susans-MacBook-Pro)

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at 48ffcda (agent-assisted review by Mars, Qingyun's Sutando seat). This is test-only: no production code changes, and the 321 Class.method ids are identical on main and across the three new files, with no duplicates.

The #5871 B1 guard still holds. Change (a) touches only the simple LAUNCHED stand-in (claude_code_fixtures.py:194-205). It sets state['sig'] = 15 and passes state into the real cm.launch_admission, so production is what leaves admission fast; the stand-in doesn't exit by itself. REAL_LAUNCHER (:208-232), which the B1 tests use, is byte-identical to main. Mutation check at this head:

  • Removing the state.get("sig") early return in launch_admission (connect_common.py:839-840) makes test_a_stop_reaches_a_launcher_waiting_in_admission_at_once fail in both shapes ("10.0 not less than 8").
  • Removing the post-acquire_lock sig check before prepare (:1387-1389) makes test_a_launcher_held_past_the_operations_wait_never_acts_on_the_root fail in both shapes.

Isolation: isolate_os_environ() runs once at fixtures import. Temp homes are per test (mkdtemp plus addCleanup), the module-level dicts are never mutated, and each file runs in its own process through its runpy wrapper. The codex suite is untouched. CI is green at 79 s, 62 s and 59 s against the 240 s cap, and python-suite-costs.txt is updated for sharding.

Nit: re-wrap the edited README line (adapters/claude-code/README.md:661-662), which is now longer than the lines around it.

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.

CI: tests/ag2-sparrow-claude-code-adapter.test.py times out at its 240 s limit on main

3 participants