fix(cli): use correct baseBranch for healthcheck

fixes #485
GoogleChrome · Nov 10, 2020 · c0bef27 · c0bef27
1 parent 543658b
commit c0bef27
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 16 deletions.
diff --git a/packages/cli/src/healthcheck/healthcheck.js b/packages/cli/src/healthcheck/healthcheck.js
@@ -82,7 +82,11 @@ const checks = [
     failureLabel: 'Ancestor hash not determinable',
     // the test only makes sense if they've configured an LHCI server
     shouldTest: opts => Boolean(opts.serverBaseUrl && opts.token),
-    test: () => getAncestorHash().length > 0,
+    test: async opts => {
+      const client = getApiClient(opts);
+      const project = await client.findProjectByToken(opts.token || '');
+      return getAncestorHash('HEAD', (project && project.baseBranch) || 'master').length > 0;
+    },
   },
   {
     id: 'lhciServer',

diff --git a/packages/cli/src/upload/upload.js b/packages/cli/src/upload/upload.js
@@ -27,10 +27,9 @@ const {
   getCommitMessage,
   getAuthor,
   getAvatarUrl,
-  getAncestorHashForBase,
-  getAncestorHashForBranch,
   getGitHubRepoSlug,
   getCurrentBranchSafe,
+  getAncestorHash,
 } = require('@lhci/utils/src/build-context.js');
 
 /** @param {string} message */
@@ -405,8 +404,7 @@ async function runLHCITarget(options) {
   const baseBranch = project.baseBranch || 'master';
   const hash = getCurrentHash();
   const branch = getCurrentBranch();
-  const ancestorHash =
-    branch === baseBranch ? getAncestorHashForBase() : getAncestorHashForBranch('HEAD', baseBranch);
+  const ancestorHash = getAncestorHash('HEAD', baseBranch);
 
   const build = await api.createBuild({
     projectId: project.id,

diff --git a/packages/utils/src/build-context.js b/packages/utils/src/build-context.js
@@ -282,10 +282,10 @@ function getGravatarUrlFromEmail(email) {
 }
 
 /**
- * @param {string} [hash]
+ * @param {string} hash
  * @return {string}
  */
-function getAncestorHashForBase(hash = 'HEAD') {
+function getAncestorHashForBase(hash) {
   const result = childProcess.spawnSync('git', ['rev-parse', `${hash}^`], {encoding: 'utf8'});
   // Ancestor hash is optional, so do not throw if it can't be computed.
   // See https://github.com/GoogleChrome/lighthouse-ci/issues/36
@@ -295,11 +295,11 @@ function getAncestorHashForBase(hash = 'HEAD') {
 }
 
 /**
- * @param {string} [hash]
- * @param {string} [baseBranch]
+ * @param {string} hash
+ * @param {string} baseBranch
  * @return {string}
  */
-function getAncestorHashForBranch(hash = 'HEAD', baseBranch = 'master') {
+function getAncestorHashForBranch(hash, baseBranch) {
   const result = runCommandsUntilFirstSuccess([
     ['git', ['merge-base', hash, `origin/${baseBranch}`]],
     ['git', ['merge-base', hash, baseBranch]],
@@ -313,19 +313,20 @@ function getAncestorHashForBranch(hash = 'HEAD', baseBranch = 'master') {
 }
 
 /**
- * @param {string} [hash]
+ * @param {string} hash
+ * @param {string} baseBranch
  * @return {string}
  */
-function getAncestorHash(hash = 'HEAD') {
+function getAncestorHash(hash, baseBranch) {
   const envHash = getEnvVarIfSet([
     // Manual override
     'LHCI_BUILD_CONTEXT__ANCESTOR_HASH',
   ]);
   if (envHash) return envHash;
 
-  return getCurrentBranch() === 'master'
+  return getCurrentBranch() === baseBranch
     ? getAncestorHashForBase(hash)
-    : getAncestorHashForBranch(hash);
+    : getAncestorHashForBranch(hash, baseBranch);
 }
 
 /** @param {string|undefined} apiHost */

diff --git a/packages/utils/test/build-context.test.js b/packages/utils/test/build-context.test.js
@@ -192,6 +192,31 @@ describe('build-context.js', () => {
     });
   });
 
+  describe('#getAncestorHash()', () => {
+    it('should use for base', () => {
+      process.env.LHCI_BUILD_CONTEXT__CURRENT_BRANCH = 'main';
+      expect(buildContext.getAncestorHash(hash, 'main')).toEqual(
+        'ec95bc8ad992c9d68845040d612fbbbe94ad7f13'
+      );
+    });
+
+    it('should use for branch', () => {
+      process.env.LHCI_BUILD_CONTEXT__CURRENT_BRANCH = 'feature-branch';
+      expect(buildContext.getAncestorHash('HEAD', 'v0.3.12')).toEqual(versionHash);
+    });
+
+    it('should return empty string when it fails', () => {
+      process.env.LHCI_BUILD_CONTEXT__CURRENT_BRANCH = 'main';
+      expect(buildContext.getAncestorHash('random' + Math.random())).toEqual('');
+    });
+
+    it('should respect env override', () => {
+      process.env.LHCI_BUILD_CONTEXT__CURRENT_BRANCH = 'main';
+      process.env.LHCI_BUILD_CONTEXT__ANCESTOR_HASH = '123456789';
+      expect(buildContext.getAncestorHash(hash, 'foo')).toEqual('123456789');
+    });
+  });
+
   describe('#getAncestorHashForBase()', () => {
     it('should work', () => {
       expect(buildContext.getAncestorHashForBase(hash)).toEqual(
@@ -212,11 +237,11 @@ describe('build-context.js', () => {
   describe('#getAncestorHashForBranch()', () => {
     it('should work', () => {
       // the merge-base of master with itself is just itself.
-      expect(buildContext.getAncestorHashForBranch(hash)).toEqual(hash);
+      expect(buildContext.getAncestorHashForBranch(hash, 'master')).toEqual(hash);
     });
 
     it('should work for alternate branches', () => {
-      // the merge-base of master with itself is just itself.
+      // the merge-base of any branch with an older version is that older version
       expect(buildContext.getAncestorHashForBranch('HEAD', 'v0.3.12')).toEqual(versionHash);
     });