From ec01b1f29fee136f03395eab0655b0f8f191dd07 Mon Sep 17 00:00:00 2001 From: matt korwel Date: Thu, 18 Sep 2025 19:00:18 -0700 Subject: [PATCH] better messaging (#8794) --- .../workflows/release-patch-1-create-pr.yml | 13 +- .../workflows/release-patch-from-comment.yml | 79 ++++- scripts/releasing/patch-create-comment.js | 30 +- scripts/tests/patch-create-comment.test.js | 280 ++++++++++++++++++ 4 files changed, 375 insertions(+), 27 deletions(-) create mode 100644 scripts/tests/patch-create-comment.test.js diff --git a/.github/workflows/release-patch-1-create-pr.yml b/.github/workflows/release-patch-1-create-pr.yml index e4461fd1e8..5e751dd8d0 100644 --- a/.github/workflows/release-patch-1-create-pr.yml +++ b/.github/workflows/release-patch-1-create-pr.yml @@ -71,9 +71,15 @@ jobs: GH_TOKEN: '${{ steps.generate_token.outputs.token }}' continue-on-error: true run: | - node scripts/releasing/create-patch-pr.js --commit=${{ github.event.inputs.commit }} --channel=${{ github.event.inputs.channel }} --dry-run=${{ github.event.inputs.dry_run }} > patch_output.log 2>&1 - echo "EXIT_CODE=$?" >> "$GITHUB_OUTPUT" - cat patch_output.log + # Capture output directly to environment variable + { + node scripts/releasing/create-patch-pr.js --commit=${{ github.event.inputs.commit }} --channel=${{ github.event.inputs.channel }} --dry-run=${{ github.event.inputs.dry_run }} + echo "EXIT_CODE=$?" >> "$GITHUB_OUTPUT" + } 2>&1 | { + echo "LOG_CONTENT<> "$GITHUB_ENV" + cat >> "$GITHUB_ENV" + echo "EOF" >> "$GITHUB_ENV" + } - name: 'Comment on Original PR' if: '!inputs.dry_run && inputs.original_pr' @@ -81,7 +87,6 @@ jobs: GH_TOKEN: '${{ steps.generate_token.outputs.token }}' ORIGINAL_PR: '${{ github.event.inputs.original_pr }}' EXIT_CODE: '${{ steps.create_patch.outputs.EXIT_CODE }}' - OUTPUT_LOG: 'patch_output.log' COMMIT: '${{ github.event.inputs.commit }}' CHANNEL: '${{ github.event.inputs.channel }}' REPOSITORY: '${{ github.repository }}' diff --git a/.github/workflows/release-patch-from-comment.yml b/.github/workflows/release-patch-from-comment.yml index 2ba2eea5e1..351d4f5313 100644 --- a/.github/workflows/release-patch-from-comment.yml +++ b/.github/workflows/release-patch-from-comment.yml @@ -52,6 +52,7 @@ jobs: - name: 'Dispatch if Merged' if: "steps.pr_status.outputs.STATE == 'MERGED'" + id: 'dispatch_patch' uses: 'actions/github-script@00f12e3e20659f42342b1c0226afda7f7c042325' env: COMMENT_BODY: '${{ github.event.comment.body }}' @@ -82,7 +83,7 @@ jobs: console.log('Detected channel:', channel); - github.rest.actions.createWorkflowDispatch({ + const response = await github.rest.actions.createWorkflowDispatch({ owner: context.repo.owner, repo: context.repo.repo, workflow_id: 'release-patch-1-create-pr.yml', @@ -93,7 +94,30 @@ jobs: dry_run: 'false', original_pr: '${{ github.event.issue.number }}' } - }) + }); + + // Wait a moment for the workflow to be created, then find it + await new Promise(resolve => setTimeout(resolve, 2000)); + + const runs = await github.rest.actions.listWorkflowRuns({ + owner: context.repo.owner, + repo: context.repo.repo, + workflow_id: 'release-patch-1-create-pr.yml', + per_page: 10 + }); + + // Find the most recent run that matches our trigger + const dispatchedRun = runs.data.workflow_runs.find(run => + run.event === 'workflow_dispatch' && + new Date(run.created_at) > new Date(Date.now() - 10000) // Within last 10 seconds + ); + + if (dispatchedRun) { + core.setOutput('dispatched_run_id', dispatchedRun.id); + core.setOutput('dispatched_run_url', dispatchedRun.html_url); + } + + core.setOutput('channel', channel); - name: 'Comment on Failure' if: "startsWith(github.event.comment.body, '/patch') && steps.pr_status.outputs.STATE != 'MERGED'" @@ -102,3 +126,54 @@ jobs: issue-number: '${{ github.event.issue.number }}' body: | :x: The `/patch` command failed. This pull request must be merged before a patch can be created. + + - name: 'Final Status Comment - Success' + if: "always() && startsWith(github.event.comment.body, '/patch') && steps.dispatch_patch.outcome == 'success' && steps.dispatch_patch.outputs.dispatched_run_url" + uses: 'peter-evans/create-or-update-comment@67dcc547d311b736a8e6c5c236542148a47adc3d' + with: + issue-number: '${{ github.event.issue.number }}' + body: | + โœ… **Patch workflow dispatched successfully!** + + **๐Ÿ“‹ Details:** + - **Channel**: `${{ steps.dispatch_patch.outputs.channel }}` + - **Commit**: `${{ steps.pr_status.outputs.MERGE_COMMIT_SHA }}` + + **๐Ÿ”— Track Progress:** + - [Dispatched patch workflow](${{ steps.dispatch_patch.outputs.dispatched_run_url }}) + - [This workflow run](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}) + + - name: 'Final Status Comment - Dispatch Success (No URL)' + if: "always() && startsWith(github.event.comment.body, '/patch') && steps.dispatch_patch.outcome == 'success' && !steps.dispatch_patch.outputs.dispatched_run_url" + uses: 'peter-evans/create-or-update-comment@67dcc547d311b736a8e6c5c236542148a47adc3d' + with: + issue-number: '${{ github.event.issue.number }}' + body: | + โœ… **Patch workflow dispatched successfully!** + + **๐Ÿ“‹ Details:** + - **Channel**: `${{ steps.dispatch_patch.outputs.channel }}` + - **Commit**: `${{ steps.pr_status.outputs.MERGE_COMMIT_SHA }}` + + **๐Ÿ”— Track Progress:** + - [View patch workflows](https://github.com/${{ github.repository }}/actions/workflows/release-patch-1-create-pr.yml) + - [This workflow run](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}) + + - name: 'Final Status Comment - Failure' + if: "always() && startsWith(github.event.comment.body, '/patch') && (steps.dispatch_patch.outcome == 'failure' || steps.dispatch_patch.outcome == 'cancelled')" + uses: 'peter-evans/create-or-update-comment@67dcc547d311b736a8e6c5c236542148a47adc3d' + with: + issue-number: '${{ github.event.issue.number }}' + body: | + โŒ **Patch workflow dispatch failed!** + + There was an error dispatching the patch creation workflow. + + **๐Ÿ” Troubleshooting:** + - Check that the PR is properly merged + - Verify workflow permissions + - Review error logs in the workflow run + + **๐Ÿ”— Debug Links:** + - [This workflow run](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}) + - [Patch workflow history](https://github.com/${{ github.repository }}/actions/workflows/release-patch-1-create-pr.yml) diff --git a/scripts/releasing/patch-create-comment.js b/scripts/releasing/patch-create-comment.js index a0d6751197..132dec5c81 100644 --- a/scripts/releasing/patch-create-comment.js +++ b/scripts/releasing/patch-create-comment.js @@ -13,7 +13,6 @@ import yargs from 'yargs'; import { hideBin } from 'yargs/helpers'; -import { readFileSync } from 'node:fs'; async function main() { const argv = await yargs(hideBin(process.argv)) @@ -27,11 +26,6 @@ async function main() { type: 'number', demandOption: !process.env.GITHUB_ACTIONS, }) - .option('output-log', { - description: 'Path to the patch output log file', - type: 'string', - default: 'patch_output.log', - }) .option('commit', { description: 'The commit SHA being patched', type: 'string', @@ -86,8 +80,6 @@ async function main() { argv.exitCode !== undefined ? argv.exitCode : parseInt(process.env.EXIT_CODE || '1'); - const outputLog = - argv.outputLog || process.env.OUTPUT_LOG || 'patch_output.log'; const commit = argv.commit || process.env.COMMIT; const channel = argv.channel || process.env.CHANNEL; const repository = @@ -111,7 +103,6 @@ async function main() { console.log('\n๐Ÿ“‹ Inputs:'); console.log(` - Original PR: ${originalPr}`); console.log(` - Exit Code: ${exitCode}`); - console.log(` - Output Log: ${outputLog}`); console.log(` - Commit: ${commit}`); console.log(` - Channel: ${channel} โ†’ npm tag: ${npmTag}`); console.log(` - Repository: ${repository}`); @@ -121,20 +112,17 @@ async function main() { let commentBody; let logContent = ''; - // Try to read the output log - try { - if (testMode) { - // Create mock log content for testing - if (exitCode === 0) { - logContent = `Creating hotfix branch hotfix/v0.5.3/${channel}/cherry-pick-${commit.substring(0, 7)} from release/v0.5.3`; - } else { - logContent = 'Error: Failed to create patch'; - } + // Get log content from environment variable or generate mock content for testing + if (testMode && !process.env.LOG_CONTENT) { + // Create mock log content for testing only if LOG_CONTENT is not provided + if (exitCode === 0) { + logContent = `Creating hotfix branch hotfix/v0.5.3/${channel}/cherry-pick-${commit.substring(0, 7)} from release/v0.5.3`; } else { - logContent = readFileSync(outputLog, 'utf8'); + logContent = 'Error: Failed to create patch'; } - } catch (error) { - console.log(`Could not read output log ${outputLog}:`, error.message); + } else { + // Use log content from environment variable + logContent = process.env.LOG_CONTENT || ''; } if (logContent.includes('already has an open PR')) { diff --git a/scripts/tests/patch-create-comment.test.js b/scripts/tests/patch-create-comment.test.js new file mode 100644 index 0000000000..803b6bec0e --- /dev/null +++ b/scripts/tests/patch-create-comment.test.js @@ -0,0 +1,280 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { execSync } from 'node:child_process'; +import { join } from 'node:path'; + +/** + * Helper function to run the patch-create-comment script with given parameters + */ +function runPatchCreateComment(args, env = {}) { + const scriptPath = join( + process.cwd(), + 'scripts/releasing/patch-create-comment.js', + ); + const fullEnv = { + ...process.env, + TEST_MODE: 'true', // Always run in test mode to avoid GitHub API calls + ...env, + }; + + try { + const result = execSync(`node ${scriptPath} ${args}`, { + encoding: 'utf8', + env: fullEnv, + }); + return { stdout: result, stderr: '', success: true }; + } catch (error) { + return { + stdout: error.stdout || '', + stderr: error.stderr || '', + success: false, + code: error.status, + }; + } +} + +describe('patch-create-comment', () => { + let originalEnv; + + beforeEach(() => { + // Save original environment + originalEnv = { ...process.env }; + }); + + afterEach(() => { + // Restore original environment + process.env = originalEnv; + vi.clearAllMocks(); + }); + describe('Environment Variable vs File Reading', () => { + it('should prefer LOG_CONTENT environment variable over file', () => { + const result = runPatchCreateComment( + '--original-pr 8655 --exit-code 0 --commit abc1234 --channel preview --repository google-gemini/gemini-cli --test', + { + LOG_CONTENT: + 'Creating hotfix branch hotfix/v0.5.3/preview/cherry-pick-abc1234 from release/v0.5.3', + }, + ); + + expect(result.success).toBe(true); + expect(result.stdout).toContain('๐Ÿš€ **Patch PR Created!**'); + expect(result.stdout).toContain('Channel**: `preview`'); + expect(result.stdout).toContain('Commit**: `abc1234`'); + }); + + it('should use empty log content when LOG_CONTENT is not set', () => { + const result = runPatchCreateComment( + '--original-pr 8655 --exit-code 1 --commit abc1234 --channel stable --repository google-gemini/gemini-cli --test', + {}, // No LOG_CONTENT env var + ); + + expect(result.success).toBe(true); + expect(result.stdout).toContain('โŒ **Patch creation failed!**'); + expect(result.stdout).toContain( + 'There was an error creating the patch release', + ); + }); + }); + + describe('Log Content Parsing - Success Scenarios', () => { + it('should generate success comment for clean cherry-pick', () => { + const result = runPatchCreateComment( + '--original-pr 8655 --exit-code 0 --commit abc1234 --channel stable --repository google-gemini/gemini-cli --test', + { + LOG_CONTENT: + 'Creating hotfix branch hotfix/v0.4.1/stable/cherry-pick-abc1234 from release/v0.4.1\nโœ… Cherry-pick successful - no conflicts detected', + }, + ); + + expect(result.success).toBe(true); + expect(result.stdout).toContain('๐Ÿš€ **Patch PR Created!**'); + expect(result.stdout).toContain('Channel**: `stable`'); + expect(result.stdout).toContain('Commit**: `abc1234`'); + expect(result.stdout).not.toContain('โš ๏ธ Status'); + }); + + it('should generate conflict comment for cherry-pick with conflicts', () => { + const result = runPatchCreateComment( + '--original-pr 8655 --exit-code 0 --commit def5678 --channel preview --repository google-gemini/gemini-cli --test', + { + LOG_CONTENT: + 'Creating hotfix branch hotfix/v0.5.0-preview.2/preview/cherry-pick-def5678 from release/v0.5.0-preview.2\nCherry-pick has conflicts in 2 file(s):\nCONFLICT (content): Merge conflict in package.json', + }, + ); + + expect(result.success).toBe(true); + expect(result.stdout).toContain('๐Ÿš€ **Patch PR Created!**'); + expect(result.stdout).toContain( + 'โš ๏ธ Status**: Cherry-pick conflicts detected', + ); + expect(result.stdout).toContain( + 'โš ๏ธ **Resolve conflicts** in the hotfix PR first', + ); + expect(result.stdout).toContain('Channel**: `preview`'); + }); + }); + + describe('Log Content Parsing - Existing PR Scenarios', () => { + it('should detect existing PR and generate appropriate comment', () => { + const result = runPatchCreateComment( + '--original-pr 8655 --exit-code 0 --commit ghi9012 --channel stable --repository google-gemini/gemini-cli --test', + { + LOG_CONTENT: + 'Hotfix branch hotfix/v0.4.1/stable/cherry-pick-ghi9012 already has an open PR.\nFound existing PR #8700: https://github.com/google-gemini/gemini-cli/pull/8700', + }, + ); + + expect(result.success).toBe(true); + expect(result.stdout).toContain('โ„น๏ธ **Patch PR already exists!**'); + expect(result.stdout).toContain( + 'A patch PR for this change already exists: [#8700](https://github.com/google-gemini/gemini-cli/pull/8700)', + ); + expect(result.stdout).toContain( + 'Review and approve the existing patch PR', + ); + }); + + it('should detect branch exists but no PR scenario', () => { + const result = runPatchCreateComment( + '--original-pr 8655 --exit-code 0 --commit jkl3456 --channel preview --repository google-gemini/gemini-cli --test', + { + LOG_CONTENT: + 'Hotfix branch hotfix/v0.5.0-preview.2/preview/cherry-pick-jkl3456 exists but has no open PR.\nHotfix branch hotfix/v0.5.0-preview.2/preview/cherry-pick-jkl3456 already exists.', + }, + ); + + expect(result.success).toBe(true); + expect(result.stdout).toContain( + 'โ„น๏ธ **Patch branch exists but no PR found!**', + ); + expect(result.stdout).toContain( + 'Delete the branch: `git branch -D hotfix/v0.5.0-preview.2/preview/cherry-pick-jkl3456`', + ); + expect(result.stdout).toContain('Run the patch command again'); + }); + }); + + describe('Log Content Parsing - Failure Scenarios', () => { + it('should generate failure comment when exit code is non-zero', () => { + const result = runPatchCreateComment( + '--original-pr 8655 --exit-code 1 --commit mno7890 --channel stable --repository google-gemini/gemini-cli --run-id 12345 --test', + { + LOG_CONTENT: 'Error: Failed to create patch', + }, + ); + + expect(result.success).toBe(true); + expect(result.stdout).toContain('โŒ **Patch creation failed!**'); + expect(result.stdout).toContain( + 'There was an error creating the patch release', + ); + expect(result.stdout).toContain( + 'View workflow run](https://github.com/google-gemini/gemini-cli/actions/runs/12345)', + ); + }); + + it('should generate fallback failure comment when no output is generated', () => { + const result = runPatchCreateComment( + '--original-pr 8655 --exit-code 1 --commit pqr4567 --channel preview --repository google-gemini/gemini-cli --run-id 67890 --test', + { + LOG_CONTENT: '', + }, + ); + + expect(result.success).toBe(true); + expect(result.stdout).toContain('โŒ **Patch creation failed!**'); + expect(result.stdout).toContain( + 'There was an error creating the patch release', + ); + }); + }); + + describe('Channel and NPM Tag Detection', () => { + it('should correctly map stable channel to latest npm tag', () => { + const result = runPatchCreateComment( + '--original-pr 8655 --exit-code 0 --commit stu8901 --channel stable --repository google-gemini/gemini-cli --test', + { + LOG_CONTENT: + 'Creating hotfix branch hotfix/v0.4.1/stable/cherry-pick-stu8901 from release/v0.4.1', + }, + ); + + expect(result.success).toBe(true); + expect(result.stdout).toContain('will publish to npm tag `latest`'); + }); + + it('should correctly map preview channel to preview npm tag', () => { + const result = runPatchCreateComment( + '--original-pr 8655 --exit-code 0 --commit vwx2345 --channel preview --repository google-gemini/gemini-cli --test', + { + LOG_CONTENT: + 'Creating hotfix branch hotfix/v0.5.0-preview.2/preview/cherry-pick-vwx2345 from release/v0.5.0-preview.2', + }, + ); + + expect(result.success).toBe(true); + expect(result.stdout).toContain('will publish to npm tag `preview`'); + }); + }); + + describe('No Original PR Scenario', () => { + it('should skip comment when no original PR is specified', () => { + const result = runPatchCreateComment( + '--original-pr 0 --exit-code 0 --commit yza6789 --channel stable --repository google-gemini/gemini-cli --test', + { + LOG_CONTENT: + 'Creating hotfix branch hotfix/v0.4.1/stable/cherry-pick-yza6789 from release/v0.4.1', + ORIGINAL_PR: '', // Override with empty PR + }, + ); + + expect(result.success).toBe(true); + expect(result.stdout).toContain( + 'No original PR specified, skipping comment', + ); + }); + }); + + describe('Error Handling', () => { + it('should handle empty LOG_CONTENT gracefully', () => { + const result = runPatchCreateComment( + '--original-pr 8655 --exit-code 1 --commit bcd0123 --channel stable --repository google-gemini/gemini-cli --test', + { LOG_CONTENT: '' }, // Empty log content + ); + + expect(result.success).toBe(true); + expect(result.stdout).toContain('โŒ **Patch creation failed!**'); + expect(result.stdout).toContain( + 'There was an error creating the patch release', + ); + }); + }); + + describe('Test Mode Flag', () => { + it('should generate mock content in test mode for success', () => { + const result = runPatchCreateComment( + '--original-pr 8655 --exit-code 0 --commit efg4567 --channel preview --repository google-gemini/gemini-cli --test', + ); + + expect(result.success).toBe(true); + expect(result.stdout).toContain( + '๐Ÿงช TEST MODE - No API calls will be made', + ); + expect(result.stdout).toContain('๐Ÿš€ **Patch PR Created!**'); + }); + + it('should generate mock content in test mode for failure', () => { + const result = runPatchCreateComment( + '--original-pr 8655 --exit-code 1 --commit hij8901 --channel stable --repository google-gemini/gemini-cli --test', + ); + + expect(result.success).toBe(true); + expect(result.stdout).toContain('โŒ **Patch creation failed!**'); + }); + }); +});