Merge pull request #6906 from Expensify/Rory-FixCPDeployComment

Update getPullRequestsMergedBetween logic
Expensify · Jan 7, 2022 · 0e5afb7 · 0e5afb7
2 parents 22d4d3e + 824d952
commit 0e5afb7
Show file tree

Hide file tree

Showing 9 changed files with 645 additions and 170 deletions.
diff --git a/.github/actions/createOrUpdateStagingDeploy/index.js b/.github/actions/createOrUpdateStagingDeploy/index.js
@@ -188,43 +188,84 @@ const _ = __nccwpck_require__(3571);
 const {execSync} = __nccwpck_require__(3129);
 
 /**
- * Takes in two git refs and returns a list of PR numbers of all PRs merged between those two refs
+ * Get merge logs between two refs (inclusive) as a JavaScript object.
  *
  * @param {String} fromRef
  * @param {String} toRef
- * @returns {Array}
+ * @returns {Object<{commit: String, subject: String}>}
  */
-function getPullRequestsMergedBetween(fromRef, toRef) {
-    const command = `git log --format="{[%B]}" ${fromRef}...${toRef}`;
+function getMergeLogsAsJSON(fromRef, toRef) {
+    const command = `git log --format='{"commit": "%H", "subject": "%s"},' ${fromRef}^...${toRef}`;
     console.log('Getting pull requests merged between the following refs:', fromRef, toRef);
     console.log('Running command: ', command);
-    const localGitLogs = execSync(command).toString();
+    const result = execSync(command).toString().trim();
 
-    // Parse the git log into an array of commit messages between the two refs
-    const commitMessages = _.map(
-        [...localGitLogs.matchAll(/{\[([\s\S]*?)\]}/gm)],
-        match => match[1],
-    );
-    console.log(`A list of commits made between ${fromRef} and ${toRef}:\n${commitMessages}`);
+    // Remove any double-quotes from commit subjects
+    const sanitizedOutput = result
+        .replace(/(?<="subject": ").*(?="})/g, subject => subject.replace(/"/g, "'"));
 
-    // We need to find which commit messages correspond to merge commits and get PR numbers.
-    // Additionally, we omit merge commits made while cherry picking using negative lookahead in the regexp.
-    const pullRequestIDs = _.reduce(commitMessages, (mergedPRs, commitMessage) => {
-        const mergeCommits = [
-            ...commitMessage.matchAll(/Merge pull request #(\d{1,6}) from (?!(?:Expensify\/(?:master|main|version-))|(?:([\s\S]*?)\(cherry picked from commit .*\)\s*))/gm),
-        ];
+    // Then format as JSON and convert to a proper JS object
+    const json = `[${sanitizedOutput}]`.replace('},]', '}]');
+    return JSON.parse(json);
+}
+
+/**
+ * Parse merged PRs, excluding those from irrelevant branches.
+ *
+ * @param {Array<String>} commitMessages
+ * @returns {Array<String>}
+ */
+function getValidMergedPRs(commitMessages) {
+    return _.reduce(commitMessages, (mergedPRs, commitMessage) => {
+        if (!_.isString(commitMessage)) {
+            return mergedPRs;
+        }
 
-        // Get the PR number of the first match (there should not be multiple matches in one commit message)
-        if (_.size(mergeCommits)) {
-            mergedPRs.push(mergeCommits[0][1]);
+        const match = commitMessage.match(/Merge pull request #(\d+) from (?!Expensify\/(?:master|main|version-|update-staging-from-main|update-production-from-staging))/);
+        if (!_.isNull(match) && match[1]) {
+            mergedPRs.push(match[1]);
         }
+
         return mergedPRs;
     }, []);
-    console.log(`A list of pull requests merged between ${fromRef} and ${toRef}:\n${pullRequestIDs}`);
-    return pullRequestIDs;
+}
+
+/**
+ * Takes in two git refs and returns a list of PR numbers of all PRs merged between those two refs
+ *
+ * @param {String} fromRef
+ * @param {String} toRef
+ * @returns {Array<String>} – Pull request numbers
+ */
+function getPullRequestsMergedBetween(fromRef, toRef) {
+    const targetMergeList = getMergeLogsAsJSON(fromRef, toRef);
+    console.log(`Commits made between ${fromRef} and ${toRef}:`, targetMergeList);
+
+    // Get the full history on this branch, inclusive of the oldest commit from our target comparison
+    const oldestCommit = _.last(targetMergeList).commit;
+    const fullMergeList = getMergeLogsAsJSON(oldestCommit, 'HEAD');
+
+    // Remove from the final merge list any commits whose message appears in the full merge list more than once.
+    // This indicates that the PR should not be included in our list because it is a duplicate, and thus has already been processed by our CI
+    // See https://github.com/Expensify/App/issues/4977 for details
+    const duplicateMergeList = _.chain(fullMergeList)
+        .groupBy('subject')
+        .values()
+        .filter(i => i.length > 1)
+        .flatten()
+        .pluck('commit')
+        .value();
+    const finalMergeList = _.filter(targetMergeList, i => !_.contains(duplicateMergeList, i.commit));
+    console.log('Filtered out the following commits which were duplicated in the full git log:', _.difference(targetMergeList, finalMergeList));
+
+    // Find which commit messages correspond to merged PR's
+    const pullRequestNumbers = getValidMergedPRs(_.pluck(finalMergeList, 'subject'));
+    console.log(`List of pull requests merged between ${fromRef} and ${toRef}`, pullRequestNumbers);
+    return pullRequestNumbers;
 }
 
 module.exports = {
+    getValidMergedPRs,
     getPullRequestsMergedBetween,
 };
 

diff --git a/.github/actions/getDeployPullRequestList/index.js b/.github/actions/getDeployPullRequestList/index.js
@@ -110,43 +110,84 @@ const _ = __nccwpck_require__(3571);
 const {execSync} = __nccwpck_require__(3129);
 
 /**
- * Takes in two git refs and returns a list of PR numbers of all PRs merged between those two refs
+ * Get merge logs between two refs (inclusive) as a JavaScript object.
  *
  * @param {String} fromRef
  * @param {String} toRef
- * @returns {Array}
+ * @returns {Object<{commit: String, subject: String}>}
  */
-function getPullRequestsMergedBetween(fromRef, toRef) {
-    const command = `git log --format="{[%B]}" ${fromRef}...${toRef}`;
+function getMergeLogsAsJSON(fromRef, toRef) {
+    const command = `git log --format='{"commit": "%H", "subject": "%s"},' ${fromRef}^...${toRef}`;
     console.log('Getting pull requests merged between the following refs:', fromRef, toRef);
     console.log('Running command: ', command);
-    const localGitLogs = execSync(command).toString();
+    const result = execSync(command).toString().trim();
 
-    // Parse the git log into an array of commit messages between the two refs
-    const commitMessages = _.map(
-        [...localGitLogs.matchAll(/{\[([\s\S]*?)\]}/gm)],
-        match => match[1],
-    );
-    console.log(`A list of commits made between ${fromRef} and ${toRef}:\n${commitMessages}`);
-
-    // We need to find which commit messages correspond to merge commits and get PR numbers.
-    // Additionally, we omit merge commits made while cherry picking using negative lookahead in the regexp.
-    const pullRequestIDs = _.reduce(commitMessages, (mergedPRs, commitMessage) => {
-        const mergeCommits = [
-            ...commitMessage.matchAll(/Merge pull request #(\d{1,6}) from (?!(?:Expensify\/(?:master|main|version-))|(?:([\s\S]*?)\(cherry picked from commit .*\)\s*))/gm),
-        ];
-
-        // Get the PR number of the first match (there should not be multiple matches in one commit message)
-        if (_.size(mergeCommits)) {
-            mergedPRs.push(mergeCommits[0][1]);
+    // Remove any double-quotes from commit subjects
+    const sanitizedOutput = result
+        .replace(/(?<="subject": ").*(?="})/g, subject => subject.replace(/"/g, "'"));
+
+    // Then format as JSON and convert to a proper JS object
+    const json = `[${sanitizedOutput}]`.replace('},]', '}]');
+    return JSON.parse(json);
+}
+
+/**
+ * Parse merged PRs, excluding those from irrelevant branches.
+ *
+ * @param {Array<String>} commitMessages
+ * @returns {Array<String>}
+ */
+function getValidMergedPRs(commitMessages) {
+    return _.reduce(commitMessages, (mergedPRs, commitMessage) => {
+        if (!_.isString(commitMessage)) {
+            return mergedPRs;
+        }
+
+        const match = commitMessage.match(/Merge pull request #(\d+) from (?!Expensify\/(?:master|main|version-|update-staging-from-main|update-production-from-staging))/);
+        if (!_.isNull(match) && match[1]) {
+            mergedPRs.push(match[1]);
         }
+
         return mergedPRs;
     }, []);
-    console.log(`A list of pull requests merged between ${fromRef} and ${toRef}:\n${pullRequestIDs}`);
-    return pullRequestIDs;
+}
+
+/**
+ * Takes in two git refs and returns a list of PR numbers of all PRs merged between those two refs
+ *
+ * @param {String} fromRef
+ * @param {String} toRef
+ * @returns {Array<String>} – Pull request numbers
+ */
+function getPullRequestsMergedBetween(fromRef, toRef) {
+    const targetMergeList = getMergeLogsAsJSON(fromRef, toRef);
+    console.log(`Commits made between ${fromRef} and ${toRef}:`, targetMergeList);
+
+    // Get the full history on this branch, inclusive of the oldest commit from our target comparison
+    const oldestCommit = _.last(targetMergeList).commit;
+    const fullMergeList = getMergeLogsAsJSON(oldestCommit, 'HEAD');
+
+    // Remove from the final merge list any commits whose message appears in the full merge list more than once.
+    // This indicates that the PR should not be included in our list because it is a duplicate, and thus has already been processed by our CI
+    // See https://github.com/Expensify/App/issues/4977 for details
+    const duplicateMergeList = _.chain(fullMergeList)
+        .groupBy('subject')
+        .values()
+        .filter(i => i.length > 1)
+        .flatten()
+        .pluck('commit')
+        .value();
+    const finalMergeList = _.filter(targetMergeList, i => !_.contains(duplicateMergeList, i.commit));
+    console.log('Filtered out the following commits which were duplicated in the full git log:', _.difference(targetMergeList, finalMergeList));
+
+    // Find which commit messages correspond to merged PR's
+    const pullRequestNumbers = getValidMergedPRs(_.pluck(finalMergeList, 'subject'));
+    console.log(`List of pull requests merged between ${fromRef} and ${toRef}`, pullRequestNumbers);
+    return pullRequestNumbers;
 }
 
 module.exports = {
+    getValidMergedPRs,
     getPullRequestsMergedBetween,
 };
 

diff --git a/.github/libs/GitUtils.js b/.github/libs/GitUtils.js
@@ -2,42 +2,83 @@ const _ = require('underscore');
 const {execSync} = require('child_process');
 
 /**
- * Takes in two git refs and returns a list of PR numbers of all PRs merged between those two refs
+ * Get merge logs between two refs (inclusive) as a JavaScript object.
  *
  * @param {String} fromRef
  * @param {String} toRef
- * @returns {Array}
+ * @returns {Object<{commit: String, subject: String}>}
  */
-function getPullRequestsMergedBetween(fromRef, toRef) {
-    const command = `git log --format="{[%B]}" ${fromRef}...${toRef}`;
+function getMergeLogsAsJSON(fromRef, toRef) {
+    const command = `git log --format='{"commit": "%H", "subject": "%s"},' ${fromRef}^...${toRef}`;
     console.log('Getting pull requests merged between the following refs:', fromRef, toRef);
     console.log('Running command: ', command);
-    const localGitLogs = execSync(command).toString();
-
-    // Parse the git log into an array of commit messages between the two refs
-    const commitMessages = _.map(
-        [...localGitLogs.matchAll(/{\[([\s\S]*?)\]}/gm)],
-        match => match[1],
-    );
-    console.log(`A list of commits made between ${fromRef} and ${toRef}:\n${commitMessages}`);
-
-    // We need to find which commit messages correspond to merge commits and get PR numbers.
-    // Additionally, we omit merge commits made while cherry picking using negative lookahead in the regexp.
-    const pullRequestIDs = _.reduce(commitMessages, (mergedPRs, commitMessage) => {
-        const mergeCommits = [
-            ...commitMessage.matchAll(/Merge pull request #(\d{1,6}) from (?!(?:Expensify\/(?:master|main|version-))|(?:([\s\S]*?)\(cherry picked from commit .*\)\s*))/gm),
-        ];
-
-        // Get the PR number of the first match (there should not be multiple matches in one commit message)
-        if (_.size(mergeCommits)) {
-            mergedPRs.push(mergeCommits[0][1]);
+    const result = execSync(command).toString().trim();
+
+    // Remove any double-quotes from commit subjects
+    const sanitizedOutput = result
+        .replace(/(?<="subject": ").*(?="})/g, subject => subject.replace(/"/g, "'"));
+
+    // Then format as JSON and convert to a proper JS object
+    const json = `[${sanitizedOutput}]`.replace('},]', '}]');
+    return JSON.parse(json);
+}
+
+/**
+ * Parse merged PRs, excluding those from irrelevant branches.
+ *
+ * @param {Array<String>} commitMessages
+ * @returns {Array<String>}
+ */
+function getValidMergedPRs(commitMessages) {
+    return _.reduce(commitMessages, (mergedPRs, commitMessage) => {
+        if (!_.isString(commitMessage)) {
+            return mergedPRs;
         }
+
+        const match = commitMessage.match(/Merge pull request #(\d+) from (?!Expensify\/(?:master|main|version-|update-staging-from-main|update-production-from-staging))/);
+        if (!_.isNull(match) && match[1]) {
+            mergedPRs.push(match[1]);
+        }
+
         return mergedPRs;
     }, []);
-    console.log(`A list of pull requests merged between ${fromRef} and ${toRef}:\n${pullRequestIDs}`);
-    return pullRequestIDs;
+}
+
+/**
+ * Takes in two git refs and returns a list of PR numbers of all PRs merged between those two refs
+ *
+ * @param {String} fromRef
+ * @param {String} toRef
+ * @returns {Array<String>} – Pull request numbers
+ */
+function getPullRequestsMergedBetween(fromRef, toRef) {
+    const targetMergeList = getMergeLogsAsJSON(fromRef, toRef);
+    console.log(`Commits made between ${fromRef} and ${toRef}:`, targetMergeList);
+
+    // Get the full history on this branch, inclusive of the oldest commit from our target comparison
+    const oldestCommit = _.last(targetMergeList).commit;
+    const fullMergeList = getMergeLogsAsJSON(oldestCommit, 'HEAD');
+
+    // Remove from the final merge list any commits whose message appears in the full merge list more than once.
+    // This indicates that the PR should not be included in our list because it is a duplicate, and thus has already been processed by our CI
+    // See https://github.com/Expensify/App/issues/4977 for details
+    const duplicateMergeList = _.chain(fullMergeList)
+        .groupBy('subject')
+        .values()
+        .filter(i => i.length > 1)
+        .flatten()
+        .pluck('commit')
+        .value();
+    const finalMergeList = _.filter(targetMergeList, i => !_.contains(duplicateMergeList, i.commit));
+    console.log('Filtered out the following commits which were duplicated in the full git log:', _.difference(targetMergeList, finalMergeList));
+
+    // Find which commit messages correspond to merged PR's
+    const pullRequestNumbers = getValidMergedPRs(_.pluck(finalMergeList, 'subject'));
+    console.log(`List of pull requests merged between ${fromRef} and ${toRef}`, pullRequestNumbers);
+    return pullRequestNumbers;
 }
 
 module.exports = {
+    getValidMergedPRs,
     getPullRequestsMergedBetween,
 };
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -36,3 +36,20 @@ jobs:
       - run: npm run test
         env:
           CI: true
+
+      - name: Decrypt OSBotify GPG key
+        run: cd .github/workflows && gpg --quiet --batch --yes --decrypt --passphrase="$LARGE_SECRET_PASSPHRASE" --output OSBotify-private-key.asc OSBotify-private-key.asc.gpg
+        env:
+          LARGE_SECRET_PASSPHRASE: ${{ secrets.LARGE_SECRET_PASSPHRASE }}
+
+      - name: Import OSBotify GPG Key
+        run: cd .github/workflows && gpg --import OSBotify-private-key.asc
+
+      - name: Set up git for OSBotify
+        run: |
+          git config --global user.name OSBotify
+          git config --global user.email infra+osbotify@expensify.com
+          git config --global user.signingkey 367811D53E34168C
+          git config --global commit.gpgsign true
+
+      - run: tests/unit/getPullRequestsMergedBetweenTest.sh
diff --git a/.github/workflows/updateProtectedBranch.yml b/.github/workflows/updateProtectedBranch.yml
@@ -54,15 +54,15 @@ jobs:
       - name: Set New Version
         run: echo "NEW_VERSION=$(npm run print-version --silent)" >> $GITHUB_ENV
 
-      - name: Decrypt Botify GPG key
+      - name: Decrypt OSBotify GPG key
         run: cd .github/workflows && gpg --quiet --batch --yes --decrypt --passphrase="$LARGE_SECRET_PASSPHRASE" --output OSBotify-private-key.asc OSBotify-private-key.asc.gpg
         env:
           LARGE_SECRET_PASSPHRASE: ${{ secrets.LARGE_SECRET_PASSPHRASE }}
 
-      - name: Import Botify GPG Key
+      - name: Import OSBotify GPG Key
         run: cd .github/workflows && gpg --import OSBotify-private-key.asc
 
-      - name: Set up git for Botify
+      - name: Set up git for OSBotify
         run: |
           git config user.signingkey 367811D53E34168C
           git config commit.gpgsign true

diff --git a/shellUtils.sh b/shellUtils.sh
@@ -0,0 +1,32 @@
+#!/bin/bash
+
+GREEN=$'\e[1;32m'
+RED=$'\e[1;31m'
+BLUE=$'\e[1;34m'
+TITLE=$'\e[1;4;34m'
+RESET=$'\e[0m'
+
+function success {
+  echo "🎉 $GREEN$1$RESET"
+}
+
+function error {
+  echo "💥 $RED$1$RESET"
+}
+
+function info {
+  echo "$BLUE$1$RESET"
+}
+
+function title {
+  printf "\n%s%s%s\n" "$TITLE" "$1" "$RESET"
+}
+
+function assert_equal {
+  if [[ "$1" != "$2" ]]; then
+    error "Assertion failed: $1 is not equal to $2"
+    exit 1
+  else
+    success "Assertion passed: $1 is equal to $1"
+  fi
+}