Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable perlcritic checks in CI #31

Closed
wants to merge 5 commits into from
Closed

Enable perlcritic checks in CI #31

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cbaf4c4
Git subrepo clone (merge) external/os-autoinst-common
b10n1k Feb 7, 2024
2373ded
Enable perlcritic checks in CI
b10n1k Feb 7, 2024
b9f34a5
Enable tidy checks in Ci
b10n1k Feb 8, 2024
4d56c6d
Enable yaml checks in CI
b10n1k Feb 8, 2024
8e26f76
Fix isotovideo CI jobs
b10n1k Feb 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions .github/workflows/isotovideo-action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@ jobs:
image: "registry.opensuse.org/devel/openqa/containers/isotovideo:qemu-x86-jq"
steps:
- uses: actions/checkout@v2
Copy link
Member

Choose a reason for hiding this comment

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

Can you also change the action to actions/checkout@v4?

Copy link
Member

Choose a reason for hiding this comment

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

That change should get rid of the deprecation warning seen in https://github.com/os-autoinst/os-autoinst-distri-example/actions/runs/7831480244

- name: install jq
run: zypper -n in jq

- name: Run isotovideo against test code
run: isotovideo qemu_no_kvm=1 casedir=.
run: isotovideo qemu_no_kvm=1 casedir=$(pwd)

- name: fail if any test module failed
run: jq .result testresults/result-*.json | grep -v ok && echo "Test modules failed" && exit 1
run: jq .result testresults/result-*.json | grep ok || (echo "Test modules failed" && exit 1)
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned elsewhere, you probably do not want subshell here, the exit will only exit the subshell.
grep ok || { echo "Test modules failed" && exit 1; }

Copy link
Contributor

Choose a reason for hiding this comment

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

Previously the grep was looking for any line not matching ok, so any failed test would make the workflow fail.
Now you are only ensuring that there is at least one line matching ok. So there could be failed modules and it would still succeed.
I don't know how many testmodules this workflow runs, but I think the logic should be kept as before, or look at @josegomezr ' PR.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, thanks for commenting that. I also noticed that but forget to note down that grep -v ok is not the exact opposite of grep ok (I got sidetracked by the subshell 😄 ). I also vote for the previous logic; I believe the motivation was to avoid the non-zero exit code provided by grep on "success". We might want to use grep -v ok && { echo "Test modules failed" && exit 1; } || true

12 changes: 6 additions & 6 deletions .github/workflows/isotovideo-check-all-test-modules.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ jobs:
steps:
- uses: actions/checkout@v2
- name: Run isotovideo against test code, fail if any test module failed
env:
image: registry.opensuse.org/devel/openqa/containers/isotovideo:qemu-x86-jq
isotovideo: isotovideo qemu_no_kvm=1 casedir=/tests
jq_filter: jq .result testresults/result-*.json
err_msg: Test modules failed
run: docker run --rm -it -v .:/tests:Z --entrypoint '' $image /bin/sh -c '$isotovideo && $jq_filter | grep ok || (echo "$err_msg" && exit 1)'
run: |
docker run --rm -v .:/tests:Z --entrypoint '' \
registry.opensuse.org/devel/openqa/containers/isotovideo:qemu-x86-jq /bin/sh \
-c 'isotovideo qemu_no_kvm=1 casedir=/tests \
&& jq .result testresults/result-*.json | grep ok \
|| (echo "Test modules failed" && exit 1)'
Copy link
Member

Choose a reason for hiding this comment

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

Again the subshell.

2 changes: 1 addition & 1 deletion .github/workflows/isotovideo.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ jobs:
steps:
- uses: actions/checkout@v2
- name: Run isotovideo against test code in happy-path scenario
run: podman run --rm -it -v .:/tests:Z registry.opensuse.org/devel/openqa/containers/isotovideo:qemu-x86 qemu_no_kvm=1 casedir=/tests
run: docker run --rm -v .:/tests:Z registry.opensuse.org/devel/openqa/containers/isotovideo:qemu-x86 qemu_no_kvm=1 casedir=/tests
Copy link
Member

Choose a reason for hiding this comment

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

I understand the removal of -i and -t, why the podman -> docker change?

Loading