feat(test-utils): add createTestStore helper #296
Conversation
…d test setup - Introduces `createTestStore` to streamline store creation in tests. - Combines test directory setup, store creation, and optional API mocking. - Supports custom file structures and various filesystem bridges. - Provides auto-initialization and returns both store and storePath for easy assertions.
…ve API mocking * Changed `structure` and `manifest` types to `DirectoryJSON` and `UCDStoreManifest` respectively. * Updated `mockApi` type to exclude `versions` and `baseUrl` from `MockStoreConfig`. * Introduced `loadNodeBridge` function for better handling of Node.js FileSystemBridge loading. * Enhanced API mocking logic to include manifest in mocked responses.
🦋 Changeset detectedLatest commit: 3a39fc0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
the
pkg: test-utils
🌏 Preview Deployments
Built from commit: 🤖 This comment will be updated automatically when you push new commits to this PR. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new createTestStore helper to @ucdjs/test-utils (with types), exposes mockStoreApi from test-utils index, adds fs-bridge dependency declarations, introduces tests for mock-store and test-store, and includes a formatting-only change in ucd-store's store.ts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor TestCode as Test Code
participant TS as createTestStore
participant TD as Testdir helper
participant FB as FS Bridge (provided or Node)
participant API as mockStoreApi (MSW)
participant UCD as UCDStore
TestCode->>TS: createTestStore(options)
alt basePath provided
TS->>TS: use provided basePath as storePath
else
TS->>TD: create temp test directory
TD-->>TS: storePath
end
opt options.mockApi
TS->>API: setup(baseUrl, versions, manifest, responses)
API-->>TS: handlers active
end
alt options.fs provided
TS->>FB: use provided fs bridge
else
TS->>FB: load Node bridge for storePath
end
FB-->>TS: fs bridge instance
TS->>UCD: new UCDStore({ fs, basePath, baseUrl, versions, globalFilters })
opt autoInit !== false
TS->>UCD: init()
UCD-->>TS: initialized
end
TS-->>TestCode: return { store, storePath }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
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. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…r handling * Reformatted method signatures for `getFileTree`, `getFilePaths`, and `getFile` for better readability. * Enhanced error handling in manifest validation to improve clarity and maintainability.
* Implement tests for basic setup, version configuration, response configuration, mixed configuration, endpoint handlers, and edge cases. * Ensure functionality for default and custom configurations, including error handling for disabled endpoints. * Validate response data structure and behavior for various scenarios.
- Added `testdirsOptions` to customize test directory creation. - Improved handling of `mockApi` option with default value. - Updated version inference logic based on provided manifest. - Introduced comprehensive tests for `createTestStore` covering various scenarios.
- Implement tests for basic store creation scenarios. - Validate behavior for auto-initialization and version handling. - Test custom filesystem bridge integration and global filters application.
…narios * Added tests for handling empty structure and manifest objects. * Verified behavior when no options are provided, ensuring proper initialization. * Included checks for custom filesystem bridge and API mocking configurations. * Improved test coverage for return values and directory creation logic.
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new createTestStore helper function to the @ucdjs/test-utils package to simplify test setup by combining testdir creation, store initialization, and API mocking into a single function.
Key changes:
- Added
createTestStorehelper with comprehensive test coverage - Updated package dependencies to include required peer dependencies
- Improved code formatting with multi-line function signatures in the store package
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ucd-store/src/store.ts | Reformatted function signatures for better readability |
| packages/test-utils/src/test-store.ts | Core implementation of createTestStore helper function |
| packages/test-utils/test/test-store.test.ts | Comprehensive test suite for createTestStore functionality |
| packages/test-utils/test/mock-store.test.ts | Test suite for mockStoreApi functionality |
| packages/test-utils/src/index.ts | Export new createTestStore function and types |
| packages/test-utils/package.json | Added fs-bridge peer dependency |
| .changeset/clever-horses-shout.md | Changelog entry documenting the new feature |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
github-actions
bot
added
the
pkg: ucd-store
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/test-utils/test/test-store.test.ts (2)
108-108: Consider using a public API instead of accessing private methods.Tests access the private
~readManifest()method via bracket notation in multiple locations. This couples tests to internal implementation details, making them brittle if the method name or behavior changes.If manifest reading is a valid test scenario, consider:
- Exposing a public
getManifest()orreadManifest()method on the store for test verification- Verifying manifest behavior indirectly through public APIs (e.g., checking that files from the manifest are accessible)
Alternatively, if this is an accepted pattern in your codebase for testing internal state, document this convention in your test utilities or contribution guidelines.
Also applies to: 124-124, 149-149, 279-279, 362-362
276-281: Clarify the test intent and expected behavior.The comment states "reading manifest should return empty object" but then explains it will throw an error, which is the expected behavior. The test catches the error and returns
{}as a fallback. This creates confusion about what's actually being tested.Make the test intent clearer:
- // NOTE: reading manifest should return empty object - // But this will throw an error, since the manifest doesn't exist. - // Which is the expected behavior, and what we are testing for. - const readManifest = await store["~readManifest"]().catch(() => ({})); - expect(readManifest).toEqual({}); + // Verify that manifest doesn't exist when basePath is provided + // The read should fail since no manifest was written to the basePath + await expect(store["~readManifest"]()).rejects.toThrow();This makes it explicit that you're testing for an error, not for an empty object return value.
packages/test-utils/src/test-store.ts (3)
46-47: Document the basePath interaction with structure/manifest.When
basePathis provided, thestructureandmanifestoptions are ignored for filesystem creation (lines 167-171), butmanifestis still included in API mock responses (lines 150-155). This creates a subtle inconsistency where the manifest exists in mocked API responses but not on the filesystem.Update the JSDoc for the
basePathoption to clarify this behavior:/** * Base path for the store - * When using structure/manifest, this will be set to the testdir path + * When provided, structure/manifest options are ignored for filesystem operations + * (the basePath directory is used as-is), but manifest may still affect API mocking. + * When not provided, a temporary testdir is created from structure/manifest. */ basePath?: string;Additionally, consider whether the current behavior is intentional. If
basePathis provided withstructure, should the function:
- Throw an error (conflicting options)
- Apply the structure to the basePath
- Ignore the structure (current behavior)
The test suite validates option 3, but this may surprise users.
Also applies to: 167-171
93-93: Make storePath non-optional or document when it's undefined.The
storePathfield is typed as optional (storePath?: string), but examining the implementation (lines 167-171), it's always set—either fromoptions.basePathor fromtestdir().Consider making it non-optional:
/** * Path to the test directory (only present when using structure/manifest or basePath) */ - storePath?: string; + storePath: string;If there's a future scenario where
storePathcould be undefined, update the implementation to explicitly handle that case and document when it occurs.
96-104: Consider error handling for bridge loading.The
loadNodeBridgefunction throws if the Node bridge can't be loaded (line 100), but there's no try-catch around its usage at line 173. While this may be acceptable (letting errors propagate), consider whether you want more specific error messages for users.If you want more helpful error messages:
const fs = options.fs || await loadNodeBridge(storePath);could become:
const fs = options.fs || await loadNodeBridge(storePath).catch((err) => { throw new Error( `Failed to load Node filesystem bridge for path "${storePath}": ${err.message}` ); });This is optional depending on your error handling philosophy—simple propagation is often clearer.
Also applies to: 173-173
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.changeset/clever-horses-shout.md(1 hunks)packages/test-utils/package.json(2 hunks)packages/test-utils/src/index.ts(1 hunks)packages/test-utils/src/test-store.ts(1 hunks)packages/test-utils/test/mock-store.test.ts(1 hunks)packages/test-utils/test/test-store.test.ts(1 hunks)packages/ucd-store/src/store.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{test,spec}.?(c|m)[jt]s?(x)
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.?(c|m)[jt]s?(x): Name test files using the pattern **/.{test,spec}.?(c|m)[jt]s?(x)
In tests, import from #internal/test-utils/ instead of @ucdjs/test-utils
Use setupMockStore() from #internal/test-utils/mock-store for UCD API mocking in tests
Use testdir() from vitest-testdirs for filesystem-related tests
Files:
packages/test-utils/test/test-store.test.tspackages/test-utils/test/mock-store.test.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: ucdjs/ucd#0
File: AGENTS.md:0-0
Timestamp: 2025-10-04T11:03:04.840Z
Learning: Applies to **/*.{test,spec}.?(c|m)[jt]s?(x) : Use setupMockStore() from #internal/test-utils/mock-store for UCD API mocking in tests
📚 Learning: 2025-07-20T05:37:40.565Z
Learnt from: luxass
PR: ucdjs/ucd#131
File: tooling/eslint-plugin/package.json:0-0
Timestamp: 2025-07-20T05:37:40.565Z
Learning: In the ucdjs/ucd project, internal tooling packages (private packages in the tooling/ directory) export TypeScript files directly without requiring a build step, unlike published packages which use tsdown to build and export from ./dist/. Examples include ucdjs/tsdown-config and ucdjs/eslint-plugin.
Applied to files:
packages/test-utils/package.json
📚 Learning: 2025-10-04T11:03:04.840Z
Learnt from: CR
PR: ucdjs/ucd#0
File: AGENTS.md:0-0
Timestamp: 2025-10-04T11:03:04.840Z
Learning: Applies to **/*.{test,spec}.?(c|m)[jt]s?(x) : Use setupMockStore() from #internal/test-utils/mock-store for UCD API mocking in tests
Applied to files:
.changeset/clever-horses-shout.mdpackages/test-utils/src/index.tspackages/test-utils/test/test-store.test.tspackages/test-utils/src/test-store.tspackages/test-utils/test/mock-store.test.ts
📚 Learning: 2025-06-09T05:10:32.105Z
Learnt from: luxass
PR: ucdjs/ucd#45
File: packages/ucd-store/src/download.ts:24-24
Timestamp: 2025-06-09T05:10:32.105Z
Learning: In the ucd-store package refactor, picomatch was moved from direct usage in download.ts to internal usage within the createPathFilter function in filter.ts. The pattern format is still picomatch-compatible, so JSDoc comments referencing picomatch pattern format remain correct.
Applied to files:
packages/ucd-store/src/store.ts
🧬 Code graph analysis (3)
packages/ucd-store/src/store.ts (4)
packages/shared/src/filter.ts (1)
PathFilterOptions(37-50)packages/shared/src/try-catch.ts (1)
OperationResult(3-3)packages/ucd-store/src/errors.ts (2)
StoreError(80-86)UCDStoreInvalidManifestError(63-71)packages/schemas/src/fs.ts (1)
UCDStoreManifestSchema(4-24)
packages/test-utils/test/test-store.test.ts (2)
tooling/moonbeam/src/esm-loader.mjs (1)
entries(44-44)packages/test-utils/src/msw.ts (1)
HttpResponse(11-11)
packages/test-utils/src/test-store.ts (2)
packages/test-utils/src/index.ts (6)
CreateTestStoreOptions(5-5)MockStoreConfig(3-3)CreateTestStoreResult(5-5)createTestStore(4-4)mockStoreApi(2-2)mockStoreApi(2-2)packages/ucd-store/src/store.ts (1)
fs(85-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: typecheck
- GitHub Check: test (windows-latest)
- GitHub Check: test (ubuntu-latest)
- GitHub Check: test (macos-latest)
- GitHub Check: lint
- GitHub Check: deploy apps / deploy app (docs)
🔇 Additional comments (5)
packages/ucd-store/src/store.ts (1)
122-221: Formatting change only; no functional impact.Confirmed the signature reflow keeps parameter lists and behavior intact. No further action needed.
packages/test-utils/src/index.ts (1)
2-5: Export surface looks good.
mockStoreApiandcreateTestStore(plus their types) are now properly exposed for consumers.packages/test-utils/test/test-store.test.ts (1)
1-9: LGTM! Comprehensive test coverage.The test suite thoroughly covers:
- Basic store creation with various configurations
- Structure and manifest handling
- Custom filesystem bridges and basePath behavior
- Global filters
- API mocking scenarios
- Return value validation
The test organization is clear and the use of
onTestFinishedfor cleanup in tests withcleanup: falsedemonstrates good testing practices.Also applies to: 21-387
packages/test-utils/src/test-store.ts (2)
178-178: Verify basePath fallback behavior.Line 178 uses
storePath || ""as a fallback, but based on the logic in lines 167-171,storePathshould always be defined at this point. The empty string fallback could mask bugs.Verify whether
storePathcan ever be undefined at line 178. If not, consider removing the fallback:- basePath: storePath || "", + basePath: storePath,Or add an assertion if it should never be undefined:
+ if (!storePath) { + throw new Error("storePath must be defined"); + } basePath: storePath,
1-189: Well-structured test utility implementation.The implementation is clean and well-organized:
- Clear type definitions with helpful JSDoc comments and examples
- Logical flow from options processing to store creation
- Good separation of concerns with the
loadNodeBridgehelper- Sensible defaults (auto-init, mock API, version inference)
The API surface is intuitive and the flexibility to customize filesystem bridges, API mocking, and directory structure makes this a powerful testing utility.
| it("should disable endpoints when responses is set to false", async () => { | ||
| try { | ||
| mockStoreApi({ | ||
| responses: { | ||
| "/api/v1/versions": false, | ||
| }, | ||
| }); | ||
|
|
||
| const res = await fetch("https://api.ucdjs.dev/api/v1/versions"); | ||
| expect(res.ok).toBe(true); | ||
| expect(res.status).toBe(200); | ||
|
|
||
| expect.fail( | ||
| "mockStoreApi should have thrown an error, since /versions mocked is disabled\n" | ||
| + "And MSW should throw have blocked it", | ||
| ); | ||
| } catch (err) { | ||
| const msg = (err as Error).message; | ||
| expect(msg).toBe("[MSW] Cannot bypass a request when using the \"error\" strategy for the \"onUnhandledRequest\" option."); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Test always fails due to expect.fail.
Inside the try block nothing throws before expect.fail, so the catch receives the expect.fail error and the assertion on the MSW message can never pass. As written, this test fails 100% of the time. Please assert on the rejected fetch (or the thrown MSW error) directly instead of relying on expect.fail.
- it("should disable endpoints when responses is set to false", async () => {
- try {
- mockStoreApi({
- responses: {
- "/api/v1/versions": false,
- },
- });
-
- const res = await fetch("https://api.ucdjs.dev/api/v1/versions");
- expect(res.ok).toBe(true);
- expect(res.status).toBe(200);
-
- expect.fail(
- "mockStoreApi should have thrown an error, since /versions mocked is disabled\n"
- + "And MSW should throw have blocked it",
- );
- } catch (err) {
- const msg = (err as Error).message;
- expect(msg).toBe("[MSW] Cannot bypass a request when using the \"error\" strategy for the \"onUnhandledRequest\" option.");
- }
- });
+ it("should disable endpoints when responses is set to false", async () => {
+ mockStoreApi({
+ responses: {
+ "/api/v1/versions": false,
+ },
+ });
+
+ await expect(
+ fetch("https://api.ucdjs.dev/api/v1/versions"),
+ ).rejects.toThrow(
+ '[MSW] Cannot bypass a request when using the "error" strategy for the "onUnhandledRequest" option.',
+ );
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("should disable endpoints when responses is set to false", async () => { | |
| try { | |
| mockStoreApi({ | |
| responses: { | |
| "/api/v1/versions": false, | |
| }, | |
| }); | |
| const res = await fetch("https://api.ucdjs.dev/api/v1/versions"); | |
| expect(res.ok).toBe(true); | |
| expect(res.status).toBe(200); | |
| expect.fail( | |
| "mockStoreApi should have thrown an error, since /versions mocked is disabled\n" | |
| + "And MSW should throw have blocked it", | |
| ); | |
| } catch (err) { | |
| const msg = (err as Error).message; | |
| expect(msg).toBe("[MSW] Cannot bypass a request when using the \"error\" strategy for the \"onUnhandledRequest\" option."); | |
| } | |
| }); | |
| it("should disable endpoints when responses is set to false", async () => { | |
| mockStoreApi({ | |
| responses: { | |
| "/api/v1/versions": false, | |
| }, | |
| }); | |
| await expect( | |
| fetch("https://api.ucdjs.dev/api/v1/versions"), | |
| ).rejects.toThrow( | |
| '[MSW] Cannot bypass a request when using the "error" strategy for the "onUnhandledRequest" option.', | |
| ); | |
| }); |
🤖 Prompt for AI Agents
In packages/test-utils/test/mock-store.test.ts around lines 78 to 98, the test
currently uses a try block that never throws and then calls expect.fail(),
causing the catch to only ever see the expect.fail error; replace this by
asserting the fetch rejects directly (for example use await
expect(fetch("https://api.ucdjs.dev/api/v1/versions")).rejects.toThrow(...) with
the MSW error message) or move the fetch into a try/catch and assert the caught
error message before calling expect.fail only as a fallback; remove the
unconditional expect.fail so the test verifies the MSW rejection correctly.
| ); | ||
| } catch (err) { | ||
| const msg = (err as Error).message; | ||
| expect(msg).toBe("[MSW] Cannot bypass a request when using the \"error\" strategy for the \"onUnhandledRequest\" option."); |
There was a problem hiding this comment.
MSW error message assertion is fragile.
The test relies on the exact MSW error message format. If MSW updates its error messages in future versions, this assertion will break even though the underlying behavior (blocking unhandled requests) remains correct.
Consider making the assertion more resilient:
- expect(msg).toBe("[MSW] Cannot bypass a request when using the \"error\" strategy for the \"onUnhandledRequest\" option.");
+ expect(msg).toContain("Cannot bypass a request");
+ expect(msg).toContain("onUnhandledRequest");Or verify the error type/code if MSW provides structured error information.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(msg).toBe("[MSW] Cannot bypass a request when using the \"error\" strategy for the \"onUnhandledRequest\" option."); | |
| expect(msg).toContain("Cannot bypass a request"); | |
| expect(msg).toContain("onUnhandledRequest"); |
🤖 Prompt for AI Agents
In packages/test-utils/test/test-store.test.ts around line 315, the test asserts
the exact MSW error message string which is brittle; update the assertion to be
resilient by checking for a stable substring or pattern (e.g., that the message
contains "Cannot bypass a request" and mentions the "error" strategy or use a
regex), or, if MSW exposes a structured error (type/code), assert on that
property instead of the full message; modify the expect to use toMatch /
toContain or to check error.name/code accordingly.
* Improved the handling of the manifest option in `createTestStore` to ensure existing responses are preserved. * Removed unnecessary import of `createUCDStore` as it is now imported at the top of the file.
🔗 Linked issue
📚 Description
Summary by CodeRabbit