Combine 2 fixes so that enough checks pass to be able to merge #15771
Conversation
unique-ify name psh, who needs loops Folder management Extracts into current path
|
AlanCoding
requested review from
TheRealHaoLiu,
chrismeyersfsu,
thedoubl3j,
arrestle and
djyasin
| for i in coverage-*; do | ||
| cp -rv $i/* ~/.ansible/collections/ansible_collections/awx/awx/tests/output/coverage/ | ||
| done | ||
| cp -rv coverage/* ~/.ansible/collections/ansible_collections/awx/awx/tests/output/coverage/ |
There was a problem hiding this comment.
It would be good to have better documentation. But this is tied into the change in .github/actions/upload_awx_devel_logs/action.yml in a non-obvious way. Because we previously used the same name for upload of each artifact. It would then put each item into a different folder based on the path. Using the same name is exactly what the v3-v4 change prohibited. So now we upload based on different names, but the actions/download-artifact changes here put them into the same local folder. So the loop is removed. Each item has kind of like coverage "fragments". Were were combining the fragments into the same folder eventually anyway, so yeah the ultimate behavior should be the same.
There was a problem hiding this comment.
this feels equivalent
It's not quite. The previous one iterates all directories starting with coverage- and copies all their files into the target. The new one just copies all the coverage-* directories into the target. i.e. the first copies all the inner files of the directories, the second copies the directories themselves.
Something like this might be equivalent if you really want to lose the loop:
cp -rv coverage/coverage-*/* ~/.ansible/collections/ansible_collections/awx/awx/tests/output/coverage/
There was a problem hiding this comment.
But it sounds like this recursion is unnecessary now based on what @AlanCoding is saying. It sounds like there's no inner directory anymore?
| - name: Download coverage artifacts A to H | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: coverage-a-h |
There was a problem hiding this comment.
I dislike the "groups" being hardcoded here 😕 - when I wrote this I tried really hard to make sure there was only one place the groups had to be defined (and that was in the CI matrix that actually splits up the groups.)
There was a problem hiding this comment.
In prior state, to accomplish that, the coverage files were uploaded with the same name. Apparently, uploading with the same name resulted in merging them into the same folder, or using subfolders specified by path. This specific behavior was yanked from the Github action, so that name must always be unique. The need to download each became a problem with this change from the upload-artifact action. If this is not satisfied, it outright fails the check whenever you get to the step.
SUMMARY
This is #15768 and #15767 because neither one fixes enough to be able to merge anything.
ISSUE TYPE
COMPONENT NAME