https://docs.all-hands.dev/ logo
Join Slack
Powered by
Just a heads up, I am experimenting with stryker m...
# proj-agent-canvas
d
Just a heads up, I am experimenting with stryker mutation testing stryker-mutator.io . This has resulted in a lot of PRs being opened! I am adding coverage to get closer to 100% coverage while leveraging mutation testing to boost the strength/effectiveness of the added tests
🙌 1
tracker for the PRs to improve test coverage github.com/OpenHands/agent-canvas/issues/1746
here is the PR that adds stryker setup to the project github.com/OpenHands/agent-canvas/pull/1688
h
Neat!
ty 1
d
in a world with agents, we can totally get better test coverage > 90%
🙌 2
l
yeah, mutation tests are a lot of fun!
🧪 2
ship it parrot 1
d
I have completed the run of adding mutation tested test coverage to agent canvas. We can get a lot closer to 90% with some confidence in those tests being effective 🙂 Ideally we can get the stryker setup PR reviewed github.com/OpenHands/agent-canvas/pull/1688 and merged first. Then I can open up the rest of the PRs for review
b
🐾 Picked up the mutation-testing PR that got stranded when this repo moved. The original — agent-canvas#1688 — was closed with "please re-open on OpenHands/OpenHands," so I rebuilt it there: OpenHands/OpenHands#16184 — test: add Stryker mutation testing @dry-book-58714's 3 commits preserved verbatim (authored 
ai.vong@openhands.dev
), rebased on current 
main
. Only 
package-lock.json
was regenerated to match. GitHub says MERGEABLE — no conflicts. I reviewed and ran it end-to-end, not just eyeballed it: • 
npm ci
clean with the regenerated lockfile (so package.json ↔️ lockfile agree) • the setup/diff unit tests: 15/15 pass • a real Stryker run on `src/utils/mcp-installed-server-display.ts`: 23 mutants killed, 100% covered-mutation score, HTML report generated — the harness genuinely mutates, runs Vitest, and reports It's report-only, dev-deps only, not wired into CI (matches what you two landed on — run on demand now, maybe a weekly automation later). @calm-artist-7206 from my side it looks fine to merge. One open call for you both: the base #1688 had no CI wiring and adds ~1000 lockfile lines — same tradeoff you discussed. If you're happy running it on demand for now, it's good to go. 🐾
d
ty
c
smolpaws please update from main and fix PR description CI failure
b
🐾 Done, @calm-artist-7206: Updated from main — rebased #16184 onto latest 
main
(picked up #16183 telemetry-banner fix). Ai Vong's 3 commits still intact and correctly authored; 
npm ci
still clean after the rebase. PR description CI — fixed. It was the "Validate PR description" check: my body was missing the template's 
HUMAN:
first line, the 
AGENT:
marker, and the 
## How to Test
section. Rewrote it to the repo template; that check is now green. One honest heads-up on the remaining red: mock-llm-e2e fails with exit 124 — a 600s suite timeout, hanging on the 
mock-llm-model-switch
/ 
profile-management
specs. This PR touches only 
stryker.config.mjs
, a script, docs, 
.gitignore
, and dev-deps — zero 
src/
runtime and zero 
tests/e2e/
code, so it can't be causing a profile-switch e2e hang. Looks like the same flaky e2e timeout. Everything else (test-and-build ubuntu + windows, validate-description, pr-title, artifacts) is green. Happy to hit re-run on that job if you want a clean board before merge. 🐾
🐾 Pushed an empty commit to retrigger (thanks for the OK — I don't have re-run rights on the repo). Everything green again except Mock-LLM E2E Tests, which timed out at 600s a second time: "40 passed (10.0m)" then it runs out of wall-clock. Two identical timeouts made me stop calling it "flaky" and actually check. It's not this PR — it's repo-wide right now. The non-Docker "Mock-LLM E2E Tests" job is currently failing on a pile of unrelated branches, including: • 
revive-local-telemetry-consent-banner
(that's #16183 — the commit that just merged to 
main
) • 
feat/chat-markdown-artifact-preview
• 
fix/vscode-button-renders-for-local-backends
• 
set-glm-52-canvas-default
…and more. The Docker Mock-LLM E2E variant passes fine on my branch every time; only the plain one times out. Looks like that job is over its 600s budget generally (it reports 40 passed and just doesn't finish the suite in time), not a real failure in anyone's code. So #16184 is genuinely ready from its own side: rebased on main, Ai Vong's 3 commits intact, CI + build + description-check all green, MERGEABLE. The one red check is an infra timeout hitting everyone. I'd say merge when you're comfortable, and separately that e2e budget/split probably wants a look. 🐾
c
@bland-bear-604 make also an issue to remind us: more packages means a higher chance of supply chain issues; we can make an automation on cloud or a confined vm to check and/or bound the risk, and have it install and run it weekly or on release; ot should propose PRs with test fixes
b
🐾 Filed it: #16195 — Run mutation testing in a confined automation to bound supply-chain risk. It captures your reasoning: more packages = bigger supply-chain surface, so instead of every contributor (or the agent) installing the Stryker toolchain in the normal path, run it in a confined ephemeral environment — Cloud automation or a sandboxed VM — that installs, runs, publishes a report, and tears down. Scheduled weekly and/or on release, and it opens PRs proposing test-strengthening fixes for actionable survivors. Acceptance checklist covers picking the runner, cadence, and how transitive deps get pinned/reviewed and kept out of the default install path. Linked to #16184 and the #15471 tracker. 🐾
3 Views
Open in Slack
PreviousNext
Powered by