Allow passed in env vars to supersede ones declared in the workflow

[liamphmurphy]

Simple approach to allowing for #1098

Since

act/pkg/runner/run_context.go

Line 78 in b3bd268

rc.Env = mergeMaps(rc.Config.Env, rc.Run.Workflow.Env, rc.Run.Job().Environment())

specifies that the rc.Config.env is the first map to iterate through (which is what contains any variables passed into act from the CLI), this simple duplicate check works for not letting the values of an env var in the workflow supersede what is passed into the CLI or env-files.

Added a few tests as well, please let me know if anything should be adjusted.

[codecov]

Codecov Report

Merging #1100 (af7a936) into master (4f8da0a) will increase coverage by 2.50%.
The diff coverage is 79.24%.

@@            Coverage Diff             @@
##           master    #1100      +/-   ##
==========================================
+ Coverage   57.50%   60.01%   +2.50%     
==========================================
  Files          32       39       +7     
  Lines        4594     4924     +330     
==========================================
+ Hits         2642     2955     +313     
+ Misses       1729     1727       -2     
- Partials      223      242      +19     

Impacted Files	Coverage Δ
pkg/model/action.go	0.00% <ø> (ø)
pkg/model/planner.go	50.73% <ø> (+0.32%)	⬆️
pkg/model/workflow.go	50.91% <ø> (ø)
pkg/container/docker_run.go	5.11% <2.50%> (-0.43%)	⬇️
pkg/container/file_collector.go	44.85% <44.85%> (ø)
pkg/runner/logger.go	66.00% <67.56%> (+0.56%)	⬆️
pkg/runner/runner.go	73.72% <73.68%> (-2.75%)	⬇️
pkg/exprparser/interpreter.go	74.57% <75.00%> (+1.17%)	⬆️
pkg/runner/expression.go	91.02% <78.26%> (+0.20%)	⬆️
pkg/runner/step.go	80.76% <80.76%> (ø)
... and 18 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[KnisterPeter]

I think changing the order of parameters from

rc.Env = mergeMaps(rc.Config.Env, rc.Run.Workflow.Env, rc.Run.Job().Environment())

to

rc.Env = mergeMaps(rc.Run.Workflow.Env, rc.Run.Job().Environment(), rc.Config.Env)

would be simpler, wouldn't it? Can you try that instead of testing for existence?

[liamphmurphy]

I think changing the order of parameters from

rc.Env = mergeMaps(rc.Config.Env, rc.Run.Workflow.Env, rc.Run.Job().Environment())

to

rc.Env = mergeMaps(rc.Run.Workflow.Env, rc.Run.Job().Environment(), rc.Config.Env)

would be simpler, wouldn't it? Can you try that instead of testing for existence?

Yeah that's a better way, idk why I didn't think of that, will push that up. Thanks!

[mergify]

@liamphmurphy this pull request has failed checks 🛠

[KnisterPeter]

Looks good

[cplee]

Thanks!