From 9a27672e99b3292647980b80a7ab67f12f99c410 Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 15 Nov 2023 13:44:13 -0800 Subject: [PATCH 01/20] Create callable workflow to create the checklist --- .github/workflows/checklist.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 .github/workflows/checklist.yml diff --git a/.github/workflows/checklist.yml b/.github/workflows/checklist.yml new file mode 100644 index 000000000000..307c72177c76 --- /dev/null +++ b/.github/workflows/checklist.yml @@ -0,0 +1,17 @@ +name: Create or update deploy checklist + +on: + workflow_call: + workflow_dispatch: + +jobs: + createChecklist: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: ./.github/actions/composite/setupNode + + - uses: ./.github/actions/javascript/createOrUpdateStagingDeploy + with: + GITHUB_TOKEN: ${{ github.token }} From 91e516de1ce4ea70c8b8a8f48bb3e341a1e01b48 Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 15 Nov 2023 13:48:01 -0800 Subject: [PATCH 02/20] Add pretty name to each step --- .github/workflows/checklist.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/checklist.yml b/.github/workflows/checklist.yml index 307c72177c76..9a1cac41ed69 100644 --- a/.github/workflows/checklist.yml +++ b/.github/workflows/checklist.yml @@ -8,10 +8,13 @@ jobs: createChecklist: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - name: Checkout + uses: actions/checkout@v4 - - uses: ./.github/actions/composite/setupNode + - name: Setup Node + uses: ./.github/actions/composite/setupNode - - uses: ./.github/actions/javascript/createOrUpdateStagingDeploy + - name: Create or update deploy checklist + uses: ./.github/actions/javascript/createOrUpdateStagingDeploy with: GITHUB_TOKEN: ${{ github.token }} From 92ca8cfa300c57cd7432cb0230d3b9cd06c88c8e Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 15 Nov 2023 13:49:06 -0800 Subject: [PATCH 03/20] Rename checklist.yml to createDeployChecklist.yml --- .github/workflows/{checklist.yml => createDeployChecklist.yml} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .github/workflows/{checklist.yml => createDeployChecklist.yml} (100%) diff --git a/.github/workflows/checklist.yml b/.github/workflows/createDeployChecklist.yml similarity index 100% rename from .github/workflows/checklist.yml rename to .github/workflows/createDeployChecklist.yml From 5c4850948a6d1bfcce540b3b9669c3e133db25b4 Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 15 Nov 2023 15:51:05 -0800 Subject: [PATCH 04/20] Use callable workflow in platformDeploy.yml --- .github/workflows/platformDeploy.yml | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/.github/workflows/platformDeploy.yml b/.github/workflows/platformDeploy.yml index d494ea0d008b..7890fda04ccd 100644 --- a/.github/workflows/platformDeploy.yml +++ b/.github/workflows/platformDeploy.yml @@ -36,24 +36,9 @@ jobs: # Note: we're updating the checklist before running the deploys and assuming that it will succeed on at least one platform deployChecklist: name: Create or update deploy checklist - runs-on: ubuntu-latest + uses: ./.github/workflows/createDeployChecklist.yml if: ${{ github.event_name != 'release' }} needs: validateActor - steps: - - name: Checkout - uses: actions/checkout@v3 - - name: Setup Node - uses: Expensify/App/.github/actions/composite/setupNode@main - - - name: Set version - id: getVersion - run: echo "VERSION=$(npm run print-version --silent)" >> "$GITHUB_OUTPUT" - - - name: Create or update staging deploy - uses: Expensify/App/.github/actions/javascript/createOrUpdateStagingDeploy@main - with: - GITHUB_TOKEN: ${{ secrets.OS_BOTIFY_TOKEN }} - NPM_VERSION: ${{ steps.getVersion.outputs.VERSION }} android: name: Build and deploy Android From 60c172fd3870f7b03c295fb4ed13a2f12a331875 Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 15 Nov 2023 15:51:30 -0800 Subject: [PATCH 05/20] Use callable workflow in deployBlocker.yml --- .github/workflows/deployBlocker.yml | 59 ++++++++++++----------------- 1 file changed, 24 insertions(+), 35 deletions(-) diff --git a/.github/workflows/deployBlocker.yml b/.github/workflows/deployBlocker.yml index f42d19ca8241..a89b1d3827dc 100644 --- a/.github/workflows/deployBlocker.yml +++ b/.github/workflows/deployBlocker.yml @@ -6,35 +6,21 @@ on: - labeled jobs: + updateChecklist: + if: github.event.label.name == 'DeployBlockerCash' + uses: ./.github/workflows/createDeployChecklist.yml + deployBlocker: + if: github.event.label.name == 'DeployBlockerCash' runs-on: ubuntu-latest - if: ${{ github.event.label.name == 'DeployBlockerCash' }} - steps: - name: Checkout - uses: actions/checkout@v3 - with: - token: ${{ secrets.GITHUB_TOKEN }} - - - name: Get URL, title, & number of new deploy blocker (issue) - if: ${{ github.event_name == 'issues' }} - env: - TITLE: ${{ github.event.issue.title }} - run: | - { echo "DEPLOY_BLOCKER_URL=${{ github.event.issue.html_url }}"; - echo "DEPLOY_BLOCKER_NUMBER=${{ github.event.issue.number }}"; - echo "DEPLOY_BLOCKER_TITLE=$(sed -e "s/'/'\\\\''/g; s/\`/\\\\\`/g; 1s/^/'/; \$s/\$/'/" <<< "$TITLE")";} >> "$GITHUB_ENV" - - - name: Update StagingDeployCash with new deploy blocker - uses: Expensify/App/.github/actions/javascript/createOrUpdateStagingDeploy@main - with: - GITHUB_TOKEN: ${{ secrets.OS_BOTIFY_TOKEN }} + uses: actions/checkout@v4 - name: Give the issue/PR the Hourly, Engineering labels - uses: andymckay/labeler@978f846c4ca6299fd136f465b42c5e87aca28cac - with: - add-labels: 'Hourly, Engineering' - remove-labels: 'Daily, Weekly, Monthly' + run: gh issue edit --add-label 'Engineering,Hourly' --remove-label 'Daily,Weekly,Monthly' + env: + GITHUB_TOKEN: ${{ github.token }} - name: 'Post the issue in the #expensify-open-source slack room' if: ${{ success() }} @@ -46,26 +32,29 @@ jobs: channel: '#expensify-open-source', attachments: [{ color: "#DB4545", - text: '💥 We have found a New Expensify Deploy Blocker, if you have any idea which PR could be causing this, please comment in the issue: <${{ env.DEPLOY_BLOCKER_URL }}|'+ `${{ env.DEPLOY_BLOCKER_TITLE }}`.replace(/(^'|'$)/gi, '').replace(/'\''/gi,'\'') + '>', + text: '💥 We have found a New Expensify Deploy Blocker, if you have any idea which PR could be causing this, please comment in the issue: <${{ github.event.issue.html_url }}|${{ toJSON(github.event.issue.title) }}>', }] } env: GITHUB_TOKEN: ${{ github.token }} SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK }} - - name: Comment on deferred PR - uses: actions-ecosystem/action-create-comment@cd098164398331c50e7dfdd0dfa1b564a1873fac - with: - github_token: ${{ secrets.OS_BOTIFY_TOKEN }} - number: ${{ env.DEPLOY_BLOCKER_NUMBER }} - body: | - :wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! [Check out the open `StagingDeployCash` deploy checklist](https://github.com/Expensify/App/issues?q=is%3Aopen+is%3Aissue+label%3AStagingDeployCash) to see the list of PRs included in this release, then work quickly to do one of the following: - 1. Identify the pull request that introduced this issue and revert it. - 2. Find someone who can quickly fix the issue. - 3. Fix the issue yourself. + - name: Comment on deploy blocker + run: | + gh issue comment ${{ github.event.issue.number }} --body "$(cat <<'EOF' + :wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! [Check out the open \`StagingDeployCash\` deploy checklist](https://github.com/Expensify/App/issues?q=is%3Aopen+is%3Aissue+label%3AStagingDeployCash) to see the list of PRs included in this release, then work quickly to do one of the following: + + 1. Identify the pull request that introduced this issue and revert it. + 2. Find someone who can quickly fix the issue. + 3. Fix the issue yourself. + + EOF + )" + env: + GITHUB_TOKEN: ${{ github.token }} - name: Announce failed workflow in Slack if: ${{ failure() }} - uses: Expensify/App/.github/actions/composite/announceFailedWorkflowInSlack@main + uses: ./.github/actions/composite/announceFailedWorkflowInSlack with: SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK }} From 4cc559930e16481cc2b7497b41b07c18a962da88 Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 15 Nov 2023 16:33:46 -0800 Subject: [PATCH 06/20] Update outdated utils function in preGenerateTest --- workflow_tests/README.md | 24 ++++++++++++------------ workflow_tests/utils/preGenerateTest.js | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/workflow_tests/README.md b/workflow_tests/README.md index f5975a421120..0c491fe6d9e0 100644 --- a/workflow_tests/README.md +++ b/workflow_tests/README.md @@ -89,8 +89,8 @@ act = utils.setUpActParams( ); ``` -### `getMockStep` -`getMockStep` allows for creating uniform mock step definitions compatible with `Act-js` and reduces time required, as well as possibility of errors/typos slipping in while developing tests. More complex behaviours have to be mocked manually +### `createMockStep` +`createMockStep` allows for creating uniform mock step definitions compatible with `Act-js` and reduces time required, as well as possibility of errors/typos slipping in while developing tests. More complex behaviours have to be mocked manually Parameters: - `name` - name of the step that **must correspond to the `name` in the `.yml` file**, otherwise the step cannot be found @@ -106,7 +106,7 @@ Returns an object with step mock definition, ready to be provided to the `Act` o Example: ```javascript -let mockStep = utils.getMockStep( +let mockStep = utils.createMockStep( 'Name of the step from .yml', 'Message to be printed', 'TEST_JOB', @@ -135,8 +135,8 @@ results in } ``` -### `getStepAssertion` -`getStepAssertion` allows for creating uniform assertions for output from executed step, compatible with step mocks provided by `getMockStep` +### `createStepAssertion` +`createStepAssertion` allows for creating uniform assertions for output from executed step, compatible with step mocks provided by `createMockStep` Parameters: - `name` - name of the step, **has to correspond to the name from `.yml` file**, and the name in the step mock if applicable @@ -151,7 +151,7 @@ Returns an object with step expected output definition ready to be provided to ` Example: ```javascript -utils.getStepAssertion( +utils.createStepAssertion( 'Name of the step from .yml', false, null, @@ -294,8 +294,8 @@ describe('test some general behaviour', () => { }, ], '': [ - utils.getMockStep('', ''), - utils.getMockStep('', ''), + utils.createMockStep('', ''), + utils.createMockStep('', ''), ], }; @@ -378,7 +378,7 @@ act = utils.setUpActParams( '', ); ``` -Set up step mocks. Here we configure which steps in the workflow should be mocked, and with what behaviour. This takes form of an object with keys corresponding to the names of the jobs in the workflow, and values being mock definitions for specific steps. The steps can be identified either by `id`, `name`, `uses` or `run`. Step mock can be defined either by hand (``) or with the helper method `utils.getMockStep()` (``). Not mocked steps will be executed normally - **make sure this will not have unexpected consequences** +Set up step mocks. Here we configure which steps in the workflow should be mocked, and with what behaviour. This takes form of an object with keys corresponding to the names of the jobs in the workflow, and values being mock definitions for specific steps. The steps can be identified either by `id`, `name`, `uses` or `run`. Step mock can be defined either by hand (``) or with the helper method `utils.createMockStep()` (``). Not mocked steps will be executed normally - **make sure this will not have unexpected consequences** ```javascript const testMockSteps = { '': [ @@ -392,8 +392,8 @@ const testMockSteps = { }, ], '': [ - utils.getMockStep('', ''), - utils.getMockStep('', ''), + utils.createMockStep('', ''), + utils.createMockStep('', ''), ], }; ``` @@ -405,7 +405,7 @@ const result = await act mockSteps: testMockSteps, }); ``` -Assert results are as expected. This can, for example, include using `expect()` to check if the steps that should be executed have indeed been executed, steps that shouldn't run have not been executed, compare statuses (which steps succeeded, which failed) and step outputs. Outputs can include additional information, like input values, environmental variables, secrets (although these are usually not accessible and represented by `***`, this can still be useful to check if the value exists or not). Here it's usually done with the helper assertion methods defined in the assertions file. Step assertions can be created manually or with `getStepAssertion()` helper method +Assert results are as expected. This can, for example, include using `expect()` to check if the steps that should be executed have indeed been executed, steps that shouldn't run have not been executed, compare statuses (which steps succeeded, which failed) and step outputs. Outputs can include additional information, like input values, environmental variables, secrets (although these are usually not accessible and represented by `***`, this can still be useful to check if the value exists or not). Here it's usually done with the helper assertion methods defined in the assertions file. Step assertions can be created manually or with `createStepAssertion()` helper method ```javascript assertions.assertSomethingHappened(result); assertions.assertSomethingDidNotHappen(result, false); diff --git a/workflow_tests/utils/preGenerateTest.js b/workflow_tests/utils/preGenerateTest.js index 4ed485abec40..df4a12f68dc6 100644 --- a/workflow_tests/utils/preGenerateTest.js +++ b/workflow_tests/utils/preGenerateTest.js @@ -96,7 +96,7 @@ describe('test workflow ${workflowName}', () => { }); `; const mockStepTemplate = (stepMockName, step, jobId) => ` -const ${stepMockName} = utils.getMockStep( +const ${stepMockName} = utils.createMockStep( '${step.name || ''}', '${step.name || ''}', ${jobId ? `'${jobId.toUpperCase()}'` : 'null'}, @@ -105,7 +105,7 @@ const ${stepMockName} = utils.getMockStep( // add outputs if needed );`; const stepAssertionTemplate = (step_name, job_id, step_message, inputs, envs) => ` - utils.getStepAssertion( + utils.createStepAssertion( '${step_name}', true, null, From 4afd9be137595d6d160c53e1871691ea6feffbf2 Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 15 Nov 2023 16:34:03 -0800 Subject: [PATCH 07/20] Include workflow_tests/repo in .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 335efdc5586a..3ed0ef54523a 100644 --- a/.gitignore +++ b/.gitignore @@ -107,6 +107,7 @@ tsconfig.tsbuildinfo # Workflow test logs /workflow_tests/logs/ +/workflow_tests/repo/ # Yalc .yalc From ea5302588a338e4c7bf052c6dcca20f9ef08cdd9 Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 16 Nov 2023 13:01:33 -0800 Subject: [PATCH 08/20] Use version from package.json instead of input --- .../createOrUpdateStagingDeploy/action.yml | 4 +--- .../createOrUpdateStagingDeploy.js | 15 +++++++-------- .../createOrUpdateStagingDeploy/index.js | 15 +++++++-------- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/.github/actions/javascript/createOrUpdateStagingDeploy/action.yml b/.github/actions/javascript/createOrUpdateStagingDeploy/action.yml index 870cab318d09..b81e5d31a603 100644 --- a/.github/actions/javascript/createOrUpdateStagingDeploy/action.yml +++ b/.github/actions/javascript/createOrUpdateStagingDeploy/action.yml @@ -4,9 +4,7 @@ inputs: GITHUB_TOKEN: description: Auth token for New Expensify Github required: true - NPM_VERSION: - description: The new NPM version of the StagingDeployCash issue. - required: false + runs: using: 'node16' main: './index.js' diff --git a/.github/actions/javascript/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js b/.github/actions/javascript/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js index f81a181cb8d3..db657829b571 100644 --- a/.github/actions/javascript/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js +++ b/.github/actions/javascript/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js @@ -1,3 +1,4 @@ +const fs = require('fs'); const format = require('date-fns/format'); const _ = require('underscore'); const core = require('@actions/core'); @@ -6,8 +7,8 @@ const GithubUtils = require('../../../libs/GithubUtils'); const GitUtils = require('../../../libs/GitUtils'); async function run() { - const newVersion = core.getInput('NPM_VERSION'); - console.log('New version found from action input:', newVersion); + // Note: require('package.json').version does not work because ncc will resolve that to a plain string at compile time + const newVersionTag = JSON.parse(fs.readFileSync('package.json')).version; try { // Start by fetching the list of recent StagingDeployCash issues, along with the list of open deploy blockers @@ -35,14 +36,12 @@ async function run() { const currentChecklistData = shouldCreateNewDeployChecklist ? {} : GithubUtils.getStagingDeployCashData(mostRecentChecklist); // Find the list of PRs merged between the current checklist and the previous checklist - // Note that any time we're creating a new checklist we MUST have `NPM_VERSION` passed in as an input - const newTag = newVersion || _.get(currentChecklistData, 'tag'); - const mergedPRs = await GitUtils.getPullRequestsMergedBetween(previousChecklistData.tag, newTag); + const mergedPRs = await GitUtils.getPullRequestsMergedBetween(previousChecklistData.tag, newVersionTag); // Next, we generate the checklist body let checklistBody = ''; if (shouldCreateNewDeployChecklist) { - checklistBody = await GithubUtils.generateStagingDeployCashBody(newTag, _.map(mergedPRs, GithubUtils.getPullRequestURLFromNumber)); + checklistBody = await GithubUtils.generateStagingDeployCashBody(newVersionTag, _.map(mergedPRs, GithubUtils.getPullRequestURLFromNumber)); } else { // Generate the updated PR list, preserving the previous state of `isVerified` for existing PRs const PRList = _.reduce( @@ -94,9 +93,9 @@ async function run() { }); } - const didVersionChange = newVersion ? newVersion !== currentChecklistData.tag : false; + const didVersionChange = newVersionTag ? newVersionTag !== currentChecklistData.tag : false; checklistBody = await GithubUtils.generateStagingDeployCashBody( - newTag, + newVersionTag, _.pluck(PRList, 'url'), _.pluck(_.where(PRList, {isVerified: true}), 'url'), _.pluck(deployBlockers, 'url'), diff --git a/.github/actions/javascript/createOrUpdateStagingDeploy/index.js b/.github/actions/javascript/createOrUpdateStagingDeploy/index.js index b7ab57e68974..a6260a0c77fc 100644 --- a/.github/actions/javascript/createOrUpdateStagingDeploy/index.js +++ b/.github/actions/javascript/createOrUpdateStagingDeploy/index.js @@ -7,6 +7,7 @@ /***/ 3926: /***/ ((module, __unused_webpack_exports, __nccwpck_require__) => { +const fs = __nccwpck_require__(7147); const format = __nccwpck_require__(2168); const _ = __nccwpck_require__(5067); const core = __nccwpck_require__(2186); @@ -15,8 +16,8 @@ const GithubUtils = __nccwpck_require__(7999); const GitUtils = __nccwpck_require__(669); async function run() { - const newVersion = core.getInput('NPM_VERSION'); - console.log('New version found from action input:', newVersion); + // Note: require('package.json').version does not work because ncc will resolve that to a plain string at compile time + const newVersionTag = JSON.parse(fs.readFileSync('package.json')).version; try { // Start by fetching the list of recent StagingDeployCash issues, along with the list of open deploy blockers @@ -44,14 +45,12 @@ async function run() { const currentChecklistData = shouldCreateNewDeployChecklist ? {} : GithubUtils.getStagingDeployCashData(mostRecentChecklist); // Find the list of PRs merged between the current checklist and the previous checklist - // Note that any time we're creating a new checklist we MUST have `NPM_VERSION` passed in as an input - const newTag = newVersion || _.get(currentChecklistData, 'tag'); - const mergedPRs = await GitUtils.getPullRequestsMergedBetween(previousChecklistData.tag, newTag); + const mergedPRs = await GitUtils.getPullRequestsMergedBetween(previousChecklistData.tag, newVersionTag); // Next, we generate the checklist body let checklistBody = ''; if (shouldCreateNewDeployChecklist) { - checklistBody = await GithubUtils.generateStagingDeployCashBody(newTag, _.map(mergedPRs, GithubUtils.getPullRequestURLFromNumber)); + checklistBody = await GithubUtils.generateStagingDeployCashBody(newVersionTag, _.map(mergedPRs, GithubUtils.getPullRequestURLFromNumber)); } else { // Generate the updated PR list, preserving the previous state of `isVerified` for existing PRs const PRList = _.reduce( @@ -103,9 +102,9 @@ async function run() { }); } - const didVersionChange = newVersion ? newVersion !== currentChecklistData.tag : false; + const didVersionChange = newVersionTag ? newVersionTag !== currentChecklistData.tag : false; checklistBody = await GithubUtils.generateStagingDeployCashBody( - newTag, + newVersionTag, _.pluck(PRList, 'url'), _.pluck(_.where(PRList, {isVerified: true}), 'url'), _.pluck(deployBlockers, 'url'), From fa629c3b7b15108a8be778bf2f18cbe3cb4332ef Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 16 Nov 2023 13:04:13 -0800 Subject: [PATCH 09/20] Sort PRs for better readability in workflow logs --- .github/actions/javascript/createOrUpdateStagingDeploy/index.js | 2 +- .github/actions/javascript/getDeployPullRequestList/index.js | 2 +- .github/libs/GitUtils.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/actions/javascript/createOrUpdateStagingDeploy/index.js b/.github/actions/javascript/createOrUpdateStagingDeploy/index.js index a6260a0c77fc..6bba68f21e69 100644 --- a/.github/actions/javascript/createOrUpdateStagingDeploy/index.js +++ b/.github/actions/javascript/createOrUpdateStagingDeploy/index.js @@ -320,7 +320,7 @@ function getPullRequestsMergedBetween(fromTag, toTag) { console.log(`Commits made between ${fromTag} and ${toTag}:`, commitList); // Find which commit messages correspond to merged PR's - const pullRequestNumbers = getValidMergedPRs(commitList); + const pullRequestNumbers = getValidMergedPRs(commitList).sort(); console.log(`List of pull requests merged between ${fromTag} and ${toTag}`, pullRequestNumbers); return _.map(pullRequestNumbers, (prNum) => Number.parseInt(prNum, 10)); }); diff --git a/.github/actions/javascript/getDeployPullRequestList/index.js b/.github/actions/javascript/getDeployPullRequestList/index.js index 1217c5e97de4..a16b3612960e 100644 --- a/.github/actions/javascript/getDeployPullRequestList/index.js +++ b/.github/actions/javascript/getDeployPullRequestList/index.js @@ -263,7 +263,7 @@ function getPullRequestsMergedBetween(fromTag, toTag) { console.log(`Commits made between ${fromTag} and ${toTag}:`, commitList); // Find which commit messages correspond to merged PR's - const pullRequestNumbers = getValidMergedPRs(commitList); + const pullRequestNumbers = getValidMergedPRs(commitList).sort(); console.log(`List of pull requests merged between ${fromTag} and ${toTag}`, pullRequestNumbers); return _.map(pullRequestNumbers, (prNum) => Number.parseInt(prNum, 10)); }); diff --git a/.github/libs/GitUtils.js b/.github/libs/GitUtils.js index 42a7ecff1263..e10e3feaa0ab 100644 --- a/.github/libs/GitUtils.js +++ b/.github/libs/GitUtils.js @@ -136,7 +136,7 @@ function getPullRequestsMergedBetween(fromTag, toTag) { console.log(`Commits made between ${fromTag} and ${toTag}:`, commitList); // Find which commit messages correspond to merged PR's - const pullRequestNumbers = getValidMergedPRs(commitList); + const pullRequestNumbers = getValidMergedPRs(commitList).sort(); console.log(`List of pull requests merged between ${fromTag} and ${toTag}`, pullRequestNumbers); return _.map(pullRequestNumbers, (prNum) => Number.parseInt(prNum, 10)); }); From ff2514c9d214fbe764a4b59ff655ece745ebfbe7 Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 16 Nov 2023 14:23:53 -0800 Subject: [PATCH 10/20] Log repack command --- .../javascript/createOrUpdateStagingDeploy/index.js | 7 +++++-- .../actions/javascript/getDeployPullRequestList/index.js | 7 +++++-- .github/libs/GitUtils.js | 7 +++++-- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/.github/actions/javascript/createOrUpdateStagingDeploy/index.js b/.github/actions/javascript/createOrUpdateStagingDeploy/index.js index 6bba68f21e69..48751162fd10 100644 --- a/.github/actions/javascript/createOrUpdateStagingDeploy/index.js +++ b/.github/actions/javascript/createOrUpdateStagingDeploy/index.js @@ -197,13 +197,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 diff --git a/.github/actions/javascript/getDeployPullRequestList/index.js b/.github/actions/javascript/getDeployPullRequestList/index.js index a16b3612960e..c02e86099b5b 100644 --- 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 diff --git a/.github/libs/GitUtils.js b/.github/libs/GitUtils.js index e10e3feaa0ab..6b1350996fc1 100644 --- 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 From 16d56b18c7b2f8beb89ebee56039ed4cffdb034e Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 16 Nov 2023 16:34:09 -0800 Subject: [PATCH 11/20] Add success log --- tests/unit/CIGitLogicTest.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/unit/CIGitLogicTest.sh b/tests/unit/CIGitLogicTest.sh index e9ec20bb74a6..5ddc9bbdcf4b 100755 --- a/tests/unit/CIGitLogicTest.sh +++ b/tests/unit/CIGitLogicTest.sh @@ -389,10 +389,12 @@ 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' '[ 9, 7, 6, 5, 2 ]' # 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' '[ 10, 8 ]' + +success "Scenario #6 completed successfully!" ### Cleanup title "Cleaning up..." From 86b8231fb24819ed813d6eb8f7d2bf7d429d37b1 Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 16 Nov 2023 16:50:51 -0800 Subject: [PATCH 12/20] Fix sorting --- .../createOrUpdateStagingDeploy/index.js | 15 +++++++-------- .../javascript/getDeployPullRequestList/index.js | 15 +++++++-------- .github/libs/GitUtils.js | 15 +++++++-------- tests/unit/CIGitLogicTest.sh | 14 +++++++------- 4 files changed, 28 insertions(+), 31 deletions(-) diff --git a/.github/actions/javascript/createOrUpdateStagingDeploy/index.js b/.github/actions/javascript/createOrUpdateStagingDeploy/index.js index 48751162fd10..86840c9ae4fa 100644 --- a/.github/actions/javascript/createOrUpdateStagingDeploy/index.js +++ b/.github/actions/javascript/createOrUpdateStagingDeploy/index.js @@ -317,16 +317,15 @@ function getValidMergedPRs(commits) { * @param {String} toTag * @returns {Promise>} – 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).sort(); - 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 index c02e86099b5b..af691cfb6d1d 100644 --- a/.github/actions/javascript/getDeployPullRequestList/index.js +++ b/.github/actions/javascript/getDeployPullRequestList/index.js @@ -260,16 +260,15 @@ function getValidMergedPRs(commits) { * @param {String} toTag * @returns {Promise>} – 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).sort(); - 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 index 6b1350996fc1..fa2cf430b277 100644 --- a/.github/libs/GitUtils.js +++ b/.github/libs/GitUtils.js @@ -133,16 +133,15 @@ function getValidMergedPRs(commits) { * @param {String} toTag * @returns {Promise>} – 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).sort(); - 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 index 5ddc9bbdcf4b..8374bcaf1f89 100755 --- 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 ]' success "Scenario #6 completed successfully!" From 9e27cd1cb586a1801c900124e40c4367e2788daf Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 17 Nov 2023 09:37:09 -0800 Subject: [PATCH 13/20] Create create_basic_pr utility function --- tests/unit/CIGitLogicTest.sh | 42 ++++++++++++++---------------------- 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/tests/unit/CIGitLogicTest.sh b/tests/unit/CIGitLogicTest.sh index 8374bcaf1f89..6f4dc16723dd 100755 --- a/tests/unit/CIGitLogicTest.sh +++ b/tests/unit/CIGitLogicTest.sh @@ -111,6 +111,18 @@ function update_production_from_staging { success "Recreated production from staging!" } +function create_basic_pr { + info "Creating PR #$1..." + checkout_repo + setup_git_as_human + git pull + git switch -c "pr-$1" + echo "Changes from PR #$1" >> "PR$1.txt" + git add "PR$1.txt" + git commit -m "Changes from PR #$1" + success "Created PR #$1 in branch pr-$1" +} + function merge_pr { info "Merging PR #$1 to main" git switch main @@ -192,14 +204,7 @@ success "Setup complete!" title "Scenario #1: Merge a pull request while the checklist is unlocked" -info "Creating PR #1..." -setup_git_as_human -git switch -c pr-1 -echo "Changes from PR #1" >> PR1.txt -git add PR1.txt -git commit -m "Changes from PR #1" -success "Created PR #1 in branch pr-1" - +create_basic_pr 1 merge_pr 1 bump_version "$SEMVER_LEVEL_BUILD" update_staging_from_main @@ -216,23 +221,14 @@ success "Scenario #1 completed successfully!" title "Scenario #2: Merge a pull request with the checklist locked, but don't CP it" -info "Creating PR #2..." -setup_git_as_human -git switch -c pr-2 -echo "Changes from PR #2" >> PR2.txt -git add PR2.txt -git commit -m "Changes from PR #2" +create_basic_pr 2 merge_pr 2 success "Scenario #2 completed successfully!" title "Scenario #3: Merge a pull request with the checklist locked and CP it to staging" -info "Creating PR #3 and merging it into main..." -git switch -c pr-3 -echo "Changes from PR #3" >> PR3.txt -git add PR3.txt -git commit -m "Changes from PR #3" +create_basic_pr 3 cherry_pick_pr 3 tag_staging @@ -270,13 +266,7 @@ success "Scenario #4B completed successfully!" title "Scenario #5: Merging another pull request when the checklist is unlocked" -info "Creating PR #5..." -setup_git_as_human -git switch main -git switch -c pr-5 -echo "Changes from PR #5" >> PR5.txt -git add PR5.txt -git commit -m "Changes from PR #5" +create_basic_pr 5 merge_pr 5 bump_version "$SEMVER_LEVEL_BUILD" From e5ec3b417b0713b0a63cf2d544209d7ce870c097 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 17 Nov 2023 09:51:34 -0800 Subject: [PATCH 14/20] DRY up script further with deploy_staging and deploy_production --- tests/unit/CIGitLogicTest.sh | 66 +++++++++++++++++------------------- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/tests/unit/CIGitLogicTest.sh b/tests/unit/CIGitLogicTest.sh index 6f4dc16723dd..2c6bc7025770 100755 --- a/tests/unit/CIGitLogicTest.sh +++ b/tests/unit/CIGitLogicTest.sh @@ -158,6 +158,8 @@ function cherry_pick_pr { git push origin staging info "Merged PR #$(($1 + 1)) into staging" + tag_staging + success "Successfully cherry-picked PR #$1 to staging!" } @@ -173,6 +175,29 @@ function tag_staging { success "Created new tag $(print_version)" } +function deploy_staging { + info "Deploying staging..." + checkout_repo + bump_version "$SEMVER_LEVEL_BUILD" + update_staging_from_main + tag_staging + success "Deployed v$(print_version) to staging!" +} + +function deploy_production { + info "Checklist closed, deploying production and staging..." + + info "Deploying production..." + update_production_from_staging + success "Deployed v$(print_version) to production!" + + info "Deploying staging..." + bump_version "$SEMVER_LEVEL_PATCH" + update_staging_from_main + tag_staging + success "Deployed v$(print_version) to staging!" +} + function assert_prs_merged_between { checkout_repo output=$(node "$getPullRequestsMergedBetween" "$1" "$2") @@ -206,12 +231,7 @@ title "Scenario #1: Merge a pull request while the checklist is unlocked" create_basic_pr 1 merge_pr 1 -bump_version "$SEMVER_LEVEL_BUILD" -update_staging_from_main - -# Tag staging -tag_staging -git switch main +deploy_staging # Verify output for checklist and deploy comment assert_prs_merged_between '1.0.0-0' '1.0.0-1' "[ 1 ]" @@ -231,8 +251,6 @@ title "Scenario #3: Merge a pull request with the checklist locked and CP it to create_basic_pr 3 cherry_pick_pr 3 -tag_staging - # Verify output for checklist assert_prs_merged_between '1.0.0-0' '1.0.0-2' "[ 1, 3 ]" @@ -243,35 +261,23 @@ success "Scenario #3 completed successfully!" title "Scenario #4: Close the checklist" -title "Scenario #4A: Run the production deploy" -update_production_from_staging +deploy_production # Verify output for release body and production deploy comments assert_prs_merged_between '1.0.0-0' '1.0.0-2' "[ 1, 3 ]" -success "Scenario #4A completed successfully!" - -title "Scenario #4B: Run the staging deploy and create a new checklist" - -bump_version "$SEMVER_LEVEL_PATCH" -update_staging_from_main -tag_staging - # Verify output for new checklist and staging deploy comments assert_prs_merged_between '1.0.0-2' '1.0.1-0' "[ 2 ]" -success "Scenario #4B completed successfully!" +success "Scenario #4 completed successfully!" title "Scenario #5: Merging another pull request when the checklist is unlocked" create_basic_pr 5 merge_pr 5 - -bump_version "$SEMVER_LEVEL_BUILD" -update_staging_from_main -tag_staging +deploy_staging # Verify output for checklist assert_prs_merged_between '1.0.0-2' '1.0.1-1' "[ 2, 5 ]" @@ -292,9 +298,7 @@ git add myFile.txt git commit -m "Add myFile.txt in PR #6" merge_pr 6 -bump_version "$SEMVER_LEVEL_BUILD" -update_staging_from_main -tag_staging +deploy_staging # Verify output for checklist assert_prs_merged_between '1.0.0-2' '1.0.1-2' "[ 2, 5, 6 ]" @@ -314,9 +318,7 @@ git add myFile.txt git commit -m "Append and prepend content in myFile.txt" merge_pr 7 -bump_version "$SEMVER_LEVEL_BUILD" -update_staging_from_main -tag_staging +deploy_staging # Verify output for checklist assert_prs_merged_between '1.0.0-2' '1.0.1-3' "[ 2, 5, 6, 7 ]" @@ -345,7 +347,6 @@ git add myFile.txt git commit -m "Revert append and prepend" cherry_pick_pr 9 -tag_staging info "Verifying that the revert is present on staging, but the unrelated change is not" if [[ "$(cat myFile.txt)" != "some content" ]]; then @@ -373,10 +374,7 @@ git add myFile.txt git commit -m "Append and prepend content in myFile.txt" merge_pr 10 -update_production_from_staging -bump_version "$SEMVER_LEVEL_PATCH" -update_staging_from_main -tag_staging +deploy_production # Verify production release list assert_prs_merged_between '1.0.0-2' '1.0.1-4' '[ 2, 5, 6, 7, 9 ]' From 5b333186dff6c0d834d98cee0825df8f53c0ccfd Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 17 Nov 2023 10:43:49 -0800 Subject: [PATCH 15/20] Add another test case --- tests/unit/CIGitLogicTest.sh | 37 ++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/unit/CIGitLogicTest.sh b/tests/unit/CIGitLogicTest.sh index 2c6bc7025770..f35ead3542d3 100755 --- a/tests/unit/CIGitLogicTest.sh +++ b/tests/unit/CIGitLogicTest.sh @@ -165,6 +165,7 @@ function cherry_pick_pr { function tag_staging { info "Tagging new version from the staging branch..." + checkout_repo setup_git_as_osbotify if ! git rev-parse --verify staging 2>/dev/null; then git fetch origin staging --depth=1 @@ -384,6 +385,42 @@ assert_prs_merged_between '1.0.1-4' '1.0.2-0' '[ 8, 10 ]' success "Scenario #6 completed successfully!" +title "Scenario #7: Force-pushing to a branch after rebasing older commits" + +create_basic_pr 11 +git push origin pr-11 + +create_basic_pr 12 +merge_pr 12 +deploy_staging + +# Verify PRs for checklist +assert_prs_merged_between '1.0.1-4' '1.0.2-1' '[ 8, 10, 12 ]' + +# Verify PRs for deploy comments +assert_prs_merged_between '1.0.2-0' '1.0.2-1' '[ 12 ]' + +info "Rebasing PR #11 onto main and merging it..." +checkout_repo +setup_git_as_human +git fetch origin pr-11 +git switch pr-11 +git rebase main -Xours +git push --force origin pr-11 +merge_pr 11 +success "Rebased PR #11 and merged it to main..." + +deploy_production + +# Verify PRs for deploy comments / release +assert_prs_merged_between '1.0.1-4' '1.0.2-1' '[ 8, 10, 12 ]' + +# Verify PRs for new checklist +assert_prs_merged_between '1.0.2-1' '1.0.3-0' '[ 11 ]' + +success "Scenario #7 complete!" + + ### Cleanup title "Cleaning up..." cd "$TEST_DIR" || exit 1 From cf71429aef87fa48d1a3fb82212604f46a5c4147 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 17 Nov 2023 11:15:52 -0800 Subject: [PATCH 16/20] Simplify didVersionChange --- .../createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js | 2 +- .github/actions/javascript/createOrUpdateStagingDeploy/index.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/javascript/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js b/.github/actions/javascript/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js index db657829b571..f0e45257bbef 100644 --- a/.github/actions/javascript/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js +++ b/.github/actions/javascript/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js @@ -93,7 +93,7 @@ async function run() { }); } - const didVersionChange = newVersionTag ? newVersionTag !== currentChecklistData.tag : false; + const didVersionChange = newVersionTag !== currentChecklistData.tag; checklistBody = await GithubUtils.generateStagingDeployCashBody( newVersionTag, _.pluck(PRList, 'url'), diff --git a/.github/actions/javascript/createOrUpdateStagingDeploy/index.js b/.github/actions/javascript/createOrUpdateStagingDeploy/index.js index 86840c9ae4fa..fb1d0aa993e7 100644 --- a/.github/actions/javascript/createOrUpdateStagingDeploy/index.js +++ b/.github/actions/javascript/createOrUpdateStagingDeploy/index.js @@ -102,7 +102,7 @@ async function run() { }); } - const didVersionChange = newVersionTag ? newVersionTag !== currentChecklistData.tag : false; + const didVersionChange = newVersionTag !== currentChecklistData.tag; checklistBody = await GithubUtils.generateStagingDeployCashBody( newVersionTag, _.pluck(PRList, 'url'), From 647d0aedf04752a37717eda6878d967b3d10ed96 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 17 Nov 2023 15:20:24 -0800 Subject: [PATCH 17/20] Use the same base tag for shallow-exclude --- .../createOrUpdateStagingDeploy/index.js | 18 +++++++++--------- .../getDeployPullRequestList/index.js | 18 +++++++++--------- .github/libs/GitUtils.js | 18 +++++++++--------- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/.github/actions/javascript/createOrUpdateStagingDeploy/index.js b/.github/actions/javascript/createOrUpdateStagingDeploy/index.js index fb1d0aa993e7..bf8214759c12 100644 --- a/.github/actions/javascript/createOrUpdateStagingDeploy/index.js +++ b/.github/actions/javascript/createOrUpdateStagingDeploy/index.js @@ -190,9 +190,9 @@ const {getPreviousVersion, SEMANTIC_VERSION_LEVELS} = __nccwpck_require__(8007); /** * @param {String} tag + * @param {String} [shallowExcludeTag] when fetching the given tag, exclude all history reachable by the shallowExcludeTag (used to make fetch much faster) */ -function fetchTag(tag) { - const previousPatchVersion = getPreviousVersion(tag, SEMANTIC_VERSION_LEVELS.PATCH); +function fetchTag(tag, shallowExcludeTag = '') { let shouldRetry = true; let needsRepack = false; while (shouldRetry) { @@ -208,11 +208,9 @@ function fetchTag(tag) { 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 - // Note that this condition would only ever _not_ be true in the 1.0.0-0 edge case - if (previousPatchVersion !== tag) { - command += ` --shallow-exclude=${previousPatchVersion}`; + // Note that this condition is only ever NOT true in the 1.0.0-0 edge case + if (shallowExcludeTag && shallowExcludeTag !== tag) { + command += ` --shallow-exclude=${shallowExcludeTag}`; } console.log(`Running command: ${command}`); @@ -239,8 +237,10 @@ function fetchTag(tag) { * @returns {Promise>>} */ function getCommitHistoryAsJSON(fromTag, toTag) { - fetchTag(fromTag); - fetchTag(toTag); + // Fetch tags, exclude commits reachable from the previous patch version (i.e: previous checklist), so that we don't have to fetch the full history + const previousPatchVersion = getPreviousVersion(fromTag, SEMANTIC_VERSION_LEVELS.PATCH); + fetchTag(fromTag, previousPatchVersion); + fetchTag(toTag, previousPatchVersion); console.log('Getting pull requests merged between the following tags:', fromTag, toTag); return new Promise((resolve, reject) => { diff --git a/.github/actions/javascript/getDeployPullRequestList/index.js b/.github/actions/javascript/getDeployPullRequestList/index.js index af691cfb6d1d..974824ac4628 100644 --- a/.github/actions/javascript/getDeployPullRequestList/index.js +++ b/.github/actions/javascript/getDeployPullRequestList/index.js @@ -133,9 +133,9 @@ const {getPreviousVersion, SEMANTIC_VERSION_LEVELS} = __nccwpck_require__(8007); /** * @param {String} tag + * @param {String} [shallowExcludeTag] when fetching the given tag, exclude all history reachable by the shallowExcludeTag (used to make fetch much faster) */ -function fetchTag(tag) { - const previousPatchVersion = getPreviousVersion(tag, SEMANTIC_VERSION_LEVELS.PATCH); +function fetchTag(tag, shallowExcludeTag = '') { let shouldRetry = true; let needsRepack = false; while (shouldRetry) { @@ -151,11 +151,9 @@ function fetchTag(tag) { 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 - // Note that this condition would only ever _not_ be true in the 1.0.0-0 edge case - if (previousPatchVersion !== tag) { - command += ` --shallow-exclude=${previousPatchVersion}`; + // Note that this condition is only ever NOT true in the 1.0.0-0 edge case + if (shallowExcludeTag && shallowExcludeTag !== tag) { + command += ` --shallow-exclude=${shallowExcludeTag}`; } console.log(`Running command: ${command}`); @@ -182,8 +180,10 @@ function fetchTag(tag) { * @returns {Promise>>} */ function getCommitHistoryAsJSON(fromTag, toTag) { - fetchTag(fromTag); - fetchTag(toTag); + // Fetch tags, exclude commits reachable from the previous patch version (i.e: previous checklist), so that we don't have to fetch the full history + const previousPatchVersion = getPreviousVersion(fromTag, SEMANTIC_VERSION_LEVELS.PATCH); + fetchTag(fromTag, previousPatchVersion); + fetchTag(toTag, previousPatchVersion); console.log('Getting pull requests merged between the following tags:', fromTag, toTag); return new Promise((resolve, reject) => { diff --git a/.github/libs/GitUtils.js b/.github/libs/GitUtils.js index fa2cf430b277..2076763fbb55 100644 --- a/.github/libs/GitUtils.js +++ b/.github/libs/GitUtils.js @@ -6,9 +6,9 @@ const {getPreviousVersion, SEMANTIC_VERSION_LEVELS} = require('../libs/versionUp /** * @param {String} tag + * @param {String} [shallowExcludeTag] when fetching the given tag, exclude all history reachable by the shallowExcludeTag (used to make fetch much faster) */ -function fetchTag(tag) { - const previousPatchVersion = getPreviousVersion(tag, SEMANTIC_VERSION_LEVELS.PATCH); +function fetchTag(tag, shallowExcludeTag = '') { let shouldRetry = true; let needsRepack = false; while (shouldRetry) { @@ -24,11 +24,9 @@ function fetchTag(tag) { 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 - // Note that this condition would only ever _not_ be true in the 1.0.0-0 edge case - if (previousPatchVersion !== tag) { - command += ` --shallow-exclude=${previousPatchVersion}`; + // Note that this condition is only ever NOT true in the 1.0.0-0 edge case + if (shallowExcludeTag && shallowExcludeTag !== tag) { + command += ` --shallow-exclude=${shallowExcludeTag}`; } console.log(`Running command: ${command}`); @@ -55,8 +53,10 @@ function fetchTag(tag) { * @returns {Promise>>} */ function getCommitHistoryAsJSON(fromTag, toTag) { - fetchTag(fromTag); - fetchTag(toTag); + // Fetch tags, exclude commits reachable from the previous patch version (i.e: previous checklist), so that we don't have to fetch the full history + const previousPatchVersion = getPreviousVersion(fromTag, SEMANTIC_VERSION_LEVELS.PATCH); + fetchTag(fromTag, previousPatchVersion); + fetchTag(toTag, previousPatchVersion); console.log('Getting pull requests merged between the following tags:', fromTag, toTag); return new Promise((resolve, reject) => { From 80c05e2d2efeb8f8d25e4a1bbde2d13593478146 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 17 Nov 2023 15:23:14 -0800 Subject: [PATCH 18/20] Make OSBotify create the checklist --- .github/workflows/createDeployChecklist.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/createDeployChecklist.yml b/.github/workflows/createDeployChecklist.yml index 9a1cac41ed69..dde65f5a1503 100644 --- a/.github/workflows/createDeployChecklist.yml +++ b/.github/workflows/createDeployChecklist.yml @@ -14,7 +14,15 @@ jobs: - name: Setup Node uses: ./.github/actions/composite/setupNode + - name: Set up git for OSBotify + id: setupGitForOSBotify + uses: ./.github/actions/composite/setupGitForOSBotifyApp + with: + GPG_PASSPHRASE: ${{ secrets.LARGE_SECRET_PASSPHRASE }} + OS_BOTIFY_APP_ID: ${{ secrets.OS_BOTIFY_APP_ID }} + OS_BOTIFY_PRIVATE_KEY: ${{ secrets.OS_BOTIFY_PRIVATE_KEY }} + - name: Create or update deploy checklist uses: ./.github/actions/javascript/createOrUpdateStagingDeploy with: - GITHUB_TOKEN: ${{ github.token }} + GITHUB_TOKEN: ${{ steps.setupGitForOSBotify.outputs.OS_BOTIFY_API_TOKEN }} From 402d64fa8ba7bae285346e44ccae6fe498ba037d Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 17 Nov 2023 16:00:25 -0800 Subject: [PATCH 19/20] Fix tests --- tests/unit/createOrUpdateStagingDeployTest.js | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/tests/unit/createOrUpdateStagingDeployTest.js b/tests/unit/createOrUpdateStagingDeployTest.js index 9183268f15f0..cffb32c25589 100644 --- a/tests/unit/createOrUpdateStagingDeployTest.js +++ b/tests/unit/createOrUpdateStagingDeployTest.js @@ -3,11 +3,16 @@ */ const core = require('@actions/core'); const fns = require('date-fns'); +const {vol} = require('memfs'); +const path = require('path'); const CONST = require('../../.github/libs/CONST'); const GitUtils = require('../../.github/libs/GitUtils'); const GithubUtils = require('../../.github/libs/GithubUtils'); const run = require('../../.github/actions/javascript/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy'); +const PATH_TO_PACKAGE_JSON = path.resolve(__dirname, '../../package.json'); + +jest.mock('fs'); const mockGetInput = jest.fn(); const mockListIssues = jest.fn(); const mockGetPullRequestsMergedBetween = jest.fn(); @@ -50,6 +55,11 @@ beforeAll(() => { // Mock GitUtils GitUtils.getPullRequestsMergedBetween = mockGetPullRequestsMergedBetween; + + vol.reset(); + vol.fromJSON({ + [PATH_TO_PACKAGE_JSON]: JSON.stringify({version: '1.0.2-1'}), + }); }); afterEach(() => { @@ -139,14 +149,14 @@ describe('createOrUpdateStagingDeployCash', () => { const baseNewPullRequests = [6, 7, 8]; test('creates new issue when there is none open', async () => { + vol.reset(); + vol.fromJSON({ + [PATH_TO_PACKAGE_JSON]: JSON.stringify({version: '1.0.2-1'}), + }); mockGetInput.mockImplementation((arg) => { if (arg === 'GITHUB_TOKEN') { return 'fake_token'; } - - if (arg === 'NPM_VERSION') { - return '1.0.2-1'; - } }); mockGetPullRequestsMergedBetween.mockImplementation((fromRef, toRef) => { @@ -231,14 +241,14 @@ describe('createOrUpdateStagingDeployCash', () => { ]; test('with NPM_VERSION input, pull requests, and deploy blockers', async () => { + vol.reset(); + vol.fromJSON({ + [PATH_TO_PACKAGE_JSON]: JSON.stringify({version: '1.0.2-2'}), + }); mockGetInput.mockImplementation((arg) => { if (arg === 'GITHUB_TOKEN') { return 'fake_token'; } - - if (arg === 'NPM_VERSION') { - return '1.0.2-2'; - } }); // New pull requests to add to open StagingDeployCash @@ -309,6 +319,10 @@ describe('createOrUpdateStagingDeployCash', () => { }); test('without NPM_VERSION input, just a new deploy blocker', async () => { + vol.reset(); + vol.fromJSON({ + [PATH_TO_PACKAGE_JSON]: JSON.stringify({version: '1.0.2-1'}), + }); mockGetInput.mockImplementation((arg) => { if (arg !== 'GITHUB_TOKEN') { return; From 0c91a094bd05de1356bd2a923662ce0faa912935 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 17 Nov 2023 16:05:27 -0800 Subject: [PATCH 20/20] fix lint --- tests/unit/createOrUpdateStagingDeployTest.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/unit/createOrUpdateStagingDeployTest.js b/tests/unit/createOrUpdateStagingDeployTest.js index cffb32c25589..31b1a9346169 100644 --- a/tests/unit/createOrUpdateStagingDeployTest.js +++ b/tests/unit/createOrUpdateStagingDeployTest.js @@ -154,9 +154,10 @@ describe('createOrUpdateStagingDeployCash', () => { [PATH_TO_PACKAGE_JSON]: JSON.stringify({version: '1.0.2-1'}), }); mockGetInput.mockImplementation((arg) => { - if (arg === 'GITHUB_TOKEN') { - return 'fake_token'; + if (arg !== 'GITHUB_TOKEN') { + return; } + return 'fake_token'; }); mockGetPullRequestsMergedBetween.mockImplementation((fromRef, toRef) => { @@ -246,9 +247,10 @@ describe('createOrUpdateStagingDeployCash', () => { [PATH_TO_PACKAGE_JSON]: JSON.stringify({version: '1.0.2-2'}), }); mockGetInput.mockImplementation((arg) => { - if (arg === 'GITHUB_TOKEN') { - return 'fake_token'; + if (arg !== 'GITHUB_TOKEN') { + return; } + return 'fake_token'; }); // New pull requests to add to open StagingDeployCash