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: Allow known failing actions to gracefully continue #652

Closed

Conversation

JustinGrote
Copy link
Contributor

@JustinGrote JustinGrote commented May 2, 2021

Some actions such as cache, upload-artifact, and download artifact do not work due to lack of an emulated api/server.

A potential path forward exists with #169, but until that is implemented we should silently continue on some actions that would otherwise fail but do not really impact the flow of the action for testing, to allow other actions in the flow to be tested.

This is intended to be temporary until the feature is implemented.

Related: #329, #285

(cherry picked from commit 523ad07)

Output from a run against JustinGrote/Press:
image

(cherry picked from commit 523ad07)
@codecov
Copy link

codecov bot commented May 2, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@f56b21f). Click here to learn what that means.
The diff coverage is 80.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #652   +/-   ##
=========================================
  Coverage          ?   49.52%           
=========================================
  Files             ?       23           
  Lines             ?     2328           
  Branches          ?        0           
=========================================
  Hits              ?     1153           
  Misses            ?     1049           
  Partials          ?      126           
Impacted Files Coverage Δ
pkg/runner/step_context.go 71.01% <80.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f56b21f...b64515f. Read the comment docs.

@pull-request-size pull-request-size bot added size/M and removed size/S labels May 2, 2021
@catthehacker
Copy link
Member

catthehacker commented May 2, 2021

I don't think we should implement that. There is already a simple way to fix that through continue-on-error: or simply using if: ${{ !env.ACT }}.

@JustinGrote
Copy link
Contributor Author

@catthehacker I suppose the env path is a good workaround but it still requires modifying the action itself to accomodate act. Further, new users are going to be confused as to why these very common functions do not work as expected, this is a less hostile approach IMHO.

@catthehacker
Copy link
Member

But this will break workflows for people who have it working already + https://github.com/nektos/act#skipping-steps

@JustinGrote
Copy link
Contributor Author

@catthehacker I'm confused, how would it break them if they're already skipping the task? it would still skip it regardless as the skip check comes before the remote action check as I recall.

That said for this PR to be viable there should absolutely be a force override command line switch that I should have included.

@catthehacker
Copy link
Member

It will break for people who are using https://github.com/anthonykawa/artifact-server

@catthehacker
Copy link
Member

That said for this PR to be viable there should absolutely be a force override command line switch that I should have included.

This seems to be too much work to just workaround the problem instead of fixing it in act.

@JustinGrote
Copy link
Contributor Author

That said for this PR to be viable there should absolutely be a force override command line switch that I should have included.

This seems to be too much work to just workaround the problem instead of fixing it in act.

Fair, just wasn't sure how far off that would be since an API has to be emulated, etc.

@JustinGrote JustinGrote closed this May 2, 2021
@JustinGrote JustinGrote deleted the JustinGrote/actionsBlacklist branch May 5, 2021 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants