From 14e5aef38a8d5331a473250f29fb4567920b02e9 Mon Sep 17 00:00:00 2001 From: Roberto Tyley <52038+rtyley@users.noreply.github.com> Date: Fri, 1 Nov 2024 12:12:19 +0000 Subject: [PATCH] Fix loss of `run-summary.md` in GHA Job Summary Scala Steward's GitHub Action Job Summary, which details success or failure of each repo in the Scala Steward run, was added with https://github.com/scala-steward-org/scala-steward/pull/3071 in June 2023, followed up by https://github.com/scala-steward-org/scala-steward/pull/3088, which ensured that old `run-summary.md` files couldn't persist in the workspace cache (where they might become misleading), by purging them before the cache was saved. Unfortunately https://github.com/scala-steward-org/scala-steward-action/pull/631 (released with v2.67.0 in September 2024) unintentionally broke the outputting of the Job Summary, because it moved the `saveWorkspaceCache()` call (which among other things deletes the `run-summary.md` file) to a point _before_ the `run-summary.md` file was read and output to the GitHub Action Job Summary. We can see this in these two successive runs of the Guardian's Scala Steward workflow: * Run 6002 (using scala-steward-action@v2.65.0) successfully output the scala-steward summary: https://github.com/guardian/scala-steward-public-repos/actions/runs/11591772739 https://github.com/guardian/scala-steward-public-repos/actions/runs/11591772739/job/32272232470#step:7:4512 * Run 6003 (using scala-steward-action@v2.70.0) silently omitted to output the summary: https://github.com/guardian/scala-steward-public-repos/actions/runs/11592668893 https://github.com/guardian/scala-steward-public-repos/actions/runs/11592668893/job/32275051343#step:7:35 The fix is just to move the code writing the run-summary earlier, to _before_ the new `finally` block introduced by #631 which executes `saveWorkspaceCache()`. I've also renamed the method to make its actions a little clearer: * OLD: `workspace.saveWorkspaceCache()` * NEW: `workspace.purgeTempFilesAndSaveCache()` --- src/action/main.ts | 16 ++++++++-------- src/modules/workspace.test.ts | 2 +- src/modules/workspace.ts | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/action/main.ts b/src/action/main.ts index 5893c7bc..3d98bdab 100644 --- a/src/action/main.ts +++ b/src/action/main.ts @@ -76,16 +76,16 @@ async function run(): Promise { '--disable-sandbox', inputs.steward.extraArgs?.value.split(' ') ?? [], ], inputs.steward.extraJars) - } finally { - await workspace.saveWorkspaceCache() - await workspace.cancelTokenRefresh() - } - if (files.existsSync(workspace.runSummary_md)) { - logger.info(`✓ Run Summary file: ${workspace.runSummary_md}`) + if (files.existsSync(workspace.runSummary_md)) { + logger.info(`✓ Run Summary file: ${workspace.runSummary_md}`) - const summaryMarkdown = files.readFileSync(workspace.runSummary_md, 'utf8') - await core.summary.addRaw(summaryMarkdown).write() + const summaryMarkdown = files.readFileSync(workspace.runSummary_md, 'utf8') + await core.summary.addRaw(summaryMarkdown).write() + } + } finally { + await workspace.purgeTempFilesAndSaveCache() + await workspace.cancelTokenRefresh() } } catch (error: unknown) { core.setFailed(` ✕ ${(error as Error).message}`) diff --git a/src/modules/workspace.test.ts b/src/modules/workspace.test.ts index b2f072a8..b47364c5 100644 --- a/src/modules/workspace.test.ts +++ b/src/modules/workspace.test.ts @@ -187,7 +187,7 @@ test('`Workspace.restoreWorkspaceCache()` → generates different hash for diffe test('`Workspace.saveWorkspaceCache()` → saves cache', async t => { const {workspace, calls} = fixture('- owner/repo') - await workspace.saveWorkspaceCache() + await workspace.purgeTempFilesAndSaveCache() const now = Date.now() diff --git a/src/modules/workspace.ts b/src/modules/workspace.ts index 917aacd0..f79b0c62 100644 --- a/src/modules/workspace.ts +++ b/src/modules/workspace.ts @@ -74,7 +74,7 @@ export class Workspace { * * @param {string} workspace - the Scala Steward workspace directory */ - async saveWorkspaceCache(): Promise { + async purgeTempFilesAndSaveCache(): Promise { try { this.logger.startGroup('Saving workspace to cache...')