Project

General

Profile

Actions

action #155188

closed

openQA Tests - coordination #96596: [qe-core][CI] CI/CD and Coding style improvements

Unify GitHub Actions for QA Projects - perltidy&perlcritic in openQA

Added by okurz 8 months ago. Updated 2 months ago.

Status:
Resolved
Priority:
Low
Assignee:
Category:
Feature requests
Target version:
Start date:
2023-10-24
Due date:
% Done:

0%

Estimated time:

Description

Motivation

See #138416

Acceptance criteria

  • AC1: openQA inherit perltidy&perlcritic checks from os-autoinst-common
  • AC2: Checks in openQA still run successfully on the master branch

Suggestions

  • Consider the dependencycron updates to dependencies which currently runs in CircleCI
    • Moving the deps to GHA would imply moving all tests to GHA
  • Follow #155062 but for openQA

Related issues 2 (1 open1 closed)

Copied from openQA Project - action #155062: Unify GitHub Actions for QA Projects - perltidy in os-autoinst size:MResolvedtinita2023-10-24

Actions
Copied to openQA Project - action #155191: Unify GitHub Actions for QA Projects - perlcritic in os-autoinstNew2023-10-24

Actions
Actions #1

Updated by okurz 8 months ago

  • Copied from action #155062: Unify GitHub Actions for QA Projects - perltidy in os-autoinst size:M added
Actions #2

Updated by okurz 8 months ago

  • Copied to action #155191: Unify GitHub Actions for QA Projects - perlcritic in os-autoinst added
Actions #3

Updated by okurz 7 months ago

  • Target version changed from Tools - Next to Ready
Actions #4

Updated by livdywan 6 months ago

  • Description updated (diff)
Actions #5

Updated by tinita 6 months ago

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.

Actions #6

Updated by okurz 6 months ago

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.

Actions #7

Updated by okurz 6 months ago

  • Target version changed from Ready to future
Actions #8

Updated by tinita 2 months ago

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.

Actions #9

Updated by okurz 2 months ago

  • Status changed from New to In Progress
  • Assignee set to tinita
  • Target version changed from future to Ready
Actions #11

Updated by tinita 2 months ago

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

Actions #13

Updated by tinita 2 months ago · Edited

  • Status changed from In Progress to Feedback

https://github.com/os-autoinst/os-autoinst-common/pull/54 Fix and test perlcritic plugins (merged)

Actions #14

Updated by tinita 2 months ago

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

Actions #15

Updated by tinita 2 months ago

  • Status changed from Feedback to Resolved
Actions

Also available in: Atom PDF