Javascript version of existing action which also saves if job fails#190
Javascript version of existing action which also saves if job fails#190jkrumbiegel wants to merge 22 commits intojulia-actions:mainfrom
Conversation
|
Yes but this requires more setup complexity, I'd bet most repos don't use that technique. I even don't for most of my own repos just because it's not the default. |
|
@IanButterworth, continuing the conversation from Slack. I fixed some minor problems the Claude review highlighted. I think the question if the job status is checked at the right time should be answered yes, as the tests check that the cache is written or not written on job failure given the I can't find a way to avoid the failing job markers which mark CI red overall, I think this just has to be accepted for an action like this which does some sort of "meta testing". But it won't affect the user experience at all. |
penelopeysm
left a comment
penelopeysm
left a comment
There was a problem hiding this comment.
The JavaScript all looks sensible -- although I have very little experience with Node (my JS is mostly frontend) so I just assume that all the libraries are doing what they should be doing :)
| using Pkg | ||
| Pkg.add(PackageSpec(name="DataFrames", version="1.6")) |
There was a problem hiding this comment.
Is there a reason why DataFrames here? I always had the impression it's quite a big package, so maybe feels a bit excessive for testing CI
There was a problem hiding this comment.
will check, this was all very much Claude and little me, so I might have missed some small weirdnesses like that
| - name: Force failure to test no-save-on-failure | ||
| run: exit 1 |
There was a problem hiding this comment.
Possibly change this one too, if you agree with the above.
Co-authored-by: Penelope Yong <[email protected]>
Co-authored-by: Penelope Yong <[email protected]>
Co-authored-by: Penelope Yong <[email protected]>
Co-authored-by: Penelope Yong <[email protected]>
|
Merged in #198 Thanks! |
|
Out in v3.0.0 |
4 participants
The
@actions/cacheaction does not save the cache if the job fails. This means that precompilation caches are lost if tests fail, a very common scenario when developing Julia packages. The cache action can be replaced by the cache save and cache restore steps, where the save step could in theory be called in a post step. But composite actions don't allow specifying post steps. In addition, the pytooling action which is currently used to schedule custom post actions works with command line tools, but not@actions/cache, so the default save functionality can't easily be used.I decided that because the app logic itself wasn't that complicated, a translation to a javascript app with Claude could be a good idea and this is the result of that. I kept the .jl file with cache deletion logic intact to introduce as few actual code changes as possible. The
jqdependency seems to have gone away too through the javascript use.A drawback of the current test setup is that I need failing jobs to test these options, however failing jobs also make the CI summary look like it failed. There's no github actions option to allow a job failure in the sense of a green tick mark. So I just prefixed the names with "EXPECTED TO FAIL" hoping that it's still reasonably clear what's going on. In that sense, this problem is more of a cosmetic nature.
I set the new default to
save-always: trueas I think everyone would benefit from that. Due to the architecture and defaults change, a major version bump would probably be appropriate.