You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "Astro-Han (via GitHub)" <gi...@apache.org> on 2026/08/31 07:48:44 UTC
[PR] refactor: collapse duplicate test infrastructure [maka]
Astro-Han opened a new pull request, #4323:
URL: https://github.com/apache/maka/pull/4323
## Summary
Three duplicate authorities in the test infrastructure, each a single fact represented more than once.
**Test execution entry points.** Every workspace carried a `test` script that was `clean && build && test:dist`, but nothing consumed it — the root `npm test` runs `build:test` and drives each workspace's `test:dist` through `run-workspace-tests-parallel`, and CI takes the same path. The nine copies had already drifted: `@maka/mcp` inlined `node --test` instead of delegating, and `@maka/desktop`'s `test` covered strictly fewer files than its own `test:dist`. Separately, `@maka/eval` listed its seven Python suites by hand and chained them with `&&`, so a failure in the first hid the other six and every new suite needed a package.json edit to run at all; `unittest discover` is the standard-library equivalent. Test execution now has one entry point per workspace.
**The `expect` shim.** `packages/core` and `packages/runtime` each carried a copy of a Jest-style `expect` built on `node:assert/strict`, used by 31 files across 1777 call sites. Seven of its ten methods forwarded straight through with no added semantics. The remaining three borrowed Jest names without Jest semantics — most consequentially `toContain`, which ran `String(actual).includes(expected)`, so 13 call sites whose subject was an array were matching against a comma-joined string rather than testing element membership. `['run_created_extra']` would have satisfied `toContain('run_created')`. The repository now has one assertion API, `node:assert`.
The three commits are separable on purpose: the mechanical rewrite is behaviour-preserving, and the semantic correction that follows it is the only commit that could change a test's colour.
## Verification
- `npm --workspace @maka/core run test:dist` — 731 passing, unchanged across all three commits
- `npm --workspace @maka/runtime run test:dist` — 3089 passing / 13 skipped, unchanged across all three commits
- `python3 -m unittest discover --start-directory harbor --pattern 'test_*.py'` — 81 tests, 12 skipped, replacing the seven hand-listed invocations
- `npm run format:check` — clean
- `tsc` via each workspace build — clean
Not run locally: the full repository suite, desktop e2e, and Storybook. CI covers them.
The semantic correction was expected to turn some tests red and did not. The assertions are stronger than they were, but nothing in the current code was relying on the weaker reading — worth a reviewer's attention, since it means the fix is preventive rather than a bug fix.
## Remaining work
This is a draft because one commit from the intended scope is still outstanding:
- [ ] Collapse the duplicated test concurrency primitives (`deferred`, `withTimeout`, `nextId`) — roughly 64 same-named definitions of `deferred` alone across `packages/runtime` and `packages/runtime-host`, in 19 signature variants. It belongs in this PR because it touches the same test files, but it needs one decision the first three commits did not: a single signature wide enough to cover every variant.
## AI use
Select exactly one:
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope: Claude Code performed the audit that found these duplicates, wrote and ran the two AST codemods behind the rewrite, and drafted the commit messages and this description. The affected commits carry `Generated-by: Claude Code` trailers. A human contributor reviews the final diff and owns the merge decision.
## Checklist
- [x] Tests cover the change and fail without it
- [x] Lint, format, typecheck and the affected suites pass locally
Does this PR entail a change in behavior?
- [x] Yes — described under Summary above
- [ ] No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor: collapse duplicate test infrastructure [maka]
Posted by "M4n5ter (via GitHub)" <gi...@apache.org>.
M4n5ter commented on code in PR #4323:
URL: https://github.com/apache/maka/pull/4323#discussion_r3894379543
##########
apps/desktop/package.json:
##########
@@ -34,8 +34,6 @@
"package:windows-x64": "electron-builder --config electron-builder.config.mjs --win --x64 --publish never",
"typecheck": "tsc -p tsconfig.preload.json --noEmit && tsc -p tsconfig.main.json --noEmit && tsc -p tsconfig.renderer.json --noEmit && tsc -p tsconfig.storybook.json --noEmit",
"typecheck:stories": "tsc -p tsconfig.storybook.json --noEmit",
- "pretest": "npm run build:workspace-deps",
- "test": "npm run clean:main && npm run build:main && node --test \"dist/main/**/*.test.js\"",
Review Comment:
**P2 — Active per-workspace test instructions are now broken.**
This deletion removes the `test` script, but these still-active instructions invoke it:
- `apps/desktop/tests/smoke.md:29`: `npm --workspace @maka/desktop test`
- `packages/computer-use/README.md:116`: `npm --workspace @maka/computer-use test`
- the three Computer Use test-file comments in `packages/computer-use/src/__tests__/`
- runtime test comments in `packages/runtime/src/__tests__/history-compact-summarizer.test.ts`, `prompt-composition.test.ts`, and `session-trace-projection.test.ts`
Following the desktop command on this head fails immediately with `Missing script: "test"`. Please update these references to the canonical `test:dist` flow, including the required build step, or retain compatibility aliases. Otherwise a documented package-level verification path is unusable.
> **Automated review notice:** This comment was posted by an automated review agent operated by M4n5ter. It is not an independent human review and does not replace one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor: collapse duplicate test infrastructure [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4323:
URL: https://github.com/apache/maka/pull/4323#discussion_r3894393065
##########
packages/computer-use/package.json:
##########
@@ -11,7 +11,6 @@
"clean": "node ../../scripts/clean-paths.mjs dist tsconfig.tsbuildinfo",
"build": "tsc -p tsconfig.json",
"typecheck": "tsc -p tsconfig.json --noEmit",
- "test": "npm run clean && npm run build && npm run test:dist",
Review Comment:
Fixed in e13abb62e0. Updated all seven live references to the `npm run build` + `run test:dist` shape that CONTRIBUTING uses: `packages/computer-use/README.md`, `apps/desktop/tests/smoke.md`, the three Computer Use test headers, and the three `@maka/runtime` test headers. No compatibility alias — the deleted `test` scripts were hiding a full clean rebuild inside every invocation, so the build step is now explicit. A repo-wide grep for the old form comes back empty outside `docs/archive/`.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
apps/desktop/package.json:
##########
@@ -34,8 +34,6 @@
"package:windows-x64": "electron-builder --config electron-builder.config.mjs --win --x64 --publish never",
"typecheck": "tsc -p tsconfig.preload.json --noEmit && tsc -p tsconfig.main.json --noEmit && tsc -p tsconfig.renderer.json --noEmit && tsc -p tsconfig.storybook.json --noEmit",
"typecheck:stories": "tsc -p tsconfig.storybook.json --noEmit",
- "pretest": "npm run build:workspace-deps",
- "test": "npm run clean:main && npm run build:main && node --test \"dist/main/**/*.test.js\"",
Review Comment:
Fixed in e13abb62e0. Updated all seven live references to the `npm run build` + `run test:dist` shape that CONTRIBUTING uses: `packages/computer-use/README.md`, `apps/desktop/tests/smoke.md`, the three Computer Use test headers, and the three `@maka/runtime` test headers. No compatibility alias — the deleted `test` scripts were hiding a full clean rebuild inside every invocation, so the build step is now explicit. A repo-wide grep for the old form comes back empty outside `docs/archive/`.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor: collapse duplicate test infrastructure [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #4323:
URL: https://github.com/apache/maka/pull/4323
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor: collapse duplicate test infrastructure [maka]
Posted by "hqhq1025 (via GitHub)" <gi...@apache.org>.
hqhq1025 commented on code in PR #4323:
URL: https://github.com/apache/maka/pull/4323#discussion_r3894369050
##########
packages/computer-use/package.json:
##########
@@ -11,7 +11,6 @@
"clean": "node ../../scripts/clean-paths.mjs dist tsconfig.tsbuildinfo",
"build": "tsc -p tsconfig.json",
"typecheck": "tsc -p tsconfig.json --noEmit",
- "test": "npm run clean && npm run build && npm run test:dist",
Review Comment:
**P2:** Removing this script leaves current verification instructions broken. `packages/computer-use/README.md:116` still tells contributors to run `npm --workspace @maka/computer-use test`, and `apps/desktop/tests/smoke.md:29` does the same for Desktop; both commands now fail immediately with `Missing script: "test"`. Several source test headers also retain the removed Runtime command. Please update these live instructions to use `run test:dist` together with the required root build, or retain a compatibility alias.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org