QA » openQA Project

We were talking about this ticket in the estimation call.
The team understood the ticket as: Use the same github action as we use in os-autoinst-common to run perltidy https://github.com/os-autoinst/os-autoinst-common/blob/master/.github/workflows/perl-lint-checks.yml and perlcritic https://github.com/os-autoinst/os-autoinst-common/blob/master/.github/workflows/perl-critic.yml

For perlcritic that would work. Existing perlcritic rules are not changing over time depending on the module version (there might be rare expections, maybe fixed bugs in perl parsing).

For perltidy we are currently using CircleCI, and just changing this check to github would be "problematic", as we would lose our automatic update of formatting whenever perltidy is updated. Maybe we can live with that, I just want to mention it.

How the dependency cron is currently working¶

So we have the dependency cron running nightly on circleci.

https://github.com/os-autoinst/openQA/blob/master/tools/ci/prepare_dependency_pr.sh
It calls
https://github.com/os-autoinst/openQA/blob/master/tools/ci/build_dependencies.sh which installs openQA-devel and then does a diff against the installed packages before the installation.

Here is an example:
https://app.circleci.com/pipelines/github/os-autoinst/openQA/13102/workflows/42deeacd-3443-4aaf-9e80-8af40f06b7aa/jobs/122138

--- a/tools/ci/ci-packages.txt
+++ b/tools/ci/ci-packages.txt
@@ -238,7 +238,7 @@ perl-Perl-Critic-Policy-Variables-ProhibitLoopOnHash-0.008
 perl-Perl-Critic-Pulp-99
 perl-PerlIO-Layers-0.012
 perl-PerlIO-utf8_strict-0.008
-perl-Perl-Tidy-20230912.0.0
+perl-Perl-Tidy-20240202.0.0
 perl-Pod-MinimumVersion-50
 perl-Pod-POM-2.01
 perl-Pod-Spell-1.25
@@ -315,7 +315,7 @@ perl-XML-SAX-Expat-0.51
 perl-XML-SemanticDiff-1.0007
 perl-XML-Simple-2.24
 perl-YAML-1.24
-perl-YAML-LibYAML-0.88
+perl-YAML-LibYAML-0.890.0
 perl-YAML-PP-0.035
 psmisc-23.0
 python3-olefile-0.46

So it sees that packages were updated.
In that case it creates a PR with the updated packges: https://github.com/os-autoinst/openQA/pull/5505
Why? To actually run our testsuite with the updated packages to see if everything is still working.
That includes formatting changes by perltidy, as they could theoretically break tests.

As a result we see the PR, and if it passes, it will automatically be merged.

Now, if we didn't run the perltidy check in CircleCI anymore, but in GitHub, then the mentioned PR would actually fail, because the Github action would run an older version of perltidy from registry.opensuse.org/devel/openqa/containers/os-autoinst_dev which would possibly (not in all cases, some changes are backwards compatible) complain about the formatting changes.

If we move the perltidy check to github we would have to remove the perltidy automatic update handling from the dependency cron and have to make the formatting changes PR on our own, as soon as the package in registry.opensuse.org/devel/openqa/containers/os-autoinst_dev is updated and PRs start to fail.

But I don't see a big problem with just leaving things as they are.
In os-autoinst, we also didn't copy the os-autoinst-common check, but instead we have this: https://github.com/os-autoinst/os-autoinst/blob/master/.github/workflows/author-tests.yaml
The difference is, it's using cmake, and we couldn't agree on simply using a normal Makefile target for perltidy.

But what we could do for this ticket is linking the .perltidyrc to os-autoinst-common.

Now I get it, thx :)

But what we could do for this ticket is linking the .perltidyrc to os-autoinst-common.

Yes, that's what we should do. The GitHub action definition is simply enough that it's ok if we don't reuse it here. The main goal for the parent epic is anyway to provide good frameworks for test distributions, not necessarily openQA itself.