Skip to content

Feature: uses in composite#793

Merged
mergify[bot] merged 56 commits into
nektos:masterfrom
ChristopherHX:feature/uses-in-composite
Dec 22, 2021
Merged

Feature: uses in composite#793
mergify[bot] merged 56 commits into
nektos:masterfrom
ChristopherHX:feature/uses-in-composite

Conversation

@ChristopherHX

@ChristopherHX ChristopherHX commented Aug 30, 2021

Copy link
Copy Markdown
Contributor

Remove the old hack for composite run steps, use the real runcontext directly.
You can also enable if's in composite, I plan to enable it in https://github.com/ChristopherHX/github-act-runner by default.
If's are now enabled see https://github.com/actions/runner/releases/tag/v2.284.0

Resolves #783
Resolves #926

This change seems to be functional complete.
Multi nested composite needs more Tests, maybe the size will get xl after more tests are added.

@mergify

This comment has been minimized.

@mergify mergify Bot added the needs-work Extra attention is needed label Aug 30, 2021
@mergify

This comment has been minimized.

@mergify

This comment has been minimized.

@mergify

This comment has been minimized.

@mergify

This comment has been minimized.

@mergify

This comment has been minimized.

@mergify

This comment has been minimized.

@mergify

This comment has been minimized.

@ChristopherHX

Copy link
Copy Markdown
Contributor Author

I'm confused,

  • the runner doesn't log the exit code of the tests
  • Check failure on linux/macOS, doesn't contains FAIL in the log either
  • No idea what to try next, is this fail due to the actions/runner???

@ChristopherHX

ChristopherHX commented Aug 30, 2021

Copy link
Copy Markdown
Contributor Author

Wow removing -v from gotest let it succeed (https://github.com/ChristopherHX/act/runs/3462707672?check_suite_focus=true)
Here the same workflow run with -v https://github.com/ChristopherHX/act/runs/3462703235?check_suite_focus=true and it fails.

Oh maybe remove setup-node from tests will solve this....

@mergify

This comment has been minimized.

@codecov

codecov Bot commented Aug 30, 2021

Copy link
Copy Markdown

Codecov Report

Merging #793 (99b21b3) into master (0f04942) will increase coverage by 7.78%.
The diff coverage is 64.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #793      +/-   ##
==========================================
+ Coverage   49.27%   57.05%   +7.78%     
==========================================
  Files          23       28       +5     
  Lines        2401     4222    +1821     
==========================================
+ Hits         1183     2409    +1226     
- Misses       1090     1605     +515     
- Partials      128      208      +80     
Impacted Files Coverage Δ
pkg/common/executor.go 46.90% <0.00%> (+2.03%) ⬆️
pkg/common/job_error.go 0.00% <0.00%> (ø)
pkg/common/outbound_ip.go 0.00% <0.00%> (ø)
pkg/common/testflag.go 0.00% <0.00%> (ø)
pkg/container/docker_volume.go 0.00% <0.00%> (ø)
pkg/model/action.go 0.00% <0.00%> (ø)
pkg/container/docker_run.go 5.54% <14.15%> (+3.61%) ⬆️
pkg/common/git.go 49.82% <31.81%> (-9.97%) ⬇️
pkg/runner/logger.go 65.43% <37.50%> (+1.28%) ⬆️
pkg/container/docker_auth.go 45.00% <45.00%> (ø)
... and 35 more

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 2ef30c3...99b21b3. Read the comment docs.

@mergify mergify Bot removed the needs-work Extra attention is needed label Aug 30, 2021
@mergify

This comment has been minimized.

@mergify mergify Bot added the needs-work Extra attention is needed label Aug 30, 2021
@mergify mergify Bot removed the needs-work Extra attention is needed label Aug 30, 2021
catthehacker
catthehacker previously approved these changes Dec 9, 2021
@jackton1

Copy link
Copy Markdown

@KnisterPeter @catthehacker @ChristopherHX any chance this would be merged soon. I’ll be happy to test the changes.

@ChristopherHX

ChristopherHX commented Dec 14, 2021

Copy link
Copy Markdown
Contributor Author

@jackton1 You can test snapshots of this PR before it is merged ( I have absolutly no permissions for this repo )

I'm awaiting review from cplee, this isn't a small change.

@jackton1

Copy link
Copy Markdown

Okay I’ll test it later today

@jackton1

Copy link
Copy Markdown

@ChristopherHX I can confirm that it works using the snapshot here: https://github.com/nektos/act/actions/runs/1556315888

Only noticed some interpolation errors which could be empty

ERRO[0015] Unable to interpolate string '${{ github.event.pull_request.head.sha }}' - [TypeError: Cannot access member 'head' of undefined] 

FULL LOG OUTPUT

[CI/Test changed-files-1]   ✅  Success - Run changed-files with specific files from a source file using a comma separator
[CI/Test changed-files-1] ⭐  Run Show output
[CI/Test changed-files-1]   🐳  docker exec cmd=[bash --noprofile --norc -e -o pipefail /Users/jacktonye/workspace/changed-files/workflow/41.sh] user= workdir=
| {
|   "added_files": "",
|   "all_changed_and_modified_files": "",
|   "all_changed_files": "",
|   "all_modified_files": "",
|   "any_changed": "false",
|   "any_deleted": "false",
|   "any_modified": "false",
|   "copied_files": "",
|   "deleted_files": "",
|   "modified_files": "",
|   "only_changed": "false",
|   "only_deleted": "",
|   "only_modified": "false",
|   "other_changed_files": "README.md",
|   "other_deleted_files": "",
|   "other_modified_files": "README.md",
|   "renamed_files": "",
|   "type_changed_files": "",
|   "unknown_files": "",
|   "unmerged_files": ""
| }
[CI/Test changed-files-1]   ✅  Success - Show output
[CI/Test changed-files-1] ⭐  Run Verify any_changed files comma separator
[CI/Test changed-files-1]   🐳  docker exec cmd=[bash --noprofile --norc -e -o pipefail /Users/jacktonye/workspace/changed-files/workflow/42.sh] user= workdir=
[CI/Test changed-files-1]   ✅  Success - Verify any_changed files comma separator
[CI/Test changed-files-1] ⭐  Run Verify any_modified files comma separator
[CI/Test changed-files-1]   🐳  docker exec cmd=[bash --noprofile --norc -e -o pipefail /Users/jacktonye/workspace/changed-files/workflow/43.sh] user= workdir=
[CI/Test changed-files-1]   ✅  Success - Verify any_modified files comma separator
[CI/Test changed-files-1] ⭐  Run Verify any_deleted files with comma separator
[CI/Test changed-files-1]   🐳  docker exec cmd=[bash --noprofile --norc -e -o pipefail /Users/jacktonye/workspace/changed-files/workflow/44.sh] user= workdir=
[CI/Test changed-files-1]   ✅  Success - Verify any_deleted files with comma separator
ERRO[0015] Unable to interpolate string '${{ github.event.pull_request.head.sha }}' - [TypeError: Cannot access member 'head' of undefined] 
[CI/Test changed-files-1] ⭐  Run Run changed-files with custom sha
ERRO[0015] Unable to interpolate string '${{ github.event.pull_request.head.sha }}' - [TypeError: Cannot access member 'head' of undefined] 

@KnisterPeter

Copy link
Copy Markdown
Member

The interpolation error should be fixed by the new expression evaluator

cplee
cplee previously approved these changes Dec 22, 2021

@cplee cplee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is awesome ❤️

@mergify

mergify Bot commented Dec 22, 2021

Copy link
Copy Markdown
Contributor

@ChristopherHX this pull request has failed checks 🛠

@mergify

mergify Bot commented Dec 22, 2021

Copy link
Copy Markdown
Contributor

@ChristopherHX this pull request is now in conflict 😩

@mergify

mergify Bot commented Dec 22, 2021

Copy link
Copy Markdown
Contributor

@ChristopherHX this pull request has failed checks 🛠

@ChristopherHX

Copy link
Copy Markdown
Contributor Author

Please help me, what is wrong with superlinter? It just crashs because a file is missing?
https://github.com/nektos/act/runs/4604475831?check_suite_focus=true

I resolved the merge conflict of today.

@catthehacker

Copy link
Copy Markdown
Member

@ChristopherHX it's broken, don't worry about your update

@catthehacker

Copy link
Copy Markdown
Member

@mergify

mergify Bot commented Dec 22, 2021

Copy link
Copy Markdown
Contributor

@ChristopherHX this pull request is now in conflict 😩

@catthehacker

Copy link
Copy Markdown
Member

🎉

@davetapley

Copy link
Copy Markdown
Contributor

@catthehacker any chance of getting a new release with this in? 🙏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL stale-exempt Exempt from stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue: Bug running composite actions Using "uses" inside composite action

7 participants