Merge pull request #31507 from Expensify/Rory-ImproveGitUtilsLogs

[No QA] Improve GitUtils logging
Expensify · Nov 17, 2023 · 3e06e0e · 3e06e0e
2 parents 7c3b07a + 97c1cb0
commit 3e06e0e
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 37 deletions.
diff --git a/.github/actions/javascript/createOrUpdateStagingDeploy/index.js b/.github/actions/javascript/createOrUpdateStagingDeploy/index.js
@@ -198,13 +198,16 @@ function fetchTag(tag) {
     let needsRepack = false;
     while (shouldRetry) {
         try {
+            let command = '';
             if (needsRepack) {
                 // We have seen some scenarios where this fixes the git fetch.
                 // Why? Who knows... https://github.com/Expensify/App/pull/31459
-                execSync('git repack -d');
+                command = 'git repack -d';
+                console.log(`Running command: ${command}`);
+                execSync(command);
             }
 
-            let command = `git fetch origin tag ${tag} --no-tags`;
+            command = `git fetch origin tag ${tag} --no-tags`;
 
             // Exclude commits reachable from the previous patch version (i.e: previous checklist),
             // so that we don't have to fetch the full history
@@ -315,16 +318,15 @@ function getValidMergedPRs(commits) {
  * @param {String} toTag
  * @returns {Promise<Array<Number>>} – Pull request numbers
  */
-function getPullRequestsMergedBetween(fromTag, toTag) {
+async function getPullRequestsMergedBetween(fromTag, toTag) {
     console.log(`Looking for commits made between ${fromTag} and ${toTag}...`);
-    return getCommitHistoryAsJSON(fromTag, toTag).then((commitList) => {
-        console.log(`Commits made between ${fromTag} and ${toTag}:`, commitList);
+    const commitList = await getCommitHistoryAsJSON(fromTag, toTag);
+    console.log(`Commits made between ${fromTag} and ${toTag}:`, commitList);
 
-        // Find which commit messages correspond to merged PR's
-        const pullRequestNumbers = getValidMergedPRs(commitList);
-        console.log(`List of pull requests merged between ${fromTag} and ${toTag}`, pullRequestNumbers);
-        return _.map(pullRequestNumbers, (prNum) => Number.parseInt(prNum, 10));
-    });
+    // Find which commit messages correspond to merged PR's
+    const pullRequestNumbers = getValidMergedPRs(commitList).sort((a, b) => a - b);
+    console.log(`List of pull requests merged between ${fromTag} and ${toTag}`, pullRequestNumbers);
+    return pullRequestNumbers;
 }
 
 module.exports = {

diff --git a/.github/actions/javascript/getDeployPullRequestList/index.js b/.github/actions/javascript/getDeployPullRequestList/index.js
@@ -140,13 +140,16 @@ function fetchTag(tag) {
     let needsRepack = false;
     while (shouldRetry) {
         try {
+            let command = '';
             if (needsRepack) {
                 // We have seen some scenarios where this fixes the git fetch.
                 // Why? Who knows... https://github.com/Expensify/App/pull/31459
-                execSync('git repack -d');
+                command = 'git repack -d';
+                console.log(`Running command: ${command}`);
+                execSync(command);
             }
 
-            let command = `git fetch origin tag ${tag} --no-tags`;
+            command = `git fetch origin tag ${tag} --no-tags`;
 
             // Exclude commits reachable from the previous patch version (i.e: previous checklist),
             // so that we don't have to fetch the full history
@@ -257,16 +260,15 @@ function getValidMergedPRs(commits) {
  * @param {String} toTag
  * @returns {Promise<Array<Number>>} – Pull request numbers
  */
-function getPullRequestsMergedBetween(fromTag, toTag) {
+async function getPullRequestsMergedBetween(fromTag, toTag) {
     console.log(`Looking for commits made between ${fromTag} and ${toTag}...`);
-    return getCommitHistoryAsJSON(fromTag, toTag).then((commitList) => {
-        console.log(`Commits made between ${fromTag} and ${toTag}:`, commitList);
+    const commitList = await getCommitHistoryAsJSON(fromTag, toTag);
+    console.log(`Commits made between ${fromTag} and ${toTag}:`, commitList);
 
-        // Find which commit messages correspond to merged PR's
-        const pullRequestNumbers = getValidMergedPRs(commitList);
-        console.log(`List of pull requests merged between ${fromTag} and ${toTag}`, pullRequestNumbers);
-        return _.map(pullRequestNumbers, (prNum) => Number.parseInt(prNum, 10));
-    });
+    // Find which commit messages correspond to merged PR's
+    const pullRequestNumbers = getValidMergedPRs(commitList).sort((a, b) => a - b);
+    console.log(`List of pull requests merged between ${fromTag} and ${toTag}`, pullRequestNumbers);
+    return pullRequestNumbers;
 }
 
 module.exports = {

diff --git a/.github/libs/GitUtils.js b/.github/libs/GitUtils.js
@@ -13,13 +13,16 @@ function fetchTag(tag) {
     let needsRepack = false;
     while (shouldRetry) {
         try {
+            let command = '';
             if (needsRepack) {
                 // We have seen some scenarios where this fixes the git fetch.
                 // Why? Who knows... https://github.com/Expensify/App/pull/31459
-                execSync('git repack -d');
+                command = 'git repack -d';
+                console.log(`Running command: ${command}`);
+                execSync(command);
             }
 
-            let command = `git fetch origin tag ${tag} --no-tags`;
+            command = `git fetch origin tag ${tag} --no-tags`;
 
             // Exclude commits reachable from the previous patch version (i.e: previous checklist),
             // so that we don't have to fetch the full history
@@ -130,16 +133,15 @@ function getValidMergedPRs(commits) {
  * @param {String} toTag
  * @returns {Promise<Array<Number>>} – Pull request numbers
  */
-function getPullRequestsMergedBetween(fromTag, toTag) {
+async function getPullRequestsMergedBetween(fromTag, toTag) {
     console.log(`Looking for commits made between ${fromTag} and ${toTag}...`);
-    return getCommitHistoryAsJSON(fromTag, toTag).then((commitList) => {
-        console.log(`Commits made between ${fromTag} and ${toTag}:`, commitList);
+    const commitList = await getCommitHistoryAsJSON(fromTag, toTag);
+    console.log(`Commits made between ${fromTag} and ${toTag}:`, commitList);
 
-        // Find which commit messages correspond to merged PR's
-        const pullRequestNumbers = getValidMergedPRs(commitList);
-        console.log(`List of pull requests merged between ${fromTag} and ${toTag}`, pullRequestNumbers);
-        return _.map(pullRequestNumbers, (prNum) => Number.parseInt(prNum, 10));
-    });
+    // Find which commit messages correspond to merged PR's
+    const pullRequestNumbers = getValidMergedPRs(commitList).sort((a, b) => a - b);
+    console.log(`List of pull requests merged between ${fromTag} and ${toTag}`, pullRequestNumbers);
+    return pullRequestNumbers;
 }
 
 module.exports = {

diff --git a/tests/unit/CIGitLogicTest.sh b/tests/unit/CIGitLogicTest.sh
@@ -238,7 +238,7 @@ cherry_pick_pr 3
 tag_staging
 
 # Verify output for checklist
-assert_prs_merged_between '1.0.0-0' '1.0.0-2' "[ 3, 1 ]"
+assert_prs_merged_between '1.0.0-0' '1.0.0-2' "[ 1, 3 ]"
 
 # Verify output for deploy comment
 assert_prs_merged_between '1.0.0-1' '1.0.0-2' "[ 3 ]"
@@ -252,7 +252,7 @@ title "Scenario #4A: Run the production deploy"
 update_production_from_staging
 
 # Verify output for release body and production deploy comments
-assert_prs_merged_between '1.0.0-0' '1.0.0-2' "[ 3, 1 ]"
+assert_prs_merged_between '1.0.0-0' '1.0.0-2' "[ 1, 3 ]"
 
 success "Scenario #4A completed successfully!"
 
@@ -284,7 +284,7 @@ update_staging_from_main
 tag_staging
 
 # Verify output for checklist
-assert_prs_merged_between '1.0.0-2' '1.0.1-1' "[ 5, 2 ]"
+assert_prs_merged_between '1.0.0-2' '1.0.1-1' "[ 2, 5 ]"
 
 # Verify output for deploy comment
 assert_prs_merged_between '1.0.1-0' '1.0.1-1' "[ 5 ]"
@@ -307,7 +307,7 @@ update_staging_from_main
 tag_staging
 
 # Verify output for checklist
-assert_prs_merged_between '1.0.0-2' '1.0.1-2' "[ 6, 5, 2 ]"
+assert_prs_merged_between '1.0.0-2' '1.0.1-2' "[ 2, 5, 6 ]"
 
 # Verify output for deploy comment
 assert_prs_merged_between '1.0.1-1' '1.0.1-2' "[ 6 ]"
@@ -329,7 +329,7 @@ update_staging_from_main
 tag_staging
 
 # Verify output for checklist
-assert_prs_merged_between '1.0.0-2' '1.0.1-3' "[ 7, 6, 5, 2 ]"
+assert_prs_merged_between '1.0.0-2' '1.0.1-3' "[ 2, 5, 6, 7 ]"
 
 # Verify output for deploy comment
 assert_prs_merged_between '1.0.1-2' '1.0.1-3' "[ 7 ]"
@@ -389,10 +389,10 @@ update_staging_from_main
 tag_staging
 
 # Verify production release list
-assert_prs_merged_between '1.0.0-2' '1.0.1-4' "[ 9, 7, 6, 5, 2 ]"
+assert_prs_merged_between '1.0.0-2' '1.0.1-4' "[ 2, 5, 6, 7, 9 ]"
 
 # Verify PR list for the new checklist
-assert_prs_merged_between '1.0.1-4' '1.0.2-0' "[ 10, 8 ]"
+assert_prs_merged_between '1.0.1-4' '1.0.2-0' "[ 8, 10 ]"
 
 ### Cleanup
 title "Cleaning up..."