Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix loss of run-summary.md in GHA Job Summary #652

Merged

Conversation

rtyley
Copy link
Contributor

@rtyley rtyley commented Nov 1, 2024

Scala Steward's GitHub Action Job Summary, which reports success or failure of individual repos in the Scala Steward run, was added with scala-steward-org/scala-steward#3071 in June 2023, followed up by scala-steward-org/scala-steward#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 #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:

Changes

The fix is just to move the code processing 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:

workspace.saveWorkspaceCache()

...becomes...

workspace.purgeTempFilesAndSaveCache()

Tests

Without refactoring, testing that the core.summary.addRaw() method is successfully called with a summary would probably require some kind of large test against run() in main.ts - there are none of those already, and I was too nervous to attempt one!

I have run the scala-steward-org/scala-steward-action@snapshots/652 snapshot of this PR with guardian/scala-steward-public-repos#78, and confirmed that the report is properly restored to the Job Summary:

image

Scala Steward's GitHub Action Job Summary, which details success or failure
of each repo in the Scala Steward run, was added with
scala-steward-org/scala-steward#3071 in June 2023,
followed up by scala-steward-org/scala-steward#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 scala-steward-org#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 [email protected]) 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 [email protected]) 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 scala-steward-org#631 which executes `saveWorkspaceCache()`.

I've also renamed the method to make its actions a little clearer:

* OLD: `workspace.saveWorkspaceCache()`
* NEW: `workspace.purgeTempFilesAndSaveCache()`
Copy link
Contributor

github-actions bot commented Nov 1, 2024

Code Coverage

Package Line Rate Branch Rate Complexity Health
core 100% 100% 0
modules 69% 88% 0
Summary 70% (495 / 703) 89% (74 / 83) 0

Copy link
Contributor

github-actions bot commented Nov 1, 2024

A snapshot release has been created as snapshots/652.

You can test it out with:

uses: scala-steward-org/scala-steward-action@snapshots/652

It will be automatically recreated on any change to this PR.

github-actions bot added a commit that referenced this pull request Nov 1, 2024
rtyley added a commit to guardian/scala-steward-public-repos that referenced this pull request Nov 1, 2024
@rtyley rtyley marked this pull request as ready for review November 1, 2024 14:45
Copy link
Member

@mzuehlke mzuehlke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Copy link
Member

@alejandrohdezma alejandrohdezma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rtyley! That's a great catch!

@alejandrohdezma alejandrohdezma merged commit 7aeabbe into scala-steward-org:master Nov 4, 2024
2 checks passed
@rtyley
Copy link
Contributor Author

rtyley commented Nov 4, 2024

Thanks @rtyley! That's a great catch!

Thanks @alejandrohdezma ! I see this has gone out in v2.71.0 - thank you!

rtyley added a commit to guardian/scala-steward-public-repos that referenced this pull request Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants