fix: composite action input pollution #2348
Conversation
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports MegaLinter is graciously provided by |
|
My outstanding task here is writing tests, please remind me if I forget that. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2348 +/- ##
===========================================
+ Coverage 61.56% 76.31% +14.75%
===========================================
Files 53 61 +8
Lines 9002 7811 -1191
===========================================
+ Hits 5542 5961 +419
+ Misses 3020 1295 -1725
- Partials 440 555 +115 ☔ View full report in Codecov by Sentry. |
| @@ -625,6 +625,7 @@ func runPostStep(step actionStep) common.Executor { | |||
| case model.ActionRunsUsingNode12, model.ActionRunsUsingNode16, model.ActionRunsUsingNode20: | |||
|
|
|||
| populateEnvsFromSavedState(step.getEnv(), step, rc) | |||
| populateEnvsFromInput(ctx, step.getEnv(), step.getActionModel(), rc) | |||
There was a problem hiding this comment.
cleaning up env caused missing default values in post steps
| @@ -242,6 +242,8 @@ func TestRunEvent(t *testing.T) { | |||
| // Uses | |||
| {workdir, "uses-composite", "push", "", platforms, secrets}, | |||
| {workdir, "uses-composite-with-error", "push", "Job 'failing-composite-action' failed", platforms, secrets}, | |||
| {workdir, "uses-composite-check-for-input-collision", "push", "", platforms, secrets}, | |||
There was a problem hiding this comment.
This test only passes via changes made here
| @@ -139,6 +139,7 @@ func TestSetupEnv(t *testing.T) { | |||
| JobContainer: cm, | |||
| } | |||
| step := &model.Step{ | |||
| Uses: "./", | |||
There was a problem hiding this comment.
See if step.getStepModel().Uses != "" { check above, empty with doesn't mean it's a run step
There was a problem hiding this comment.
@ChristopherHX - I appreciate the comments on the PR. Makes it easier to understand intentions!
|
Errata of this PR is #2426 |
The full shape of this bug not yet discovered, it's big and needs testsandact refuses to use the default for an nodejs input when an composite action has an input with the same nameFixes #2243