Auto-file staging retest request for cherry-picked blocker fixes#96103
Auto-file staging retest request for cherry-picked blocker fixes#96103jasperhuangg wants to merge 5 commits into
Conversation
melvin-bot
Bot
requested review from
JmillsExpensify
and removed request for
a team
jasperhuangg
changed the title
melvin-bot
Bot
requested review from
neil-marcellini
and removed request for
a team
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
jasperhuangg
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9a96ce8c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jasperhuangg
changed the title
| } | ||
|
|
||
| async function run(): Promise<void> { | ||
| const deploySHA = process.env.DEPLOY_SHA; |
There was a problem hiding this comment.
Can we use the CLI lib from expensify-common to pass these in as CLI args rather than through the env?
| /** Map a hit to the flat string payload the Slack workflow webhook expects. */ | ||
| function buildRetestPayload(hit: RetestHit): Record<string, string> { | ||
| return { | ||
| isDb: 'dbTrue', |
There was a problem hiding this comment.
Do we have control over this payload shape? Ideally we wouldn't abbreviate this, and it's unclear to me why we're using dbTrue as a string instead of isDeployBlocker: true (as a boolean)
|
|
||
| const hits: RetestHit[] = []; | ||
| for (const prNumber of candidatePRNumbers) { | ||
| // Gate 2: the cherry-picked PR must be on the current checklist. |
There was a problem hiding this comment.
I must've missed Gate 1 😂
| continue; | ||
| } | ||
|
|
||
| if (await alreadyFiled(prNumber, deployTag)) { |
There was a problem hiding this comment.
| if (await alreadyFiled(prNumber, deployTag)) { | |
| if (await isRetestAlreadyRequested(prNumber, deployTag)) { |
| export-env: false | ||
| env: | ||
| OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }} | ||
| SLACK_RETEST_WEBHOOK: op://${{ vars.OP_VAULT }}/Repository-Secrets/SLACK_RETEST_WEBHOOK |
There was a problem hiding this comment.
I don't see this vault in 1Password. Should it be in the Mobile-Deploy-CI vault?
2 participants
Explanation of Change
When a deploy-blocker fix is cherry-picked to staging, the deployer currently files a Slack
#retestsrequest by hand. This does it automatically: a new deploy job fires on a staging cherry-pick (once all platforms land) when the cherry-picked PR is on the currentStagingDeployCashchecklist and links a deploy-blocker issue on it, then POSTs the retest request to the Slack webhook.The webhook URL is loaded from 1Password (
Repository-Secrets/SLACK_RETEST_WEBHOOKin theMobile-Deploy-CIvault), not the code.How it works
StagingDeployCashchecklist and links a deploy-blocker issue on it.#retestsworkflow.Fixed Issues
$
PROPOSAL:
Tests
This is a deploy-workflow change with no app-facing UI. It only runs inside the staging deploy for a cherry-pick, so it can't be exercised in the local dev environment. Logic is covered by unit tests:
npx jest tests/unit/createRetestRequestForCPTest.ts.Offline tests
N/A, this runs in CI during a deploy, there is no offline client behavior.
QA Steps
N/A. This only runs in the staging deploy workflow when a deploy-blocker fix is cherry-picked to staging, so it can't be triggered on demand by QA. Title includes
[No QA].PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
N/A, this is a CI/deploy workflow change with no app UI.
Android: Native
N/A
Android: mWeb Chrome
N/A
iOS: Native
N/A
iOS: mWeb Safari
N/A
MacOS: Chrome / Safari
N/A