From 5ad071cf9ce1ee726a76d9cfc685338eff3772f6 Mon Sep 17 00:00:00 2001 From: Jigar wala Date: Thu, 6 Apr 2023 12:22:53 +0530 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20increase=20interval=20for=20polling?= =?UTF-8?q?=20build=20(#1228)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * :boom: increase interval for polling build * PR suggestions * remove log line * fix tests * fix tests * refactor * update warning message --- packages/cli-build/README.md | 2 +- packages/cli-build/src/wait.js | 2 +- packages/cli-build/test/wait.test.js | 6 +++--- packages/client/README.md | 2 +- packages/client/src/client.js | 8 +++++++- packages/client/test/client.test.js | 16 +++++++++++++--- 6 files changed, 26 insertions(+), 10 deletions(-) diff --git a/packages/cli-build/README.md b/packages/cli-build/README.md index b71097916..a6f2ab2d3 100644 --- a/packages/cli-build/README.md +++ b/packages/cli-build/README.md @@ -36,7 +36,7 @@ Options: -p, --project Build project slug, requires '--commit' -c, --commit Build commit sha, requires '--project' -t, --timeout Timeout before exiting without updates, defaults to 10 minutes - -i, --interval Interval at which to poll for updates, defaults to 1 second + -i, --interval Interval at which to poll for updates, defaults to 10 second -f, --fail-on-changes Exit with an error when diffs are found --pass-if-approved Doesn't Exit with an error if the build is approved, requires '--fail-on-changes' diff --git a/packages/cli-build/src/wait.js b/packages/cli-build/src/wait.js index 8704ab74c..6483fe038 100644 --- a/packages/cli-build/src/wait.js +++ b/packages/cli-build/src/wait.js @@ -28,7 +28,7 @@ export const wait = command('wait', { short: 't' }, { name: 'interval', - description: 'Interval at which to poll for updates, defaults to 1 second', + description: 'Interval at which to poll for updates, defaults to 10 second', type: 'ms', parse: Number, short: 'i' diff --git a/packages/cli-build/test/wait.test.js b/packages/cli-build/test/wait.test.js index cd8b65227..b3db5161c 100644 --- a/packages/cli-build/test/wait.test.js +++ b/packages/cli-build/test/wait.test.js @@ -54,7 +54,7 @@ describe('percy build:wait', () => { await wait(['--project=foo/bar', '--commit=sha123', '--timeout=500', '--interval=50']); - expect(logger.stderr).toEqual([]); + expect(logger.stderr).toEqual(['[percy] Ignoring interval since it cannot be less than 1000ms.']); expect(logger.stdout).toEqual(jasmine.arrayContaining([ '[percy] Waiting for build...' ])); @@ -72,7 +72,7 @@ describe('percy build:wait', () => { await wait(['--build=123', '--interval=50']); - expect(logger.stderr).toEqual([]); + expect(logger.stderr).toEqual(['[percy] Ignoring interval since it cannot be less than 1000ms.']); expect(logger.stdout).toEqual(jasmine.arrayContaining([ '[percy] Recieving snapshots...' ])); @@ -95,7 +95,7 @@ describe('percy build:wait', () => { await wait(['--build=123', '--interval=50']); - expect(logger.stderr).toEqual([]); + expect(logger.stderr).toEqual(['[percy] Ignoring interval since it cannot be less than 1000ms.']); expect(logger.stdout).toEqual(jasmine.arrayContaining([ '[percy] Processing 18 snapshots - 16 of 72 comparisons finished...', '[percy] Processing 18 snapshots - 32 of 72 comparisons finished...', diff --git a/packages/client/README.md b/packages/client/README.md index 7ef7ddc5f..b35e43ff2 100644 --- a/packages/client/README.md +++ b/packages/client/README.md @@ -164,4 +164,4 @@ await client.waitForBuild({ - `commit` — Commit SHA (**required** when missing `build`) - `project` — Project slug (**required** when using `commit`) - `timeout` — Timeout in milliseconds to wait with no updates (**default** `10 * 60 * 1000`) -- `interval` — Interval in miliseconds to check for updates (**default** `1000`) +- `interval` — Interval in miliseconds to check for updates (**default** `10000`) diff --git a/packages/client/src/client.js b/packages/client/src/client.js index f1b58df20..f14b170b9 100644 --- a/packages/client/src/client.js +++ b/packages/client/src/client.js @@ -15,6 +15,8 @@ import { // Default client API URL can be set with an env var for API development const { PERCY_CLIENT_API_URL = 'https://percy.io/api/v1' } = process.env; const pkg = getPackageJSON(import.meta.url); +// minimum polling interval milliseconds +const MIN_POLLING_INTERVAL = 1_000; // Validate ID arguments function validateId(type, id) { @@ -195,8 +197,12 @@ export class PercyClient { project, commit, timeout = 10 * 60 * 1000, - interval = 1000 + interval = 10_000 }, onProgress) { + if (interval < MIN_POLLING_INTERVAL) { + this.log.warn(`Ignoring interval since it cannot be less than ${MIN_POLLING_INTERVAL}ms.`); + interval = MIN_POLLING_INTERVAL; + } if (!project && commit) { throw new Error('Missing project path for commit'); } else if (!project && !build) { diff --git a/packages/client/test/client.test.js b/packages/client/test/client.test.js index 3649e5e73..e7b3b69d7 100644 --- a/packages/client/test/client.test.js +++ b/packages/client/test/client.test.js @@ -9,7 +9,7 @@ describe('PercyClient', () => { let client; beforeEach(async () => { - await logger.mock(); + await logger.mock({ level: 'debug' }); await api.mock(); client = new PercyClient({ @@ -55,7 +55,7 @@ describe('PercyClient', () => { }] })).toBeResolved(); - expect(logger.stderr).toEqual(['[percy] Warning: Missing `clientInfo` and/or `environmentInfo` properties']); + expect(logger.stderr).toEqual(jasmine.arrayContaining(['[percy:client] Warning: Missing `clientInfo` and/or `environmentInfo` properties'])); }); it('it logs a debug warning when partial info is passed', async () => { @@ -77,7 +77,7 @@ describe('PercyClient', () => { }] })).toBeResolved(); - expect(logger.stderr).toEqual(['[percy] Warning: Missing `clientInfo` and/or `environmentInfo` properties']); + expect(logger.stderr).toEqual(jasmine.arrayContaining(['[percy:client] Warning: Missing `clientInfo` and/or `environmentInfo` properties'])); }); it('does not duplicate or include empty client and environment information', () => { @@ -365,6 +365,16 @@ describe('PercyClient', () => { .toThrowError('Invalid project path. Expected "org/project" but received "test"'); }); + it('warns when interval is less than 1000ms', async () => { + api + .reply('/builds/123', () => [200, { + data: { attributes: { state: 'finished' } } + }]); + + await client.waitForBuild({ build: '123', interval: 50 }); + expect(logger.stderr).toEqual(jasmine.arrayContaining(['[percy:client] Ignoring interval since it cannot be less than 1000ms.'])); + }); + it('invokes the callback when data changes while waiting', async () => { let progress = 0;