also looking at this
fix(gates): actually refresh the dashboard instead of reporting that it did
#1425
3 files
+86
-3
| @@ -3148,6 +3148,11 @@ cli | ||
| 3148 | 3148 | const outcome = await runPostMerge(provider, pr, config.gates?.postMerge ?? {}, { |
| 3149 | 3149 | baseBranch: config.repository?.baseBranch ?? pr.base, |
| 3150 | 3150 | logger, |
| 3151 | // Supplied here because refreshing means a full scan, which is | |
| 3152 | // `Buddy`'s job rather than the gate module's. | |
| 3153 | refreshDashboard: async () => { | |
| 3154 | await new Buddy(config).createOrUpdateDashboard() | |
| 3155 | }, | |
| 3151 | 3156 | }) |
| 3152 | 3157 | |
| 3153 | 3158 | for (const action of outcome.performed) |
| @@ -82,7 +82,19 @@ export async function runPostMerge( | ||
| 82 | 82 | provider: GitProvider, |
| 83 | 83 | pr: PullRequest, |
| 84 | 84 | config: PostMergeConfig = {}, |
| 85 | options: { baseBranch?: string, logger?: Logger } = {}, | |
| 85 | options: { | |
| 86 | baseBranch?: string | |
| 87 | logger?: Logger | |
| 88 | /** | |
| 89 | * Refresh the dependency dashboard. | |
| 90 | * | |
| 91 | * A hook rather than something this module does, because refreshing means | |
| 92 | * a full scan and this file only knows about a provider. Absent means the | |
| 93 | * caller cannot refresh, which is reported as a skip rather than passed | |
| 94 | * off as done. | |
| 95 | */ | |
| 96 | refreshDashboard?: () => Promise<void> | |
| 97 | } = {}, | |
| 86 | 98 | ): Promise<PostMergeOutcome> { |
| 87 | 99 | const logger = options.logger ?? getDefaultLogger() |
| 88 | 100 | const baseBranch = options.baseBranch ?? pr.base ?? 'main' |
| @@ -138,8 +150,24 @@ export async function runPostMerge( | ||
| 138 | 150 | } |
| 139 | 151 | } |
| 140 | 152 | |
| 141 | if (config.refreshDashboard) | |
| 142 | performed.push('refresh-dashboard') | |
| 153 | if (config.refreshDashboard) { | |
| 154 | // This branch used to push `refresh-dashboard` onto `performed` and do | |
| 155 | // nothing at all — the one action here that reported success without | |
| 156 | // having any work behind it. | |
| 157 | if (!options.refreshDashboard) { | |
| 158 | skipped.push({ action: 'refresh-dashboard', reason: 'no dashboard refresher was supplied' }) | |
| 159 | } | |
| 160 | else { | |
| 161 | try { | |
| 162 | await options.refreshDashboard() | |
| 163 | performed.push('refresh-dashboard') | |
| 164 | } | |
| 165 | catch (error) { | |
| 166 | logger.warn(`⚠️ Could not refresh the dashboard: ${error}`) | |
| 167 | skipped.push({ action: 'refresh-dashboard', reason: 'refresh failed' }) | |
| 168 | } | |
| 169 | } | |
| 170 | } | |
| 143 | 171 | |
| 144 | 172 | return { performed, skipped } |
| 145 | 173 | } |
| @@ -303,6 +303,56 @@ describe('post-merge actions', () => { | ||
| 303 | 303 | draft: false, |
| 304 | 304 | } |
| 305 | 305 | |
| 306 | it('failure case - refreshing the dashboard is not reported without a refresher', async () => { | |
| 307 | // This branch used to push `refresh-dashboard` onto `performed` and do | |
| 308 | // nothing — the one action here that claimed success with no work behind | |
| 309 | // it, which is worse than not offering the action at all. | |
| 310 | const outcome = await runPostMerge(fakeProvider(), merged, { refreshDashboard: true }) | |
| 311 | ||
| 312 | expect(outcome.performed).not.toContain('refresh-dashboard') | |
| 313 | expect(outcome.skipped).toContainEqual({ | |
| 314 | action: 'refresh-dashboard', | |
| 315 | reason: 'no dashboard refresher was supplied', | |
| 316 | }) | |
| 317 | }) | |
| 318 | ||
| 319 | it('success case - refreshes the dashboard when a refresher is supplied', async () => { | |
| 320 | let refreshed = false | |
| 321 | const outcome = await runPostMerge(fakeProvider(), merged, { refreshDashboard: true }, { | |
| 322 | refreshDashboard: async () => { | |
| 323 | refreshed = true | |
| 324 | }, | |
| 325 | }) | |
| 326 | ||
| 327 | expect(refreshed).toBe(true) | |
| 328 | expect(outcome.performed).toContain('refresh-dashboard') | |
| 329 | }) | |
| 330 | ||
| 331 | it('failure case - a refresh that throws is reported, not claimed', async () => { | |
| 332 | // The pull request has already merged, so a failed refresh must not read | |
| 333 | // as a failed run — but it must not read as a success either. | |
| 334 | const outcome = await runPostMerge(fakeProvider(), merged, { refreshDashboard: true }, { | |
| 335 | logger: Logger.silent(), | |
| 336 | refreshDashboard: async () => { | |
| 337 | throw new Error('rate limited') | |
| 338 | }, | |
| 339 | }) | |
| 340 | ||
| 341 | expect(outcome.performed).not.toContain('refresh-dashboard') | |
| 342 | expect(outcome.skipped).toContainEqual({ action: 'refresh-dashboard', reason: 'refresh failed' }) | |
| 343 | }) | |
| 344 | ||
| 345 | it('edge case - a refresher is not called when the action is off', async () => { | |
| 346 | let refreshed = false | |
| 347 | await runPostMerge(fakeProvider(), merged, {}, { | |
| 348 | refreshDashboard: async () => { | |
| 349 | refreshed = true | |
| 350 | }, | |
| 351 | }) | |
| 352 | ||
| 353 | expect(refreshed).toBe(false) | |
| 354 | }) | |
| 355 | ||
| 306 | 356 | it('success case - appends to the changelog', async () => { |
| 307 | 357 | const provider = fakeProvider() |
| 308 | 358 | const outcome = await runPostMerge(provider, merged, { changelog: { enabled: true } }) |