🐛 @percy/env parallel fixes (#129)

* 🐛 Fix parallel total and nonce collection

Parallel total - `parseInt` does not throw but can return NaN, Infinity, or hex values; which should
be checked and removed if present.

Parallel nonce - the API does not like it when a nonce is provided without a total, which happens by
default when somebody has parallel CI jobs even without parallel Percy builds.

* ✅ Adjust parallel nonce tests to account for parallel total

packages/env/src/environment.js

@@ -198,9 +198,13 @@ export default class PercyEnvironment {
     return pr || null;
   }
 
-  // parallel nonce & total
+  // parallel total & nonce
   get parallel() {
-    let nonce = (() => {
+    let total = parseInt(this.vars.PERCY_PARALLEL_TOTAL, 10);
+    if (!Number.isInteger(total)) total = null;
+
+    // no nonce if no total
+    let nonce = total && (() => {
       if (this.vars.PERCY_PARALLEL_NONCE) {
         return this.vars.PERCY_PARALLEL_NONCE;
       }
@@ -238,13 +242,9 @@ export default class PercyEnvironment {
       }
     })();
 
-    let total = (() => {
-      try { return parseInt(this.vars.PERCY_PARALLEL_TOTAL); } catch {}
-    })();
-
     return {
-      nonce: nonce || null,
-      total: total || null
+      total: total || null,
+      nonce: nonce || null
     };
   }
 

packages/env/test/appveyor.test.js

@@ -6,10 +6,11 @@ describe('Appveyor', () => {
 
   beforeEach(() => {
     env = new PercyEnvironment({
-      APPVEYOR: 'True',
+      PERCY_PARALLEL_TOTAL: '-1',
       APPVEYOR_BUILD_ID: 'appveyor-build-id',
       APPVEYOR_REPO_COMMIT: 'appveyor-commit-sha',
-      APPVEYOR_REPO_BRANCH: 'appveyor-branch'
+      APPVEYOR_REPO_BRANCH: 'appveyor-branch',
+      APPVEYOR: 'True'
     });
   });
 
@@ -21,7 +22,7 @@ describe('Appveyor', () => {
     expect(env).toHaveProperty('target.branch', null);
     expect(env).toHaveProperty('pullRequest', null);
     expect(env).toHaveProperty('parallel.nonce', 'appveyor-build-id');
-    expect(env).toHaveProperty('parallel.total', null);
+    expect(env).toHaveProperty('parallel.total', -1);
   });
 
   it('has the correct properties for PR builds', () => {

packages/env/test/azure.test.js

@@ -6,6 +6,7 @@ describe('Azure', () => {
 
   beforeEach(() => {
     env = new PercyEnvironment({
+      PERCY_PARALLEL_TOTAL: '-1',
       BUILD_BUILDID: 'azure-build-id',
       BUILD_SOURCEVERSION: 'azure-commit-sha',
       BUILD_SOURCEBRANCHNAME: 'azure-branch',
@@ -21,7 +22,7 @@ describe('Azure', () => {
     expect(env).toHaveProperty('target.branch', null);
     expect(env).toHaveProperty('pullRequest', null);
     expect(env).toHaveProperty('parallel.nonce', 'azure-build-id');
-    expect(env).toHaveProperty('parallel.total', null);
+    expect(env).toHaveProperty('parallel.total', -1);
   });
 
   it('has the correct properties for PR builds', () => {

packages/env/test/bitbucket.test.js

@@ -6,6 +6,7 @@ describe('Bitbucket', () => {
 
   beforeEach(() => {
     env = new PercyEnvironment({
+      PERCY_PARALLEL_TOTAL: '-1',
       BITBUCKET_BUILD_NUMBER: 'bitbucket-build-number',
       BITBUCKET_COMMIT: 'bitbucket-commit-sha',
       BITBUCKET_BRANCH: 'bitbucket-branch',
@@ -21,6 +22,6 @@ describe('Bitbucket', () => {
     expect(env).toHaveProperty('target.branch', null);
     expect(env).toHaveProperty('pullRequest', '981');
     expect(env).toHaveProperty('parallel.nonce', 'bitbucket-build-number');
-    expect(env).toHaveProperty('parallel.total', null);
+    expect(env).toHaveProperty('parallel.total', -1);
   });
 });

packages/env/test/buildkite.test.js

@@ -6,11 +6,12 @@ describe('Buildkite', () => {
 
   beforeEach(() => {
     env = new PercyEnvironment({
-      BUILDKITE: 'true',
+      PERCY_PARALLEL_TOTAL: '-1',
       BUILDKITE_COMMIT: 'buildkite-commit-sha',
       BUILDKITE_BRANCH: 'buildkite-branch',
       BUILDKITE_PULL_REQUEST: 'false',
-      BUILDKITE_BUILD_ID: 'buildkite-build-id'
+      BUILDKITE_BUILD_ID: 'buildkite-build-id',
+      BUILDKITE: 'true'
     });
   });
 
@@ -22,7 +23,7 @@ describe('Buildkite', () => {
     expect(env).toHaveProperty('target.branch', null);
     expect(env).toHaveProperty('pullRequest', null);
     expect(env).toHaveProperty('parallel.nonce', 'buildkite-build-id');
-    expect(env).toHaveProperty('parallel.total', null);
+    expect(env).toHaveProperty('parallel.total', -1);
   });
 
   it('has the correct properties for PR builds', () => {

packages/env/test/circle.test.js

@@ -6,11 +6,12 @@ describe('CircleCI', () => {
 
   beforeEach(() => {
     env = new PercyEnvironment({
-      CIRCLECI: 'true',
+      PERCY_PARALLEL_TOTAL: '-1',
       CIRCLE_BRANCH: 'circle-branch',
       CIRCLE_SHA1: 'circle-commit-sha',
       CI_PULL_REQUESTS: 'https://github.com/owner/repo-name/pull/123',
-      CIRCLE_BUILD_NUM: 'build-number'
+      CIRCLE_BUILD_NUM: 'build-number',
+      CIRCLECI: 'true'
     });
   });
 
@@ -22,7 +23,7 @@ describe('CircleCI', () => {
     expect(env).toHaveProperty('target.branch', null);
     expect(env).toHaveProperty('pullRequest', '123');
     expect(env).toHaveProperty('parallel.nonce', 'build-number');
-    expect(env).toHaveProperty('parallel.total', null);
+    expect(env).toHaveProperty('parallel.total', -1);
   });
 
   it('has the correct parallel nonce in 2.x', () => {

packages/env/test/codeship.test.js

@@ -6,12 +6,13 @@ describe('CodeShip', () => {
 
   beforeEach(function() {
     env = new PercyEnvironment({
-      CI_NAME: 'codeship',
+      PERCY_PARALLEL_TOTAL: '-1',
       CI_BRANCH: 'codeship-branch',
       CI_BUILD_NUMBER: 'codeship-build-number',
       CI_BUILD_ID: 'codeship-build-id',
       CI_COMMIT_ID: 'codeship-commit-sha',
-      CI_PULL_REQUEST: 'false'
+      CI_PULL_REQUEST: 'false',
+      CI_NAME: 'codeship'
     });
   });
 
@@ -23,7 +24,7 @@ describe('CodeShip', () => {
     expect(env).toHaveProperty('target.branch', null);
     expect(env).toHaveProperty('pullRequest', null);
     expect(env).toHaveProperty('parallel.nonce', 'codeship-build-number');
-    expect(env).toHaveProperty('parallel.total', null);
+    expect(env).toHaveProperty('parallel.total', -1);
   });
 
   it('has nonce fallback for CodeShip Pro', () => {

packages/env/test/defaults.test.js

@@ -81,7 +81,7 @@ describe('Defaults', () => {
     expect(env).toHaveProperty('ci', 'bitbucket');
     expect(env).toHaveProperty('commit', 'bitbucket-commit-sha');
     expect(env).toHaveProperty('git.sha', 'fully-valid-git-sha');
-    expect(env).toHaveProperty('parallel.nonce', 'bitbucket-build-number');
+    expect(env).toHaveProperty('parallel.nonce', null);
   });
 
   it('can be overridden with PERCY env vars', () => {
@@ -132,6 +132,23 @@ describe('Defaults', () => {
     expect(env).toHaveProperty('git.message', 'percy git commit');
   });
 
+  it('does not collect parallel nonce with invalid or no parallel total', () => {
+    env = new PercyEnvironment({
+      PERCY_PARALLEL_NONCE: 'percy-nonce',
+      PERCY_PARALLEL_TOTAL: 'invalid'
+    });
+
+    expect(env).toHaveProperty('parallel.nonce', null);
+    expect(env).toHaveProperty('parallel.total', null);
+
+    env = new PercyEnvironment({
+      PERCY_PARALLEL_NONCE: 'percy-nonce'
+    });
+
+    expect(env).toHaveProperty('parallel.nonce', null);
+    expect(env).toHaveProperty('parallel.total', null);
+  });
+
   it('falls back to GIT env vars with missing or invalid git commit data', () => {
     mockgit.commit(() => 'missing or invalid');
     mockgit.branch(() => 'mock branch');

packages/env/test/drone.test.js

@@ -10,6 +10,7 @@ describe('Drone', () => {
       DRONE_COMMIT: 'drone-commit-sha',
       DRONE_BRANCH: 'drone-branch',
       DRONE_BUILD_NUMBER: 'drone-build-number',
+      PERCY_PARALLEL_TOTAL: '-1',
       CI_PULL_REQUEST: '123'
     });
   });
@@ -22,6 +23,6 @@ describe('Drone', () => {
     expect(env).toHaveProperty('target.branch', null);
     expect(env).toHaveProperty('pullRequest', '123');
     expect(env).toHaveProperty('parallel.nonce', 'drone-build-number');
-    expect(env).toHaveProperty('parallel.total', null);
+    expect(env).toHaveProperty('parallel.total', -1);
   });
 });

packages/env/test/gitlab.test.js

@@ -10,7 +10,8 @@ describe('GitLab', () => {
       CI_COMMIT_SHA: 'gitlab-commit-sha',
       CI_COMMIT_REF_NAME: 'gitlab-branch',
       CI_PIPELINE_ID: 'gitlab-job-id',
-      CI_SERVER_VERSION: '8.14.3-ee'
+      CI_SERVER_VERSION: '8.14.3-ee',
+      PERCY_PARALLEL_TOTAL: '-1'
     });
   });
 
@@ -23,7 +24,7 @@ describe('GitLab', () => {
     expect(env).toHaveProperty('target.branch', null);
     expect(env).toHaveProperty('pullRequest', null);
     expect(env).toHaveProperty('parallel.nonce', 'gitlab-job-id');
-    expect(env).toHaveProperty('parallel.total', null);
+    expect(env).toHaveProperty('parallel.total', -1);
   });
 
   it('has the correct properties for PR builds', () => {

packages/env/test/heroku.test.js

@@ -6,6 +6,7 @@ describe('Heroku', () => {
 
   beforeEach(() => {
     env = new PercyEnvironment({
+      PERCY_PARALLEL_TOTAL: '-1',
       HEROKU_TEST_RUN_COMMIT_VERSION: 'heroku-commit-sha',
       HEROKU_TEST_RUN_BRANCH: 'heroku-branch',
       HEROKU_TEST_RUN_ID: 'heroku-test-run-id',
@@ -21,6 +22,6 @@ describe('Heroku', () => {
     expect(env).toHaveProperty('target.branch', null);
     expect(env).toHaveProperty('pullRequest', '123');
     expect(env).toHaveProperty('parallel.nonce', 'heroku-test-run-id');
-    expect(env).toHaveProperty('parallel.total', null);
+    expect(env).toHaveProperty('parallel.total', -1);
   });
 });

packages/env/test/jenkins.test.js

@@ -10,7 +10,8 @@ describe('Jenkins', () => {
       JENKINS_URL: 'http://jenkins.local/',
       GIT_COMMIT: 'jenkins-commit-sha',
       GIT_BRANCH: 'jenkins-branch',
-      BUILD_TAG: 'xxxx-project-branch-build-number-123'
+      BUILD_TAG: 'xxxx-project-branch-build-number-123',
+      PERCY_PARALLEL_TOTAL: '-1'
     });
   });
 
@@ -22,7 +23,7 @@ describe('Jenkins', () => {
     expect(env).toHaveProperty('target.branch', null);
     expect(env).toHaveProperty('pullRequest', null);
     expect(env).toHaveProperty('parallel.nonce', '321-rebmun-dliub-hcnarb-tcejorp-xxxx');
-    expect(env).toHaveProperty('parallel.total', null);
+    expect(env).toHaveProperty('parallel.total', -1);
   });
 
   it('has the correct properties for PR builds', () => {
@@ -65,7 +66,8 @@ describe('Jenkins', () => {
         BUILD_NUMBER: '111',
         ghprbPullId: '256',
         ghprbActualCommit: 'jenkins-prb-commit-sha',
-        ghprbSourceBranch: 'jenkins-prb-branch'
+        ghprbSourceBranch: 'jenkins-prb-branch',
+        PERCY_PARALLEL_TOTAL: '-1'
       });
     });
 
@@ -77,7 +79,7 @@ describe('Jenkins', () => {
       expect(env).toHaveProperty('target.branch', null);
       expect(env).toHaveProperty('pullRequest', '256');
       expect(env).toHaveProperty('parallel.nonce', '111');
-      expect(env).toHaveProperty('parallel.total', null);
+      expect(env).toHaveProperty('parallel.total', -1);
     });
 
     it('has the correct fallback when PRB commit var is missing', () => {

packages/env/test/probo.test.js

@@ -10,7 +10,8 @@ describe('Probo', () => {
       BUILD_ID: 'probo-build-id',
       COMMIT_REF: 'probo-commit-sha',
       BRANCH_NAME: 'probo-branch',
-      PULL_REQUEST_LINK: 'https://github.com/owner/repo-name/pull/123'
+      PULL_REQUEST_LINK: 'https://github.com/owner/repo-name/pull/123',
+      PERCY_PARALLEL_TOTAL: '-1'
     });
   });
 
@@ -22,6 +23,6 @@ describe('Probo', () => {
     expect(env).toHaveProperty('target.branch', null);
     expect(env).toHaveProperty('pullRequest', '123');
     expect(env).toHaveProperty('parallel.nonce', 'probo-build-id');
-    expect(env).toHaveProperty('parallel.total', null);
+    expect(env).toHaveProperty('parallel.total', -1);
   });
 });

packages/env/test/semaphore.test.js

@@ -11,6 +11,7 @@ describe('Semaphore', () => {
       REVISION: 'semaphore-commit-sha',
       SEMAPHORE_BRANCH_ID: 'semaphore-branch-id',
       SEMAPHORE_BUILD_NUMBER: 'semaphore-build-number',
+      PERCY_PARALLEL_TOTAL: '-1',
       PULL_REQUEST_NUMBER: '123'
     });
   });
@@ -24,7 +25,7 @@ describe('Semaphore', () => {
     expect(env).toHaveProperty('target.branch', null);
     expect(env).toHaveProperty('pullRequest', '123');
     expect(env).toHaveProperty('parallel.nonce', 'semaphore-branch-id/semaphore-build-number');
-    expect(env).toHaveProperty('parallel.total', null);
+    expect(env).toHaveProperty('parallel.total', -1);
   });
 
   describe('Semaphore 2.0', () => {
@@ -33,7 +34,8 @@ describe('Semaphore', () => {
         SEMAPHORE: 'true',
         SEMAPHORE_GIT_SHA: 'semaphore-2-sha',
         SEMAPHORE_GIT_BRANCH: 'semaphore-2-branch',
-        SEMAPHORE_WORKFLOW_ID: 'semaphore-2-workflow-id'
+        SEMAPHORE_WORKFLOW_ID: 'semaphore-2-workflow-id',
+        PERCY_PARALLEL_TOTAL: '-1'
       });
     });
 
@@ -46,7 +48,7 @@ describe('Semaphore', () => {
       expect(env).toHaveProperty('target.branch', null);
       expect(env).toHaveProperty('pullRequest', null);
       expect(env).toHaveProperty('parallel.nonce', 'semaphore-2-workflow-id');
-      expect(env).toHaveProperty('parallel.total', null);
+      expect(env).toHaveProperty('parallel.total', -1);
     });
 
     it('has the correct properties for PR builds', () => {
@@ -57,7 +59,8 @@ describe('Semaphore', () => {
         SEMAPHORE_GIT_BRANCH: 'semaphore-2-branch',
         SEMAPHORE_GIT_PR_BRANCH: 'semaphore-2-pr-branch',
         SEMAPHORE_GIT_PR_NUMBER: '50',
-        SEMAPHORE_WORKFLOW_ID: 'semaphore-2-workflow-id'
+        SEMAPHORE_WORKFLOW_ID: 'semaphore-2-workflow-id',
+        PERCY_PARALLEL_TOTAL: '-1'
       });
 
       expect(env).toHaveProperty('ci', 'semaphore');
@@ -68,7 +71,7 @@ describe('Semaphore', () => {
       expect(env).toHaveProperty('target.branch', null);
       expect(env).toHaveProperty('pullRequest', '50');
       expect(env).toHaveProperty('parallel.nonce', 'semaphore-2-workflow-id');
-      expect(env).toHaveProperty('parallel.total', null);
+      expect(env).toHaveProperty('parallel.total', -1);
     });
   });
 });

packages/env/test/travis.test.js

@@ -11,7 +11,8 @@ describe('Travis', () => {
       TRAVIS_PULL_REQUEST_BRANCH: '',
       TRAVIS_COMMIT: 'travis-commit-sha',
       TRAVIS_BRANCH: 'travis-branch',
-      TRAVIS_BUILD_NUMBER: 'build-number'
+      TRAVIS_BUILD_NUMBER: 'build-number',
+      PERCY_PARALLEL_TOTAL: '-1'
     });
   });
 
@@ -23,7 +24,7 @@ describe('Travis', () => {
     expect(env).toHaveProperty('target.branch', null);
     expect(env).toHaveProperty('pullRequest', null);
     expect(env).toHaveProperty('parallel.nonce', 'build-number');
-    expect(env).toHaveProperty('parallel.total', null);
+    expect(env).toHaveProperty('parallel.total', -1);
   });
 
   it('has the correct properties for PR builds', () => {