From c18a9d5c7990d9f8bd76d54b3cc85d99c109cf3a Mon Sep 17 00:00:00 2001
From: George Kalpakas <kalpakas.g@gmail.com>
Date: Fri, 11 Dec 2020 19:02:45 +0200
Subject: [PATCH] ci: reduce flakiness of docs-infra `deploy-to-firebase.js`
 tests (#40088)

The `deploy-to-firebase.js` tests rely on git info retrieved from the
`angular/angular` repository (via `git ls-remote ...`).

Previously, different calls to `git ls-remote ...` could return
different values if a new commit was pushed or a new branch was created
during test execution, resulting in errors ([example CI failure][1]).

This commit makes the tests more stable by memoizing the result of
`git ls-remote ...` and returning the same result for subsequent calls
with the same arguments (even if meanwhile the remote has been updated).

[1]: https://circleci.com/gh/angular/angular/877626

PR Close #40088
---
 aio/scripts/deploy-to-firebase.js      | 30 ++++++++++++++++++++------
 aio/scripts/deploy-to-firebase.spec.js |  9 ++++++--
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/aio/scripts/deploy-to-firebase.js b/aio/scripts/deploy-to-firebase.js
index 735a8d740a..e5c5f460ee 100644
--- a/aio/scripts/deploy-to-firebase.js
+++ b/aio/scripts/deploy-to-firebase.js
@@ -12,6 +12,7 @@ set('-e');
 // Constants
 const REPO_SLUG = 'angular/angular';
 const NG_REMOTE_URL = `https://github.com/${REPO_SLUG}.git`;
+const GIT_REMOTE_REFS_CACHE = new Map();
 
 // Exports
 module.exports = {
@@ -55,7 +56,6 @@ if (require.main === module) {
       }
     }
   });
-
 }
 
 // Helpers
@@ -260,13 +260,29 @@ function deploy(data) {
   postDeployActions.forEach(fn => fn(data));
 }
 
-function getRemoteRefs(refOrPattern, remote = NG_REMOTE_URL) {
-  return exec(`git ls-remote ${remote} ${refOrPattern}`, {silent: true}).trim().split('\n');
+function getRemoteRefs(refOrPattern, {remote = NG_REMOTE_URL, retrieveFromCache = true} = {}) {
+  // If remote refs for the same `refOrPattern` and `remote` have been requested before, return the
+  // cached results. This improves the performance and ensures a more stable behavior.
+  //
+  // NOTE:
+  // This shouldn't make any difference during normal execution (since there are no duplicate
+  // requests atm), but makes the tests more stable (for example, avoiding errors caused by pushing
+  // a new commit on a branch while the tests execute, which would cause `getLatestCommit()` to
+  // return a different value).
+  const cmd = `git ls-remote ${remote} ${refOrPattern}`;
+  const result = (retrieveFromCache && GIT_REMOTE_REFS_CACHE.has(cmd))
+      ? GIT_REMOTE_REFS_CACHE.get(cmd)
+      : exec(cmd, {silent: true}).trim().split('\n');
+
+  // Cache the result for future use (regardless of the value of `retrieveFromCache`).
+  GIT_REMOTE_REFS_CACHE.set(cmd, result);
+
+  return result;
 }
 
-function getMostRecentMinorBranch(major = '*') {
+function getMostRecentMinorBranch(major = '*', options = undefined) {
   // List the branches that start with the given major version (or any major if none given).
-  return getRemoteRefs(`refs/heads/${major}.*.x`)
+  return getRemoteRefs(`refs/heads/${major}.*.x`, options)
       // Extract the branch name.
       .map(line => line.split('/')[2])
       // Filter out branches that are not of the format `<number>.<number>.x`.
@@ -281,8 +297,8 @@ function getMostRecentMinorBranch(major = '*') {
       .pop();
 }
 
-function getLatestCommit(branchName, remote = undefined) {
-  return getRemoteRefs(branchName, remote)[0].slice(0, 40);
+function getLatestCommit(branchName, options = undefined) {
+  return getRemoteRefs(branchName, options)[0].slice(0, 40);
 }
 
 function redirectToAngularIo() {
diff --git a/aio/scripts/deploy-to-firebase.spec.js b/aio/scripts/deploy-to-firebase.spec.js
index 42467f2ddd..c701977cd3 100644
--- a/aio/scripts/deploy-to-firebase.spec.js
+++ b/aio/scripts/deploy-to-firebase.spec.js
@@ -388,13 +388,18 @@ describe('deploy-to-firebase:', () => {
   });
 
   it('integration - should run the main script without error', () => {
+    // NOTE:
+    // This test executes a new instance of the `deploy-to-firebase.js` script on a separate process
+    // and thus does not share the `getRemoteRefs()` cache. To improve stability, we retrieve the
+    // latest commit from master ignoring any cached entries.
+    const latestCommitOnMaster = getLatestCommit('master', {retrieveFromCache: false});
     const cmd = `"${process.execPath}" "${__dirname}/deploy-to-firebase" --dry-run`;
     const env = {
       CI_REPO_OWNER: 'angular',
       CI_REPO_NAME: 'angular',
       CI_PULL_REQUEST: 'false',
       CI_BRANCH: 'master',
-      CI_COMMIT: latestCommits.master,
+      CI_COMMIT: latestCommitOnMaster,
     };
     const result = execSync(cmd, {encoding: 'utf8', env}).trim();
     expect(result).toBe(
@@ -405,7 +410,7 @@ describe('deploy-to-firebase:', () => {
         'Deployment 1 of 1\n' +
         '-----------------\n' +
         'Git branch          : master\n' +
-        `Git commit          : ${latestCommits.master}\n` +
+        `Git commit          : ${latestCommitOnMaster}\n` +
         'Build/deploy mode   : next\n' +
         'Firebase project    : angular-io\n' +
         'Firebase site       : next-angular-io-site\n' +