From d5e21c4b38dda48e4bdc15240bd5ae1376bfb2c6 Mon Sep 17 00:00:00 2001 From: Wil Wilsman Date: Thu, 28 Jul 2022 10:50:05 -0500 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Use=20env=20sha=20for=20build:wait?= =?UTF-8?q?=20when=20there's=20a=20project=20with=20no=20commit=20(#1008)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/cli-build/src/wait.js | 13 ++++++------ packages/cli-build/test/wait.test.js | 17 +++++++++++++++- packages/client/src/client.js | 12 ++++++----- packages/client/test/client.test.js | 30 ++++++++++++++++++++++------ 4 files changed, 54 insertions(+), 18 deletions(-) diff --git a/packages/cli-build/src/wait.js b/packages/cli-build/src/wait.js index 8b26d11b2..9385a9bcf 100644 --- a/packages/cli-build/src/wait.js +++ b/packages/cli-build/src/wait.js @@ -12,7 +12,6 @@ export const wait = command('wait', { }, { name: 'project', description: 'Build project slug, requires \'--commit\'', - requires: ['commit'], type: 'slug', short: 'p' }, { @@ -78,9 +77,11 @@ function logProgress({ 'total-comparisons': total, 'total-comparisons-diff': diffs, 'total-comparisons-finished': finished - } -}, log) { + } = {} +} = {}, log) { switch (state) { + case undefined: + return log.progress('Waiting for build...'); case 'pending': return log.progress('Recieving snapshots...'); case 'processing': @@ -123,10 +124,10 @@ function failureMessage(type, { // Return true or false if a build is considered failing or not function isFailing({ - attributes: { state, 'total-comparisons-diff': diffs } -}, { failOnChanges }) { + attributes: { state, 'total-comparisons-diff': diffs } = {} +} = {}, { failOnChanges }) { // not pending and not processing - return state !== 'pending' && state !== 'processing' && + return state != null && state !== 'pending' && state !== 'processing' && // not finished or finished with diffs (state !== 'finished' || (failOnChanges && !!diffs)); } diff --git a/packages/cli-build/test/wait.test.js b/packages/cli-build/test/wait.test.js index 57c9ab0ab..41a5e3d45 100644 --- a/packages/cli-build/test/wait.test.js +++ b/packages/cli-build/test/wait.test.js @@ -41,10 +41,25 @@ describe('percy build:wait', () => { expect(logger.stdout).toEqual([]); expect(logger.stderr).toEqual([ - '[percy] Error: Missing build ID or commit SHA' + '[percy] Error: Missing project path or build ID' ]); }); + it('logs while waiting for a matching build', async () => { + api + .reply('/projects/foo/bar/builds?filter[sha]=sha123', () => [200, { data: [] }]) + .reply('/projects/foo/bar/builds?filter[sha]=sha123', () => [200, { + data: [build({ 'total-comparisons-finished': 72, state: 'finished' }).data] + }]); + + await wait(['--project=foo/bar', '--commit=sha123', '--timeout=500', '--interval=50']); + + expect(logger.stderr).toEqual([]); + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + '[percy] Waiting for build...' + ])); + }); + it('logs while recieving snapshots', async () => { api .reply('/builds/123', () => [200, build({ diff --git a/packages/client/src/client.js b/packages/client/src/client.js index def0ffbdf..2b5eeb468 100644 --- a/packages/client/src/client.js +++ b/packages/client/src/client.js @@ -195,19 +195,21 @@ export class PercyClient { timeout = 10 * 60 * 1000, interval = 1000 }, onProgress) { - if (commit && !project) { + if (!project && commit) { throw new Error('Missing project path for commit'); - } else if (!commit && !build) { - throw new Error('Missing build ID or commit SHA'); + } else if (!project && !build) { + throw new Error('Missing project path or build ID'); } else if (project) { validateProjectPath(project); } + commit ||= this.env.git.sha; + if (!build && !commit) throw new Error('Missing build commit'); let sha = commit && (git(`rev-parse ${commit}`) || commit); let fetchData = async () => build ? (await this.getBuild(build)).data - : (await this.getBuilds(project, { sha })).data[0]; + : (await this.getBuilds(project, { sha })).data?.[0]; this.log.debug(`Waiting for build ${build || `${project} (${commit})`}...`); @@ -225,7 +227,7 @@ export class PercyClient { // no new data within the timeout } else if (Date.now() - t >= timeout) { - throw new Error('Timeout exceeded without an update'); + throw new Error(state == null ? 'Build not found' : 'Timeout exceeded with no updates'); } // call progress every update after the first update diff --git a/packages/client/test/client.test.js b/packages/client/test/client.test.js index 3ef61f1ad..adf833a2f 100644 --- a/packages/client/test/client.test.js +++ b/packages/client/test/client.test.js @@ -277,16 +277,23 @@ describe('PercyClient', () => { }); describe('#waitForBuild()', () => { - it('throws when missing a build or commit sha', () => { - expect(() => client.waitForBuild({})) - .toThrowError('Missing build ID or commit SHA'); + it('throws when missing a project or build', () => { + expect(() => client.waitForBuild({ commit: null })) + .toThrowError('Missing project path or build ID'); }); - it('throws when missing a project with a commit sha', () => { + it('throws when missing a project with a commit', () => { expect(() => client.waitForBuild({ commit: '...' })) .toThrowError('Missing project path for commit'); }); + it('throws when missing a commit for a project', () => { + Object.defineProperty(client.env, 'git', { value: { sha: null } }); + + expect(() => client.waitForBuild({ project: 'foo/bar' })) + .toThrowError('Missing build commit'); + }); + it('throws when using an invalid project path', () => { expect(() => client.waitForBuild({ commit: '...', project: 'test' })) .toThrowError('Invalid project path. Expected "org/project" but received "test"'); @@ -320,7 +327,18 @@ describe('PercyClient', () => { }]); await expectAsync(client.waitForBuild({ build: '123', timeout: 1500, interval: 50 })) - .toBeRejectedWithError('Timeout exceeded without an update'); + .toBeRejectedWithError('Timeout exceeded with no updates'); + }); + + it('throws when no build is found within the timeout', async () => { + api.reply('/projects/foo/bar/builds?filter[sha]=sha123', () => [200, { data: [] }]); + + await expectAsync(client.waitForBuild({ + project: 'foo/bar', + commit: 'sha123', + timeout: 500, + interval: 50 + })).toBeRejectedWithError('Build not found'); }); it('resolves when the build completes', async () => { @@ -347,7 +365,7 @@ describe('PercyClient', () => { data: [{ attributes: { state: 'finished' } }] }]); - await expectAsync(client.waitForBuild({ project: 'foo/bar', commit: 'HEAD', interval: 50 })) + await expectAsync(client.waitForBuild({ project: 'foo/bar', interval: 50 })) .toBeResolvedTo({ data: { attributes: { state: 'finished' } } }); });