test: add unit tests for build adapter#12
Open
Jack-sh1 wants to merge 3 commits into
Open
Conversation
createBuild had zero test coverage. Add tests for output directory creation/cleanup, SPA dist copying, __connection.json generation, __rpc-dump directory and manifest writing, spa-loader.json conditional output, and setup hook invocation.
✅ Deploy Preview for devfra ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
userquin
reviewed
May 13, 2026
|
|
||
| afterEach(async () => { | ||
| await fs.rm(outDir, { recursive: true, force: true }) | ||
| await fs.rm(distDir, { recursive: true, force: true }) |
Contributor
There was a problem hiding this comment.
use try-finally block or use await Promise.allSettled, if something is wrong deleting outDir then distDir won't be removed: it would be also nice if we can add some retry options:
// fs comes from `import fs from 'node:fs/promises`
await fs.rm(tempPath, {
recursive: true,
force: true,
maxRetries: 3,
retryDelay: 100,
}).catch((err) => {
console.error(`Failed to cleanup sandbox at ${tempPath}:`, err)
})Ensures both outDir and distDir are cleaned up even if one removal fails, and adds maxRetries/retryDelay for transient fs errors.
userquin
reviewed
May 13, 2026
There was a problem hiding this comment.
Pull request overview
Adds Vitest unit coverage for the createBuild adapter to validate the static-build output structure and key generated metadata files.
Changes:
- Introduces a new
createBuildtest suite covering outDir creation/cleanup, SPA copying, connection meta, and RPC dump output. - Adds assertions for conditional
spa-loader.jsonemission and missing-distDirerror behavior. - Verifies the
setup()hook is invoked during the build flow.
Comments suppressed due to low confidence (3)
packages/devframe/src/adapters/build.test.ts:37
afterEachswallows cleanup failures and logs withconsole.warn, which can both hide real failures and violateno-consolelinting rules. Consider usingfs.rm(..., { recursive: true, force: true })without logging, or explicitly fail the test when cleanup fails (and if you need silence, stubconsoleviavi.spyOn).
afterEach(async () => {
const rmOpts = { recursive: true, force: true, maxRetries: 3, retryDelay: 100 } as const
await Promise.allSettled([
fs.rm(outDir, rmOpts).catch((err) => {
console.warn(`Failed to cleanup outDir at ${outDir}, manual deletion required:`, err)
}),
fs.rm(distDir, rmOpts).catch((err) => {
console.warn(`Failed to cleanup distDir at ${distDir}, manual deletion required:`, err)
}),
])
packages/devframe/src/adapters/build.test.ts:38
createBuildprints multipleconsole.loglines per invocation; since these tests call it many times, the test run will be very noisy. It’s better to stubconsole.log/console.errorfor this suite (e.g. inbeforeEachwithvi.spyOn(console, 'log').mockImplementation(() => {})) and restore inafterEach.
describe('createBuild', () => {
const outDir = resolve('/tmp/devframe-build-test-out')
const distDir = resolve('/tmp/test-spa-dist')
beforeEach(async () => {
// Create a fake SPA dist directory with an index.html
await fs.mkdir(distDir, { recursive: true })
await fs.writeFile(resolve(distDir, 'index.html'), '<html></html>')
})
afterEach(async () => {
const rmOpts = { recursive: true, force: true, maxRetries: 3, retryDelay: 100 } as const
await Promise.allSettled([
fs.rm(outDir, rmOpts).catch((err) => {
console.warn(`Failed to cleanup outDir at ${outDir}, manual deletion required:`, err)
}),
fs.rm(distDir, rmOpts).catch((err) => {
console.warn(`Failed to cleanup distDir at ${distDir}, manual deletion required:`, err)
}),
])
})
packages/devframe/src/adapters/build.test.ts:104
- The test name says it verifies build-mode context, but it only asserts
setupwas called. To actually cover the behavior, assert thatsetupreceived a context withmode: 'build'(and any other relevant fields) so the test fails ifcreateBuildstops passing the correct context.
it('calls d.setup with build mode context', async () => {
const setup = vi.fn(async () => {})
const d = stubDefinition({ setup })
await createBuild(d, { outDir, distDir })
expect(setup).toHaveBeenCalledOnce()
})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+18
to
+26
| describe('createBuild', () => { | ||
| const outDir = resolve('/tmp/devframe-build-test-out') | ||
| const distDir = resolve('/tmp/test-spa-dist') | ||
|
|
||
| beforeEach(async () => { | ||
| // Create a fake SPA dist directory with an index.html | ||
| await fs.mkdir(distDir, { recursive: true }) | ||
| await fs.writeFile(resolve(distDir, 'index.html'), '<html></html>') | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
createBuildhad zero test coverage. This PR adds unit tests covering:__connection.jsongeneration withbackend: 'static'__rpc-dump/directory andindex.jsonmanifest writingspa-loader.jsonconditional output (only whend.spais defined)d.setup()hook invocation with build mode contextdistDiris not providedTest plan
vitest