Skip to content

test: make the suite runnable cold and fail on webpack/rspack build errors - #629

Merged
antfu merged 4 commits into
unjs:mainfrom
antfubot:test/harness-cold-start
Oct 27, 2026
Merged

antfu merged 4 commits into
unjs:mainfrom
antfubot:test/harness-cold-start

Conversation

@antfubot

@antfubot antfubot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Implements plans/001-test-harness.md: makes the test suite trustworthy and runnable from a clean clone. No runtime (src/) changes.

  • pnpm test builds first. The webpack/rspack adapters load their loaders from dist/ under vitest (import.meta.dev), so a fresh clone failed with confusing loader errors. test is now nr build && nr test:build && vitest run; a new test:unit script runs only test/unit-tests for the fast loop after a build.
  • Fixture builder fixed. rm(dist) was missing { recursive: true }, rejected with ERR_FS_EISDIR and was swallowed, so stale output was never cleaned. The 21 --version spawns are gone, the seven bundlers of a fixture now build concurrently (each writes to its own dist/<bundler>), a failing bundler no longer aborts the rest, and a second positional arg filters by bundler: pnpm test:build transform webpack.
  • webpack/rspack integration tests reject on build errors. 16 call sites used build.webpack(config, resolve) and passed whatever the compilation reported. They now go through runWebpack/runRspack in test/unit-tests/utils.ts, which reject on stats.hasErrors().
  • Loader filter tests use real filter shapes. filter: vi.fn().mockReturnValue(false) is not a filter — normalizeObjectHook reads filter.id/filter.code, finds undefined, and matches everything. The "filter does not match" tests passed only because the handler mock returned undefined. They now use { id: /\.never-matches$/ } and assert the handler is not called (so the binary pass-through test from fix(webpack): keep binary files intact in the load and transform loaders #623 actually tests the pass-through).
  • plans/** is excluded from ESLint: the plan documents' illustrative code blocks made pnpm lint fail on main after docs: add advisor implementation plans from codebase audit #628.

Found along the way

Making the webpack/rspack runners honest surfaced that test/unit-tests/id-consistency had been producing erroring builds all along: externals: ['node:path'] with the default externalsType: 'var' emits var x = node:path, which neither webpack nor rspack can parse. The hook-count assertions still passed, so nobody noticed. Fixed by setting externalsType: 'commonjs' in that test.

Verification

  • rm -rf dist test/fixtures/*/dist && pnpm test → 200 passed, 63 skipped
  • pnpm test:unit, pnpm lint, pnpm typecheck → green
  • Sanity: pointing a webpack test at a missing entry now fails it; making a transform loader's filter match again now fails the "does not match" test.

Created with the help of an agent.

Summary by CodeRabbit

  • Tests

    • Added dedicated unit-test execution and updated build checks to provide more consistent results across supported bundlers.
    • Test runs now report individual build failures while continuing through the remaining fixtures.
  • Chores

    • Updated linting exclusions and recorded the completed status of a project plan.

@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Caution

Review failed

The pull request is closed.

Walkthrough

The pull request updates fixture builds to run asynchronously and concurrently. It adds shared Webpack and Rspack test runners, updates compiler tests to use them, changes loader filter fixtures to regular expressions, and adjusts test scripts and repository metadata.

Changes: Fixture and test execution

Layer / File(s) Summary
Fixture build orchestration
scripts/buildFixtures.ts
Fixture builds use configurable bundlers, optional filters, concurrent execution, scoped cleanup, captured output, and aggregated failure status.
Shared compiler test runners
test/unit-tests/utils.ts, test/unit-tests/*
Webpack and Rspack tests use promise-based runWebpack and runRspack helpers. The ID-consistency tests also set externalsType to commonjs.
Loader filter test updates
test/unit-tests/webpack/loaders/transform.test.ts, test/unit-tests/rspack/loaders/transform.test.ts
Loader fixtures use matching or nonmatching ID regular expressions instead of mocked filter functions.
Test workflow and repository metadata
package.json, eslint.config.js, plans/README.md
The test command builds the package first, a separate unit-test command is added, plans are ignored by ESLint, and Plan 001 is marked DONE.

Priority: Not assessed

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Other

Sequence Diagram(s)

sequenceDiagram
  participant TestCommand
  participant buildFixtures
  participant Bundlers
  participant Vitest
  TestCommand->>buildFixtures: build selected fixtures
  buildFixtures->>Bundlers: run selected bundlers concurrently
  Bundlers-->>buildFixtures: return build output or failure
  TestCommand->>Vitest: run unit tests
  Vitest->>Webpack: compile through runWebpack
  Vitest->>Rspack: compile through runRspack
Loading

Possibly related issues

  • unjs/unplugin#421: The tests cover Webpack and Rspack virtual IDs, but the pull request does not add Windows path normalization.

Suggested reviewers: antfu, sxzz

Merge Risk: Moderate · up to c960a

Build the loader entries before running unit tests directly, and make mistyped bundler filters fail, before merging.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Oct 27, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/unjs/unplugin@629

commit: c960ad3

@antfu
antfu merged commit dafbcc5 into unjs:main Oct 27, 2026
12 of 13 checks passed
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