✨ increase interval for polling build (#1228)

* 💥 increase interval for polling build

* PR suggestions

* remove log line

* fix tests

* fix tests

* refactor

* update warning message

packages/cli-build/README.md

@@ -36,7 +36,7 @@ Options:
   -p, --project <slug>   Build project slug, requires '--commit'
   -c, --commit <sha>     Build commit sha, requires '--project'
   -t, --timeout <ms>     Timeout before exiting without updates, defaults to 10 minutes
-  -i, --interval <ms>    Interval at which to poll for updates, defaults to 1 second
+  -i, --interval <ms>    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'
 

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'

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...',

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`)

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) {

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;