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.

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

DRAFT: SLE-M: introduced the ownership filter #20797

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

m-dati
Copy link
Contributor

@m-dati m-dati commented Dec 12, 2024

Introduced, for SLE-Micro test code flow, the QEC_SLEM_TESTS_FILTER parameter:
when =1, the ownership is triggered, based on poo#161150 and coverage table, so that Not qe-c owned test modules are Skipped.

Copy link

Great PR! Please pay attention to the following items before merging:

Files matching lib/**.pm:

  • Consider adding or extending unit tests in t/

This is an automatically generated QA checklist based on modified files.

@m-dati m-dati changed the title SLE-M: introduced the QEC_TEST_FILTER filter SLE-M: introduced the ownership filter Dec 12, 2024
@m-dati
Copy link
Contributor Author

m-dati commented Dec 12, 2024

Note: having test slem_image_default_test added with MR 1974, after this PR merged, we can add the QEC parameter filter too in the yaml, to run owned sle-m tests only.

@@ -32,6 +32,11 @@ sub is_dvd {
return get_required_var('FLAVOR') =~ /dvd/i;
}

sub is_qec_test_run {
my $vx = get_var('QEC_TEST_FILTER');
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
my $vx = get_var('QEC_TEST_FILTER');
return check_var('SLEM_IMAGE_TESTS', '1');

QEC_TEST_FILTER appears too generic to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK; but being it dedicated to ownership, then better: QEC_SLEM_IMAGE_TESTS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, I considered the case when param. undefined no filtering shall be applied

Copy link
Contributor Author

@m-dati m-dati Dec 12, 2024

Choose a reason for hiding this comment

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

But, another consideration on name: this new param. is not only for image tests, even being now used for that case only 🙂, so I'd prefer to call it QEC_SLEM_TESTS or QEC_SLEM_TESTS_FILTER

Copy link
Contributor

Choose a reason for hiding this comment

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

QEC runs container engine, SLEM image and BCI tests on SLE-Micro. We have already now three different test cases, so I'd argue that QEC_SLEM_TESTS or QEC_SLEM_TESTS_FILTER does not distinguish between them sufficiently.

lib/main_micro_alp.pm Outdated Show resolved Hide resolved
lib/main_micro_alp.pm Outdated Show resolved Hide resolved
@m-dati
Copy link
Contributor Author

m-dati commented Dec 12, 2024

VRs rerun, after review applied, QEC_SLEM_TESTS_FILTER=1

https://openqa.suse.de/tests/16185952 VR2

https://openqa.suse.de/tests/16185953 containers: non regression

QEC_SLEM_TESTS_FILTER undefined:
https://openqa.suse.de/tests/16185975 run does not limited

  When =1 ownership triggered, based on poo#161150 and coverage table
@mloviska
Copy link
Contributor

Although this is one of the possible solutions, I would personally prefer to have a set of functions that are loaded according to test definition. This way we avoid creating team specific spaghetti and give it a more logical definition.

In TW there is EXTRA variable used, where users can set certain test scope/topic e.g. provisioning, networking etc additional to the standard tests.

A code example would be over here https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/lib/main_micro_alp.pm#L438

In our case, we need to load only tests that are testing the image setup itself. So we can either create a several atomic load functions, and then specify them using EXTRA.

@m-dati
Copy link
Contributor Author

m-dati commented Dec 13, 2024

Here are some VR cloned from last added test slem_image_default_test run in grp 420 in this PR's branch.

Adding the QEC_SLEM_TESTS_FILTER=1 to trigger the checks added in this PR, the only qec modules were executed:
https://openqa.suse.de/tests/16192371
https://openqa.suse.de/tests/16192372
https://openqa.suse.de/tests/16192373
https://openqa.suse.de/tests/16192374
https://openqa.suse.de/tests/16192375

@m-dati m-dati marked this pull request as draft December 13, 2024 14:46
@m-dati m-dati changed the title SLE-M: introduced the ownership filter DRAFT: SLE-M: introduced the ownership filter Dec 13, 2024
@m-dati m-dati added the WIP Work in progress label Dec 13, 2024
@m-dati m-dati assigned m-dati and unassigned m-dati Dec 13, 2024
@m-dati
Copy link
Contributor Author

m-dati commented Dec 13, 2024

Given the discussion still ongoing, PR set draft, for new analysis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants