Skip to content

LTRAC-1583: Add a unit test harness to core - #3177

Closed
jorgemoya wants to merge 1 commit into
canaryfrom
jorgemoya/core-vitest-setup
Closed

LTRAC-1583: Add a unit test harness to core#3177
jorgemoya wants to merge 1 commit into
canaryfrom
jorgemoya/core-vitest-setup

Conversation

@jorgemoya

Copy link
Copy Markdown
Contributor

Linear: LTRAC-1583

Base for #3170 and #3176.

What/Why?

core has never had a unit test runner. tests/ holds 63 Playwright specs run by playwright test; there was no test script, no runner, and no way to unit test anything under lib/.

This was originally bundled into #3170. Split out because "add a test runner to a package that has never had one" carries its own questions — layout, scoping, CI — that shouldn't be reviewed as a footnote to a caching change, and because two unrelated PRs needing it shouldn't be serialised behind each other.

include: ['lib/**/*.spec.ts'] is the load-bearing part. Playwright specs share the *.spec.ts suffix, so an unscoped Vitest would collect all 63 and fail on their fixture imports. Verified the scoping holds: a run on this commit reports zero files despite those 63 being present.

Colocated rather than under tests/, matching packages/catalyst, where build.spec.ts sits next to build.ts. Putting them under tests/ was considered — it would need either a separate suffix or a testIgnore entry, since testDir: './tests' means Playwright collects anything .spec.ts there, and tests/lib/ already means "helpers for Playwright tests" so tests/lib/kv would read ambiguously.

passWithNoTests because turbo run test invokes this in every package defining a test script, and Vitest treats an empty run as a failure. Nothing under lib/ has specs until the PRs building on this land, so without it this commit would fail the CI job it exists to enable. Reasonable to drop once specs are in — flagging it because it does mean a future broken include glob would fail silently rather than loudly.

The ~ alias mirrors the tsconfig path alias so lib code importing ~/... resolves as it does in the app.

Testing

pnpm --filter @bigcommerce/catalyst-core test exits 0 with No test files found. pnpm install --frozen-lockfile passes, so the lockfile entry is consistent.

The two PRs on top of this exercise it for real — #3170 adds 36 specs, #3176 adds 9.

Migration

None. No changeset: dev tooling with no consumer-visible behaviour.

core has never had one. `tests/` holds 63 Playwright specs run by
`playwright test`; there was no `test` script, no runner, and no way to unit
test anything under `lib/`.

Scoped to `lib/**/*.spec.ts` deliberately. Playwright specs share the
`*.spec.ts` suffix, so an unscoped Vitest would collect all 63 and fail on
their fixture imports. Confirmed the scoping holds: a run in this commit
reports zero files despite those 63 being present.

Colocated rather than under `tests/`, matching how packages/catalyst already
does unit tests -- build.spec.ts next to build.ts, and so on. `tests/` is
Playwright's testDir, so specs placed there would need either a separate
suffix or a testIgnore entry to keep the two runners apart; and `tests/lib/`
already means "helpers for Playwright tests", which would make `tests/lib/kv`
ambiguous.

passWithNoTests because `turbo run test` invokes this in every package
defining a `test` script, and Vitest treats an empty run as a failure. Nothing
under `lib/` has specs until the PRs that build on this land, so without it
this commit would fail the CI job it exists to enable.

The `~` alias mirrors the tsconfig path alias so lib code importing `~/...`
resolves the same way it does in the app.

Split out of the Cloudflare KV adapter work so that adding a test runner --
which carries its own questions about layout, scoping and CI -- is reviewable
and revertable on its own, and so the changes that need it aren't serialised
behind each other.

No changeset: dev tooling with no consumer-visible behaviour.

Refs LTRAC-1583

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jorgemoya
jorgemoya requested a review from a team as a code owner August 18, 2026 02:20
@changeset-bot

changeset-bot Bot commented Aug 18, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 82f1943

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel

vercel Bot commented Aug 18, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
catalyst Ready Ready Preview Aug 18, 2026 2:20am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

Bundle Size Report

Comparing against baseline from 0c49112 (2026-08-18).

No bundle size changes detected.

@chanceaclark chanceaclark left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we have any unit tests in core?

@jorgemoya jorgemoya closed this Aug 18, 2026
@jorgemoya

Copy link
Copy Markdown
Contributor Author

Do we have any unit tests in core?

No, only Playwright tests. I will fold this PR into another one that includes unit tests, to skip adding passWithNoTests: true,.

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.

2 participants