QA (public) » openQA Project (public)

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.

I could not resist: https://github.com/os-autoinst/openQA/pull/5838 Use new perltidy wrapper tools/tidyall

I was just looking for tidyall in openQA and didn't find it, only in os-autoinst.
I'm not sure why we split this out in this way and leave this ticket lying around for so long. It's not helpful if one project has /tools/tidyall and the other one doesn't.

https://github.com/os-autoinst/openQA/pull/5838 merged.

https://github.com/os-autoinst/os-autoinst-common/pull/53 Fix Perl::Critic::Policy::HashKeyQuotes

Draft: https://github.com/os-autoinst/openQA/pull/5841

I closed https://github.com/os-autoinst/openQA/pull/5841 , there was something wrong with circleci.

https://github.com/os-autoinst/openQA/pull/5843 Use perlcritic wrapper and plugins from common repo

https://github.com/os-autoinst/openQA/pull/5847 Remove duplicated use
https://github.com/os-autoinst/openQA/pull/5844 Fix some perlcritic complaints
Draft https://github.com/os-autoinst/os-autoinst-common/pull/54

https://github.com/os-autoinst/openQA/pull/5843 is now passing and ready. This should conclude the ticket.