Skip to content

Evaluate if condition when calling a reusable workflow#2087

Merged
mergify[bot] merged 3 commits intonektos:masterfrom
jenseng:fix-reusable-workflow-if
Nov 12, 2023
Merged

Evaluate if condition when calling a reusable workflow#2087
mergify[bot] merged 3 commits intonektos:masterfrom
jenseng:fix-reusable-workflow-if

Conversation

@jenseng
Copy link
Contributor

@jenseng jenseng commented Nov 10, 2023

Ensure we evaluate the if condition if the job calls a reusable workflow, rather than always assuming it should run.

Fixes #2085

@jenseng jenseng requested a review from a team as a code owner November 10, 2023 17:24
@mergify
Copy link
Contributor

mergify bot commented Nov 10, 2023

@jenseng this pull request has failed checks 🛠

@mergify mergify bot added the needs-work Extra attention is needed label Nov 10, 2023
@jenseng
Copy link
Contributor Author

jenseng commented Nov 10, 2023

Failure appears unrelated, the test in question also fails for me locally on master 🤷

2023-11-10T17:34:32.3426212Z [test network setup/test]   | fv-az888-640.pbnfqiwlmsbu5gn45r0pdzho1a.cx.internal.cloudapp.net
2023-11-10T17:34:32.3426771Z [test network setup/test]   | fv-az888-640.pbnfqiwlmsbu5gn45r0pdzho1a.cx.internal.cloudapp.net has address 10.1.0.103
2023-11-10T17:34:32.3427419Z [test network setup/test]   | Host fv-az888-640.pbnfqiwlmsbu5gn45r0pdzho1a.cx.internal.cloudapp.net not found: 3(NXDOMAIN)
2023-11-10T17:34:32.3427693Z [test network setup/test]   ❌  Failure - Main Run hostname test
2023-11-10T17:34:32.3427886Z [test network setup/test] exitcode '1': failure
2023-11-10T17:34:32.3428053Z [test network setup/test] 🏁  Job failed
2023-11-10T17:34:32.3428302Z [test network setup/test] [DEBUG] Loading revision from git directory
2023-11-10T17:34:32.3428622Z [test network setup/test] [DEBUG] Found revision: 1786ee9eae35d9f3b088ec34b126b22a3e5dc059
2023-11-10T17:34:32.3429016Z [test network setup/test] [DEBUG] HEAD points to '1786ee9eae35d9f3b088ec34b126b22a3e5dc059'
2023-11-10T17:34:32.3429886Z [test network setup/test] unable to get git ref: failed to identify reference (tag/branch) for the checked-out revision '1786ee9eae35d9f3b088ec34b126b22a3e5dc059'
2023-11-10T17:34:32.3430198Z [test network setup/test] [DEBUG] Found revision: 1786ee9eae35d9f3b088ec34b126b22a3e5dc059
2023-11-10T17:34:32.3430368Z     runner_test.go:203: 
2023-11-10T17:34:32.3430769Z         	Error Trace:	/home/runner/work/act/act/pkg/runner/runner_test.go:203
2023-11-10T17:34:32.3431331Z         	            				/home/runner/work/act/act/pkg/runner/runner_test.go:323
2023-11-10T17:34:32.3431811Z         	Error:      	Expected nil, but got: &errors.errorString{s:"Job 'test' failed"}
2023-11-10T17:34:32.3432011Z         	Test:       	TestRunEvent/networking
2023-11-10T17:34:32.3432374Z         	Messages:   	/home/runner/work/act/act/pkg/runner/testdata/networking

@jenseng jenseng force-pushed the fix-reusable-workflow-if branch from 9fba010 to 84365d2 Compare November 11, 2023 19:54
@codecov
Copy link

codecov bot commented Nov 11, 2023 •
edited
Loading

Codecov Report

Merging #2087 (7fe5e12) into master (4989f44) will increase coverage by 0.40%.
Report is 272 commits behind head on master.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##           master    #2087      +/-   ##
==========================================
+ Coverage   61.22%   61.62%   +0.40%     
==========================================
  Files          46       53       +7     
  Lines        7141     8808    +1667     
==========================================
+ Hits         4372     5428    +1056     
- Misses       2462     2950     +488     
- Partials      307      430     +123
Files Coverage Δ
pkg/common/executor.go 51.69% <100.00%> (+1.69%) ⬆️
pkg/container/docker_cli.go 82.23% <ø> (ø)
pkg/container/docker_logger.go 52.08% <ø> (ø)
pkg/runner/step_action_local.go 93.54% <100.00%> (ø)
pkg/runner/step_action_remote.go 91.56% <100.00%> (+0.65%) ⬆️
pkg/runner/step_docker.go 93.18% <100.00%> (ø)
pkg/container/file_collector.go 39.68% <0.00%> (+2.38%) ⬆️
pkg/container/util.go 0.00% <0.00%> (ø)
pkg/container/docker_build.go 60.00% <80.00%> (+1.02%) ⬆️
...ontainer/linux_container_environment_extensions.go 23.07% <0.00%> (-1.25%) ⬇️
... and 31 more

... and 2 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@mergify mergify bot removed the needs-work Extra attention is needed label Nov 11, 2023
ChristopherHX
ChristopherHX approved these changes Nov 12, 2023
View reviewed changes
Copy link
Contributor

@ChristopherHX ChristopherHX left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this.

I believe to have mentioned this defect during review, but my review comment was ignored.
Due to me not using that part of act, I did nothing.

@mergify
Copy link
Contributor

mergify bot commented Nov 12, 2023

@jenseng this pull request has failed checks 🛠

@mergify mergify bot added the needs-work Extra attention is needed label Nov 12, 2023
@mergify mergify bot removed the needs-work Extra attention is needed label Nov 12, 2023
@mergify mergify bot merged commit 5547789 into nektos:master Nov 12, 2023
jmikedupont2 pushed a commit to meta-introspector/act that referenced this pull request Mar 10, 2024
@jenseng @mergify @ChristopherHX
Evaluate if condition when calling a reusable workflow (nektos#2087)
aa6f0fc 
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: ChristopherHX <christopher.homberger@web.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cplee cplee cplee approved these changes

@ChristopherHX ChristopherHX ChristopherHX approved these changes

Assignees

No one assigned

Labels

size/S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Job-level if ignored when calling a reusable workflow

3 participants

@jenseng @cplee @ChristopherHX