From e6673a55ded62ee2461b19e04d4fce760a201d03 Mon Sep 17 00:00:00 2001 From: Rory Abraham <47436092+roryabraham@users.noreply.github.com> Date: Wed, 13 Jul 2022 16:27:31 -0700 Subject: [PATCH] Revert "[No QA] Make GitUtils use spawn/stream instead of execSync/buffer" --- .../createOrUpdateStagingDeploy.js | 58 +++---- .../createOrUpdateStagingDeploy/index.js | 155 +++++++----------- .../getDeployPullRequestList.js | 4 +- .../getDeployPullRequestList/index.js | 101 ++++-------- .github/libs/GitUtils.js | 93 ++++------- .../unit/getPullRequestsMergedBetweenTest.sh | 2 +- tests/utils/getPullRequestsMergedBetween.js | 12 ++ tests/utils/getPullRequestsMergedBetween.mjs | 15 -- 8 files changed, 173 insertions(+), 267 deletions(-) create mode 100755 tests/utils/getPullRequestsMergedBetween.js delete mode 100755 tests/utils/getPullRequestsMergedBetween.mjs diff --git a/.github/actions/javascript/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js b/.github/actions/javascript/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js index 27db22f3ca71..854349998902 100644 --- a/.github/actions/javascript/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js +++ b/.github/actions/javascript/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js @@ -9,10 +9,7 @@ const run = function () { console.log('New version found from action input:', newVersion); let shouldCreateNewStagingDeployCash = false; - let newTag = newVersion; - let newDeployBlockers = []; - let previousStagingDeployCashData = {}; - let currentStagingDeployCashData = null; + let currentStagingDeployCashIssueNumber = null; // Start by fetching the list of recent StagingDeployCash issues, along with the list of open deploy blockers return Promise.all([ @@ -40,12 +37,6 @@ const run = function () { console.log('Failed fetching DeployBlockerCash issues from Github, continuing...'); } - newDeployBlockers = _.map(deployBlockerResponse.data, ({html_url}) => ({ - url: html_url, - number: GithubUtils.getIssueOrPullRequestNumberFromURL(html_url), - isResolved: false, - })); - // Look at the state of the most recent StagingDeployCash, // if it is open then we'll update the existing one, otherwise, we'll create a new one. shouldCreateNewStagingDeployCash = Boolean(stagingDeployResponse.data[0].state !== 'open'); @@ -61,37 +52,46 @@ const run = function () { // Parse the data from the previous StagingDeployCash // (newest if there are none open, otherwise second-newest) - previousStagingDeployCashData = shouldCreateNewStagingDeployCash + const previousStagingDeployCashData = shouldCreateNewStagingDeployCash ? GithubUtils.getStagingDeployCashData(stagingDeployResponse.data[0]) : GithubUtils.getStagingDeployCashData(stagingDeployResponse.data[1]); console.log('Found tag of previous StagingDeployCash:', previousStagingDeployCashData.tag); - // Find the list of PRs merged between the last StagingDeployCash and the new version if (shouldCreateNewStagingDeployCash) { - return GitUtils.getPullRequestsMergedBetween(previousStagingDeployCashData.tag, newTag); - } - - currentStagingDeployCashData = GithubUtils.getStagingDeployCashData(stagingDeployResponse.data[0]); - console.log('Parsed the following data from the current StagingDeployCash:', currentStagingDeployCashData); - - // If we aren't sent a tag, then use the existing tag - newTag = newTag || currentStagingDeployCashData.tag; - - return GitUtils.getPullRequestsMergedBetween(previousStagingDeployCashData.tag, newTag); - }) - .then((mergedPRs) => { - console.log(`The following PRs have been merged between the previous StagingDeployCash (${previousStagingDeployCashData.tag}) and new version (${newVersion}):`, mergedPRs); + // Find the list of PRs merged between the last StagingDeployCash and the new version + const mergedPRs = GitUtils.getPullRequestsMergedBetween(previousStagingDeployCashData.tag, newVersion); + // eslint-disable-next-line max-len + console.log(`The following PRs have been merged between the previous StagingDeployCash (${previousStagingDeployCashData.tag}) and new version (${newVersion}):`, mergedPRs); - if (shouldCreateNewStagingDeployCash) { + // TODO: if there are open DeployBlockers and we are opening a new checklist, + // then we should close / remove the DeployBlockerCash label from those return GithubUtils.generateStagingDeployCashBody( - newTag, + newVersion, _.map(mergedPRs, GithubUtils.getPullRequestURLFromNumber), ); } + // There is an open StagingDeployCash, so we'll be updating it, not creating a new one + const currentStagingDeployCashData = GithubUtils.getStagingDeployCashData(stagingDeployResponse.data[0]); + console.log('Parsed the following data from the current StagingDeployCash:', currentStagingDeployCashData); + currentStagingDeployCashIssueNumber = currentStagingDeployCashData.number; + + const newDeployBlockers = _.map(deployBlockerResponse.data, ({html_url}) => ({ + url: html_url, + number: GithubUtils.getIssueOrPullRequestNumberFromURL(html_url), + isResolved: false, + })); + + // If we aren't sent a tag, then use the existing tag + const tag = newVersion || currentStagingDeployCashData.tag; const didVersionChange = newVersion ? newVersion !== currentStagingDeployCashData.tag : false; + // Find the list of PRs merged between the last StagingDeployCash and the new version + const mergedPRs = GitUtils.getPullRequestsMergedBetween(previousStagingDeployCashData.tag, tag); + // eslint-disable-next-line max-len + console.log(`The following PRs have been merged between the previous StagingDeployCash (${previousStagingDeployCashData.tag}) and new version (${tag}):`, mergedPRs); + // Generate the PR list, preserving the previous state of `isVerified` for existing PRs const PRList = _.sortBy( _.unique( @@ -123,7 +123,7 @@ const run = function () { ); return GithubUtils.generateStagingDeployCashBody( - newTag, + tag, _.pluck(PRList, 'url'), _.pluck(_.where(PRList, {isVerified: true}), 'url'), _.pluck(_.where(PRList, {isAccessible: true}), 'url'), @@ -151,7 +151,7 @@ const run = function () { return GithubUtils.octokit.issues.update({ ...defaultPayload, - issue_number: currentStagingDeployCashData.number, + issue_number: currentStagingDeployCashIssueNumber, }); }) .then(({data}) => { diff --git a/.github/actions/javascript/createOrUpdateStagingDeploy/index.js b/.github/actions/javascript/createOrUpdateStagingDeploy/index.js index 97ea937ef3e9..155d321bac76 100644 --- a/.github/actions/javascript/createOrUpdateStagingDeploy/index.js +++ b/.github/actions/javascript/createOrUpdateStagingDeploy/index.js @@ -19,10 +19,7 @@ const run = function () { console.log('New version found from action input:', newVersion); let shouldCreateNewStagingDeployCash = false; - let newTag = newVersion; - let newDeployBlockers = []; - let previousStagingDeployCashData = {}; - let currentStagingDeployCashData = null; + let currentStagingDeployCashIssueNumber = null; // Start by fetching the list of recent StagingDeployCash issues, along with the list of open deploy blockers return Promise.all([ @@ -50,12 +47,6 @@ const run = function () { console.log('Failed fetching DeployBlockerCash issues from Github, continuing...'); } - newDeployBlockers = _.map(deployBlockerResponse.data, ({html_url}) => ({ - url: html_url, - number: GithubUtils.getIssueOrPullRequestNumberFromURL(html_url), - isResolved: false, - })); - // Look at the state of the most recent StagingDeployCash, // if it is open then we'll update the existing one, otherwise, we'll create a new one. shouldCreateNewStagingDeployCash = Boolean(stagingDeployResponse.data[0].state !== 'open'); @@ -71,37 +62,46 @@ const run = function () { // Parse the data from the previous StagingDeployCash // (newest if there are none open, otherwise second-newest) - previousStagingDeployCashData = shouldCreateNewStagingDeployCash + const previousStagingDeployCashData = shouldCreateNewStagingDeployCash ? GithubUtils.getStagingDeployCashData(stagingDeployResponse.data[0]) : GithubUtils.getStagingDeployCashData(stagingDeployResponse.data[1]); console.log('Found tag of previous StagingDeployCash:', previousStagingDeployCashData.tag); - // Find the list of PRs merged between the last StagingDeployCash and the new version if (shouldCreateNewStagingDeployCash) { - return GitUtils.getPullRequestsMergedBetween(previousStagingDeployCashData.tag, newTag); - } - - currentStagingDeployCashData = GithubUtils.getStagingDeployCashData(stagingDeployResponse.data[0]); - console.log('Parsed the following data from the current StagingDeployCash:', currentStagingDeployCashData); - - // If we aren't sent a tag, then use the existing tag - newTag = newTag || currentStagingDeployCashData.tag; - - return GitUtils.getPullRequestsMergedBetween(previousStagingDeployCashData.tag, newTag); - }) - .then((mergedPRs) => { - console.log(`The following PRs have been merged between the previous StagingDeployCash (${previousStagingDeployCashData.tag}) and new version (${newVersion}):`, mergedPRs); + // Find the list of PRs merged between the last StagingDeployCash and the new version + const mergedPRs = GitUtils.getPullRequestsMergedBetween(previousStagingDeployCashData.tag, newVersion); + // eslint-disable-next-line max-len + console.log(`The following PRs have been merged between the previous StagingDeployCash (${previousStagingDeployCashData.tag}) and new version (${newVersion}):`, mergedPRs); - if (shouldCreateNewStagingDeployCash) { + // TODO: if there are open DeployBlockers and we are opening a new checklist, + // then we should close / remove the DeployBlockerCash label from those return GithubUtils.generateStagingDeployCashBody( - newTag, + newVersion, _.map(mergedPRs, GithubUtils.getPullRequestURLFromNumber), ); } + // There is an open StagingDeployCash, so we'll be updating it, not creating a new one + const currentStagingDeployCashData = GithubUtils.getStagingDeployCashData(stagingDeployResponse.data[0]); + console.log('Parsed the following data from the current StagingDeployCash:', currentStagingDeployCashData); + currentStagingDeployCashIssueNumber = currentStagingDeployCashData.number; + + const newDeployBlockers = _.map(deployBlockerResponse.data, ({html_url}) => ({ + url: html_url, + number: GithubUtils.getIssueOrPullRequestNumberFromURL(html_url), + isResolved: false, + })); + + // If we aren't sent a tag, then use the existing tag + const tag = newVersion || currentStagingDeployCashData.tag; const didVersionChange = newVersion ? newVersion !== currentStagingDeployCashData.tag : false; + // Find the list of PRs merged between the last StagingDeployCash and the new version + const mergedPRs = GitUtils.getPullRequestsMergedBetween(previousStagingDeployCashData.tag, tag); + // eslint-disable-next-line max-len + console.log(`The following PRs have been merged between the previous StagingDeployCash (${previousStagingDeployCashData.tag}) and new version (${tag}):`, mergedPRs); + // Generate the PR list, preserving the previous state of `isVerified` for existing PRs const PRList = _.sortBy( _.unique( @@ -133,7 +133,7 @@ const run = function () { ); return GithubUtils.generateStagingDeployCashBody( - newTag, + tag, _.pluck(PRList, 'url'), _.pluck(_.where(PRList, {isVerified: true}), 'url'), _.pluck(_.where(PRList, {isAccessible: true}), 'url'), @@ -161,7 +161,7 @@ const run = function () { return GithubUtils.octokit.issues.update({ ...defaultPayload, - issue_number: currentStagingDeployCashData.number, + issue_number: currentStagingDeployCashIssueNumber, }); }) .then(({data}) => { @@ -188,52 +188,28 @@ module.exports = run; /***/ ((module, __unused_webpack_exports, __nccwpck_require__) => { const _ = __nccwpck_require__(3571); -const {spawn} = __nccwpck_require__(3129); +const {execSync} = __nccwpck_require__(3129); /** * Get merge logs between two refs (inclusive) as a JavaScript object. * * @param {String} fromRef * @param {String} toRef - * @returns {Promise>} + * @returns {Object<{commit: String, subject: String}>} */ 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); - return new Promise((resolve, reject) => { - let stdout = ''; - let stderr = ''; - const spawnedProcess = spawn('git', ['log', '--format={"commit": "%H", "subject": "%s"},', `${fromRef}...${toRef}`]); - spawnedProcess.on('message', console.log); - spawnedProcess.stdout.on('data', (chunk) => { - console.log(chunk.toString()); - stdout += chunk.toString(); - }); - spawnedProcess.stderr.on('data', (chunk) => { - console.error(chunk.toString()); - stderr += chunk.toString(); - }); - spawnedProcess.on('close', (code) => { - if (code !== 0) { - return reject(new Error(`${stderr}`)); - } + const result = execSync(command).toString().trim(); - resolve(stdout); - }); - spawnedProcess.on('error', err => reject(err)); - }) - .then((stdout) => { - // Remove any double-quotes from commit subjects - let sanitizedOutput = stdout.replace(/(?<="subject": ").*(?="})/g, subject => subject.replace(/"/g, "'")); + // Remove any double-quotes from commit subjects + const sanitizedOutput = result + .replace(/(?<="subject": ").*(?="})/g, subject => subject.replace(/"/g, "'")); - // Also remove any newlines - sanitizedOutput = sanitizedOutput.replace(/(\r\n|\n|\r)/gm, ''); - - // Then format as JSON and convert to a proper JS object - const json = `[${sanitizedOutput}]`.replace('},]', '}]'); - return JSON.parse(json); - }); + // Then format as JSON and convert to a proper JS object + const json = `[${sanitizedOutput}]`.replace('},]', '}]'); + return JSON.parse(json); } /** @@ -262,38 +238,33 @@ function getValidMergedPRs(commitMessages) { * * @param {String} fromRef * @param {String} toRef - * @returns {Promise>} – Pull request numbers + * @returns {Array} – Pull request numbers */ function getPullRequestsMergedBetween(fromRef, toRef) { - let targetMergeList; - return getMergeLogsAsJSON(fromRef, toRef) - .then((mergeList) => { - console.log(`Commits made between ${fromRef} and ${toRef}:`, mergeList); - targetMergeList = mergeList; - - // Get the full history on this branch, inclusive of the oldest commit from our target comparison - const oldestCommit = _.last(mergeList).commit; - return getMergeLogsAsJSON(oldestCommit, 'HEAD'); - }) - .then((fullMergeList) => { - // 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; - }); + 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 = { diff --git a/.github/actions/javascript/getDeployPullRequestList/getDeployPullRequestList.js b/.github/actions/javascript/getDeployPullRequestList/getDeployPullRequestList.js index 753e3b98d43e..32f0e494c7ad 100644 --- a/.github/actions/javascript/getDeployPullRequestList/getDeployPullRequestList.js +++ b/.github/actions/javascript/getDeployPullRequestList/getDeployPullRequestList.js @@ -55,9 +55,7 @@ getTagsOrReleases(isProductionDeploy) console.log(`Given ${itemToFetch}: ${inputTag}`); console.log(`Prior ${itemToFetch}: ${priorTag}`); - return GitUtils.getPullRequestsMergedBetween(priorTag, inputTag); - }) - .then((pullRequestList) => { + const pullRequestList = GitUtils.getPullRequestsMergedBetween(priorTag, inputTag); console.log(`Found the pull request list: ${pullRequestList}`); return core.setOutput('PR_LIST', pullRequestList); }) diff --git a/.github/actions/javascript/getDeployPullRequestList/index.js b/.github/actions/javascript/getDeployPullRequestList/index.js index 3efbe9136d02..d1e1163657a3 100644 --- a/.github/actions/javascript/getDeployPullRequestList/index.js +++ b/.github/actions/javascript/getDeployPullRequestList/index.js @@ -65,9 +65,7 @@ getTagsOrReleases(isProductionDeploy) console.log(`Given ${itemToFetch}: ${inputTag}`); console.log(`Prior ${itemToFetch}: ${priorTag}`); - return GitUtils.getPullRequestsMergedBetween(priorTag, inputTag); - }) - .then((pullRequestList) => { + const pullRequestList = GitUtils.getPullRequestsMergedBetween(priorTag, inputTag); console.log(`Found the pull request list: ${pullRequestList}`); return core.setOutput('PR_LIST', pullRequestList); }) @@ -126,52 +124,28 @@ module.exports = { /***/ ((module, __unused_webpack_exports, __nccwpck_require__) => { const _ = __nccwpck_require__(3571); -const {spawn} = __nccwpck_require__(3129); +const {execSync} = __nccwpck_require__(3129); /** * Get merge logs between two refs (inclusive) as a JavaScript object. * * @param {String} fromRef * @param {String} toRef - * @returns {Promise>} + * @returns {Object<{commit: String, subject: String}>} */ 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); - return new Promise((resolve, reject) => { - let stdout = ''; - let stderr = ''; - const spawnedProcess = spawn('git', ['log', '--format={"commit": "%H", "subject": "%s"},', `${fromRef}...${toRef}`]); - spawnedProcess.on('message', console.log); - spawnedProcess.stdout.on('data', (chunk) => { - console.log(chunk.toString()); - stdout += chunk.toString(); - }); - spawnedProcess.stderr.on('data', (chunk) => { - console.error(chunk.toString()); - stderr += chunk.toString(); - }); - spawnedProcess.on('close', (code) => { - if (code !== 0) { - return reject(new Error(`${stderr}`)); - } - - resolve(stdout); - }); - spawnedProcess.on('error', err => reject(err)); - }) - .then((stdout) => { - // Remove any double-quotes from commit subjects - let sanitizedOutput = stdout.replace(/(?<="subject": ").*(?="})/g, subject => subject.replace(/"/g, "'")); + const result = execSync(command).toString().trim(); - // Also remove any newlines - sanitizedOutput = sanitizedOutput.replace(/(\r\n|\n|\r)/gm, ''); + // 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); - }); + // Then format as JSON and convert to a proper JS object + const json = `[${sanitizedOutput}]`.replace('},]', '}]'); + return JSON.parse(json); } /** @@ -200,38 +174,33 @@ function getValidMergedPRs(commitMessages) { * * @param {String} fromRef * @param {String} toRef - * @returns {Promise>} – Pull request numbers + * @returns {Array} – Pull request numbers */ function getPullRequestsMergedBetween(fromRef, toRef) { - let targetMergeList; - return getMergeLogsAsJSON(fromRef, toRef) - .then((mergeList) => { - console.log(`Commits made between ${fromRef} and ${toRef}:`, mergeList); - targetMergeList = mergeList; - - // Get the full history on this branch, inclusive of the oldest commit from our target comparison - const oldestCommit = _.last(mergeList).commit; - return getMergeLogsAsJSON(oldestCommit, 'HEAD'); - }) - .then((fullMergeList) => { - // 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; - }); + 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 = { diff --git a/.github/libs/GitUtils.js b/.github/libs/GitUtils.js index 327ef6b2b5e4..931876037092 100644 --- a/.github/libs/GitUtils.js +++ b/.github/libs/GitUtils.js @@ -1,50 +1,26 @@ const _ = require('underscore'); -const {spawn} = require('child_process'); +const {execSync} = require('child_process'); /** * Get merge logs between two refs (inclusive) as a JavaScript object. * * @param {String} fromRef * @param {String} toRef - * @returns {Promise>} + * @returns {Object<{commit: String, subject: String}>} */ 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); - return new Promise((resolve, reject) => { - let stdout = ''; - let stderr = ''; - const spawnedProcess = spawn('git', ['log', '--format={"commit": "%H", "subject": "%s"},', `${fromRef}...${toRef}`]); - spawnedProcess.on('message', console.log); - spawnedProcess.stdout.on('data', (chunk) => { - console.log(chunk.toString()); - stdout += chunk.toString(); - }); - spawnedProcess.stderr.on('data', (chunk) => { - console.error(chunk.toString()); - stderr += chunk.toString(); - }); - spawnedProcess.on('close', (code) => { - if (code !== 0) { - return reject(new Error(`${stderr}`)); - } + const result = execSync(command).toString().trim(); - resolve(stdout); - }); - spawnedProcess.on('error', err => reject(err)); - }) - .then((stdout) => { - // Remove any double-quotes from commit subjects - let sanitizedOutput = stdout.replace(/(?<="subject": ").*(?="})/g, subject => subject.replace(/"/g, "'")); + // Remove any double-quotes from commit subjects + const sanitizedOutput = result + .replace(/(?<="subject": ").*(?="})/g, subject => subject.replace(/"/g, "'")); - // Also remove any newlines - sanitizedOutput = sanitizedOutput.replace(/(\r\n|\n|\r)/gm, ''); - - // Then format as JSON and convert to a proper JS object - const json = `[${sanitizedOutput}]`.replace('},]', '}]'); - return JSON.parse(json); - }); + // Then format as JSON and convert to a proper JS object + const json = `[${sanitizedOutput}]`.replace('},]', '}]'); + return JSON.parse(json); } /** @@ -73,38 +49,33 @@ function getValidMergedPRs(commitMessages) { * * @param {String} fromRef * @param {String} toRef - * @returns {Promise>} – Pull request numbers + * @returns {Array} – Pull request numbers */ function getPullRequestsMergedBetween(fromRef, toRef) { - let targetMergeList; - return getMergeLogsAsJSON(fromRef, toRef) - .then((mergeList) => { - console.log(`Commits made between ${fromRef} and ${toRef}:`, mergeList); - targetMergeList = mergeList; + 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'); - // Get the full history on this branch, inclusive of the oldest commit from our target comparison - const oldestCommit = _.last(mergeList).commit; - return getMergeLogsAsJSON(oldestCommit, 'HEAD'); - }) - .then((fullMergeList) => { - // 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)); + // 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; - }); + // 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 = { diff --git a/tests/unit/getPullRequestsMergedBetweenTest.sh b/tests/unit/getPullRequestsMergedBetweenTest.sh index 538de83700b4..4611a2c6c825 100755 --- a/tests/unit/getPullRequestsMergedBetweenTest.sh +++ b/tests/unit/getPullRequestsMergedBetweenTest.sh @@ -6,7 +6,7 @@ set -e TEST_DIR=$(dirname "$(dirname "$(cd "$(dirname "$0")" || exit 1;pwd)/$(basename "$0")")") SCRIPTS_DIR="$TEST_DIR/../scripts" DUMMY_DIR="$HOME/DumDumRepo" -getPullRequestsMergedBetween="$TEST_DIR/utils/getPullRequestsMergedBetween.mjs" +getPullRequestsMergedBetween="$TEST_DIR/utils/getPullRequestsMergedBetween.js" source "$SCRIPTS_DIR/shellUtils.sh" diff --git a/tests/utils/getPullRequestsMergedBetween.js b/tests/utils/getPullRequestsMergedBetween.js new file mode 100755 index 000000000000..6fcb96960fb0 --- /dev/null +++ b/tests/utils/getPullRequestsMergedBetween.js @@ -0,0 +1,12 @@ +#!/usr/bin/env node + +const GitUtils = require('../../.github/libs/GitUtils'); + +/* eslint-disable no-console */ +const realConsoleLog = console.log; +console.log = () => {}; + +const output = GitUtils.getPullRequestsMergedBetween(process.argv[2], process.argv[3]); + +console.log = realConsoleLog; +console.log(output); diff --git a/tests/utils/getPullRequestsMergedBetween.mjs b/tests/utils/getPullRequestsMergedBetween.mjs deleted file mode 100755 index 05dedaa3e4f1..000000000000 --- a/tests/utils/getPullRequestsMergedBetween.mjs +++ /dev/null @@ -1,15 +0,0 @@ -#!/usr/bin/env node - -import GitUtils from '../../.github/libs/GitUtils.js'; - -const fromRef = process.argv[2]; -const toRef = process.argv[3]; - -/* eslint-disable no-console */ -const realConsoleLog = console.log; -console.log = () => {}; - -const output = await GitUtils.getPullRequestsMergedBetween(fromRef, toRef); - -console.log = realConsoleLog; -console.log(output);