mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-07 16:11:46 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Paperclip runs the server and runner test suites with Vitest. > - Vitest 5 removes the deprecated `describe.sequential` property, so the pending Vitest 5 upgrade fails the type-check and test jobs. > - `describe.sequential` only changes behaviour inside a `describe.concurrent` suite, or when `sequence.concurrent` is on. > - This repository has neither, so the modifier changed nothing at run time. > - The benefit is that the Vitest 5 upgrade can land, and the test files lose a modifier that did no work. ## Linked Issues or Issue Description Refs: #12969 ## What Changed - Replace every `describe.sequential` use with a plain `describe` call. - Drop the `{ concurrent: false }` suite option from the two runner test files. - Add a comment to `server/vitest.config.ts` that records why these suites must run one test at a time. - Leave the package manifests and the lockfile unchanged. ## Why the modifier did nothing The Vitest documentation states that `describe.sequential` is useful to run tests in sequence inside a `describe.concurrent` suite, or with the `--sequence.concurrent` option. `sequence.concurrent` defaults to `false`. This repository satisfies neither condition: - No test file uses `describe.concurrent`, `it.concurrent`, or `test.concurrent`. - `server/vitest.config.ts` sets `sequence.concurrent: false`, with `maxWorkers: 1`, `maxConcurrency: 1`, and `isolate: true`. - `packages/paperclip-runner/vitest.config.ts` sets no `sequence` block, so the `false` default applies. `packages/db` and `cli` already run the same embedded-Postgres suites with a plain `describe`, and those jobs are green. The server package was the only outlier. The modifier did carry one real piece of knowledge: these suites need their tests to run one at a time. The new comment in `server/vitest.config.ts` records that reason next to the setting that enforces it. ## Verification - `git grep` for `describe.sequential` returns nothing outside `node_modules`. - The author ran the changed server test files under the installed Vitest 4, and the results match the results without this change. - Two very large embedded-Postgres test files exceeded the author's local memory limit, so the CI test jobs cover those two. - The two changed runner test files have pre-existing local failures caused by a missing Rust toolchain and a missing global `pnpm` binary. The failures are identical with and without this change. - The author type-checked the changed files and found no new error. - CI must pass the typecheck, build, server test, and runner verify jobs. ## Risks - Low risk. Suite execution stays serial, because the Vitest config enforces it. - The change adds no dependency and changes no package manifest or lockfile. - A future change that turns `sequence.concurrent` on would break these suites. The new config comment warns against it. ## Model Used - Claude Sonnet 5 — code edits and local verification. - OpenAI Codex, GPT-5 — the earlier revision of this branch. ## Test plan - [x] Every CI check reaches a terminal green state. A pending or queued check is not a pass. - [x] The `Typecheck + Release Registry` job passes. This change must not introduce a type error. - [x] The `Build` job passes. - [x] The server test jobs and the runner verify jobs pass. - [x] Greptile re-reviews this commit set and posts a passing verdict. The dependabot waiver does not apply to this pull request. - [x] `mergeable` reads `MERGEABLE` as a terminal value. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have linked the related public issue with `Refs: #12969` - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-Authored-By: Priya Raman <priya.raman@paperclip.ing> --------- Co-authored-by: Priya Raman <priya.raman@paperclip.ing> Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: nickyleach <331803+nickyleach@users.noreply.github.com>
52 lines
2.0 KiB
TypeScript
52 lines
2.0 KiB
TypeScript
import { fileURLToPath } from "node:url";
|
|
|
|
import { defineConfig } from "vitest/config";
|
|
|
|
export default defineConfig({
|
|
resolve: {
|
|
alias: [
|
|
{
|
|
find: /^@paperclipai\/paperclip-runner$/,
|
|
replacement: fileURLToPath(
|
|
new URL("../packages/paperclip-runner/src/index.ts", import.meta.url),
|
|
),
|
|
},
|
|
],
|
|
},
|
|
test: {
|
|
environment: "node",
|
|
include: ["src/**/*.test.ts", "scripts/**/*.test.mjs"],
|
|
// Each server suite boots + tears down its own embedded Postgres in
|
|
// beforeAll/afterAll. Under the loaded serial shard (maxWorkers=1) the
|
|
// graceful shutdown can occasionally cross vitest's default 10s hookTimeout,
|
|
// producing flaky "Hook timed out in 10000ms" afterAll failures on CI. Give
|
|
// the boot/teardown hooks generous headroom; 30s is far above the observed
|
|
// worst-case teardown yet still catches a genuinely hung hook. teardownTimeout
|
|
// mirrors it for the same reason.
|
|
hookTimeout: 30000,
|
|
teardownTimeout: 30000,
|
|
// The route/authz suites import very large modules (for example
|
|
// src/routes/issues.ts and its dependency graph). The first test in each
|
|
// file pays the one-time transform cost inside its own timeout budget. On
|
|
// the loaded serial shard (maxWorkers=1) that cost can cross vitest's
|
|
// default 5s testTimeout and fail the first test, which also lets its
|
|
// fire-and-forget wake leak into the next test. Give each test generous
|
|
// headroom; 15s is far above the observed module-load cost yet still
|
|
// catches a genuinely hung test well inside the 20 minute job limit.
|
|
testTimeout: 15000,
|
|
isolate: true,
|
|
maxConcurrency: 1,
|
|
maxWorkers: 1,
|
|
minWorkers: 1,
|
|
pool: "forks",
|
|
// Server suites share process state and one embedded Postgres instance,
|
|
// so tests inside a file must run one at a time. Do not set
|
|
// sequence.concurrent to true.
|
|
sequence: {
|
|
concurrent: false,
|
|
hooks: "list",
|
|
},
|
|
setupFiles: ["./src/__tests__/setup-supertest.ts"],
|
|
},
|
|
});
|