Skip to content

fix: silence zip output when creating source zips #8323

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ndhoule wants to merge 1 commit into
base: main
Choose a base branch
from
+62 −79
Open

fix: silence zip output when creating source zips #8323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 20 additions & 13 deletions src/utils/deploy/upload-source-zip.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
import { execFile } from 'child_process'
import { readFile, mkdir } from 'fs/promises'
import { join, dirname } from 'path'
import { promisify } from 'util'
import type { PathLike } from 'fs'
import { platform } from 'os'
import { mkdir, readFile } from 'node:fs/promises'
import { join, dirname } from 'node:path'
import type { PathLike } from 'node:fs'
import { platform } from 'node:os'

import execa, { ExecaError } from 'execa'
import fetch from 'node-fetch'

import { log, warn } from '../command-helpers.js'
import { temporaryDirectory } from '../temporary-file.js'
import type { DeployEvent } from './status-cb.js'

const execFileAsync = promisify(execFile)

interface UploadSourceZipOptions {
sourceDir: string
uploadUrl: string
Expand Down Expand Up @@ -76,10 +73,20 @@ const createSourceZip = async ({
const excludeArgs = DEFAULT_IGNORE_PATTERNS.flatMap((pattern) => ['-x', pattern])

// Use system zip command to create the archive
await execFileAsync('zip', ['-r', zipPath, '.', ...excludeArgs], {
cwd: sourceDir,
maxBuffer: 1024 * 1024 * 100, // 100MB buffer
})
try {
await execa('zip', ['-r', '-q', zipPath, '.', ...excludeArgs], {
all: true,
cwd: sourceDir,
stdio: ['ignore', 'pipe', 'pipe'],
})
} catch (_baseErr) {
let message = 'zip command failed'
if (_baseErr instanceof Error && 'command' in _baseErr) {
const baseErr = _baseErr as ExecaError
message = `${baseErr.shortMessage}\n\n${baseErr.stdout}`
}
throw new Error(message, { cause: _baseErr })
}
Comment thread
ndhoule marked this conversation as resolved.

return zipPath
}
Expand Down Expand Up @@ -161,7 +168,7 @@ export const uploadSourceZip = async ({
// Clean up temporary zip file
if (zipPath) {
try {
await import('fs/promises').then((fs) => fs.unlink(zipPath as unknown as PathLike))
await (await import('node:fs/promises')).unlink(zipPath as unknown as PathLike)
} catch {
// Ignore cleanup errors
}
Expand Down
108 changes: 42 additions & 66 deletions tests/unit/utils/deploy/upload-source-zip.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
import { describe, expect, test, vi, beforeEach } from 'vitest'
import { join } from 'path'
import { join } from 'node:path'

import type { ExecaReturnValue } from 'execa'
import type { Response } from 'node-fetch'
import type { ChildProcess } from 'child_process'
import { describe, expect, test, vi, beforeEach } from 'vitest'

// Mock all dependencies at the top level
vi.mock('node-fetch', () => ({
default: vi.fn(),
}))

vi.mock('child_process', () => ({
execFile: vi.fn(),
vi.mock('execa', () => ({
default: vi.fn(),
}))

vi.mock('fs/promises', () => ({
Expand Down Expand Up @@ -47,7 +48,7 @@ describe('uploadSourceZip', () => {

// Setup mocks using vi.mocked()
const mockFetch = await import('node-fetch')
const mockChildProcess = await import('child_process')
const mockExeca = await import('execa')
const mockFs = await import('fs/promises')
const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js')
const mockTempFile = await import('../../../../src/utils/temporary-file.js')
Expand All @@ -58,11 +59,9 @@ describe('uploadSourceZip', () => {
statusText: 'OK',
} as unknown as Response)

vi.mocked(mockChildProcess.execFile).mockImplementation((_command, _args, _options, callback) => {
if (callback) {
callback(null, '', '')
}
return {} as ChildProcess
// @ts-expect-error(ndhoule): getting the type on this fairly challenging
vi.mocked(mockExeca.default).mockImplementation((..._args) => {
return Promise.resolve({} as ExecaReturnValue)
})

vi.mocked(mockFs.readFile).mockResolvedValue(Buffer.from('mock zip content'))
Expand All @@ -78,14 +77,10 @@ describe('uploadSourceZip', () => {
statusCb: mockStatusCb,
})

expect(mockChildProcess.execFile).toHaveBeenCalledWith(
expect(mockExeca.default).toHaveBeenCalledWith(
'zip',
expect.arrayContaining(['-r', expect.stringMatching(/test-source\.zip$/), '.']),
expect.objectContaining({
cwd: '/test/source',
maxBuffer: 104857600,
}),
expect.any(Function),
expect.arrayContaining(['-r', '-q', expect.stringMatching(/test-source\.zip$/), '.']),
expect.objectContaining({ cwd: '/test/source' }),
)

expect(mockFetch.default).toHaveBeenCalledWith(
Expand Down Expand Up @@ -116,7 +111,7 @@ describe('uploadSourceZip', () => {
const { uploadSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js')

const mockFetch = await import('node-fetch')
const mockChildProcess = await import('child_process')
const mockExeca = await import('execa')
const mockFs = await import('fs/promises')
const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js')
const mockTempFile = await import('../../../../src/utils/temporary-file.js')
Expand All @@ -127,11 +122,9 @@ describe('uploadSourceZip', () => {
statusText: 'Forbidden',
} as unknown as Response)

vi.mocked(mockChildProcess.execFile).mockImplementation((_command, _args, _options, callback) => {
if (callback) {
callback(null, '', '')
}
return {} as ChildProcess
// @ts-expect-error(ndhoule): getting the type on this fairly challenging
vi.mocked(mockExeca.default).mockImplementation((..._args) => {
return Promise.resolve({} as ExecaReturnValue)
})

vi.mocked(mockFs.readFile).mockResolvedValue(Buffer.from('mock zip content'))
Expand Down Expand Up @@ -168,7 +161,7 @@ describe('uploadSourceZip', () => {
const { uploadSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js')

const mockFetch = await import('node-fetch')
const mockChildProcess = await import('child_process')
const mockExeca = await import('execa')
const mockFs = await import('fs/promises')
const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js')
const mockTempFile = await import('../../../../src/utils/temporary-file.js')
Expand All @@ -179,11 +172,9 @@ describe('uploadSourceZip', () => {
statusText: 'OK',
} as unknown as Response)

vi.mocked(mockChildProcess.execFile).mockImplementation((_command, _args, _options, callback) => {
if (callback) {
callback(null, '', '')
}
return {} as ChildProcess
// @ts-expect-error(ndhoule): getting the type on this fairly challenging
vi.mocked(mockExeca.default).mockImplementation((..._args) => {
return Promise.resolve({} as ExecaReturnValue)
})

vi.mocked(mockFs.readFile).mockResolvedValue(Buffer.from('mock zip content'))
Expand All @@ -199,14 +190,10 @@ describe('uploadSourceZip', () => {
statusCb: mockStatusCb,
})

expect(mockChildProcess.execFile).toHaveBeenCalledWith(
expect(mockExeca.default).toHaveBeenCalledWith(
'zip',
expect.arrayContaining(['-x', 'node_modules*', '-x', '.git*', '-x', '.netlify*', '-x', '.env']),
expect.objectContaining({
cwd: '/test/source',
maxBuffer: 104857600,
}),
expect.any(Function),
expect.objectContaining({ cwd: '/test/source' }),
)
})

Expand Down Expand Up @@ -245,16 +232,14 @@ describe('uploadSourceZip', () => {

const { uploadSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js')

const mockChildProcess = await import('child_process')
const mockExeca = await import('execa')
const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js')
const mockTempFile = await import('../../../../src/utils/temporary-file.js')

// Mock execFile to simulate failure
vi.mocked(mockChildProcess.execFile).mockImplementation((_command, _args, _options, callback) => {
if (callback) {
callback(new Error('zip command failed'), '', 'zip: error creating archive')
}
return {} as import('child_process').ChildProcess
// @ts-expect-error(ndhoule): getting the type on this fairly challenging
vi.mocked(mockExeca.default).mockImplementation((..._args) => {
return Promise.reject(new Error('zip command failed'))
})

vi.mocked(mockCommandHelpers.warn).mockImplementation(() => {})
Expand Down Expand Up @@ -290,17 +275,15 @@ describe('uploadSourceZip', () => {
const { uploadSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js')

const mockFetch = await import('node-fetch')
const mockChildProcess = await import('child_process')
const mockExeca = await import('execa')
const mockFs = await import('fs/promises')
const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js')
const mockTempFile = await import('../../../../src/utils/temporary-file.js')

// Mock successful zip creation but failed upload
vi.mocked(mockChildProcess.execFile).mockImplementation((_command, _args, _options, callback) => {
if (callback) {
callback(null, '', '')
}
return {} as import('child_process').ChildProcess
// @ts-expect-error(ndhoule): getting the type on this fairly challenging
vi.mocked(mockExeca.default).mockImplementation((..._args) => {
return Promise.resolve({} as ExecaReturnValue)
})

vi.mocked(mockFs.readFile).mockResolvedValue(Buffer.from('mock zip content'))
Expand Down Expand Up @@ -337,7 +320,7 @@ describe('uploadSourceZip', () => {
const { uploadSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js')

const mockFetch = await import('node-fetch')
const mockChildProcess = await import('child_process')
const mockExeca = await import('execa')
const mockFs = await import('fs/promises')
const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js')
const mockTempFile = await import('../../../../src/utils/temporary-file.js')
Expand All @@ -349,11 +332,9 @@ describe('uploadSourceZip', () => {
json: vi.fn().mockResolvedValue({ url: 'https://test-source-zip-url.com' }),
} as unknown as import('node-fetch').Response)

vi.mocked(mockChildProcess.execFile).mockImplementation((_command, _args, _options, callback) => {
if (callback) {
callback(null, '', '')
}
return {} as import('child_process').ChildProcess
// @ts-expect-error(ndhoule): getting the type on this fairly challenging
vi.mocked(mockExeca.default).mockImplementation((..._args) => {
return {} as ExecaReturnValue
})

vi.mocked(mockFs.readFile).mockResolvedValue(Buffer.from('mock zip content'))
Expand All @@ -379,7 +360,7 @@ describe('uploadSourceZip', () => {
const { uploadSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js')

const mockFetch = await import('node-fetch')
const mockChildProcess = await import('child_process')
const mockExeca = await import('execa')
const mockFs = await import('fs/promises')
const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js')
const mockTempFile = await import('../../../../src/utils/temporary-file.js')
Expand All @@ -390,11 +371,9 @@ describe('uploadSourceZip', () => {
statusText: 'OK',
} as unknown as Response)

vi.mocked(mockChildProcess.execFile).mockImplementation((_command, _args, _options, callback) => {
if (callback) {
callback(null, '', '')
}
return {} as ChildProcess
// @ts-expect-error(ndhoule): getting the type on this fairly challenging
vi.mocked(mockExeca.default).mockImplementation((..._args) => {
return {} as ExecaReturnValue
})

vi.mocked(mockFs.readFile).mockResolvedValue(Buffer.from('mock zip content'))
Expand All @@ -416,18 +395,15 @@ describe('uploadSourceZip', () => {
expect(mockFs.mkdir).toHaveBeenCalledWith(join('/tmp/test-temp-dir', 'workspace-snapshots'), { recursive: true })

// Should still call zip command with the full path
expect(mockChildProcess.execFile).toHaveBeenCalledWith(
expect(mockExeca.default).toHaveBeenCalledWith(
'zip',
expect.arrayContaining([
'-r',
'-q',
join('/tmp/test-temp-dir', 'workspace-snapshots', 'source-abc123-def456.zip'),
'.',
]),
expect.objectContaining({
cwd: '/test/source',
maxBuffer: 104857600,
}),
expect.any(Function),
expect.objectContaining({ cwd: '/test/source' }),
)
})
})
Loading