From 50cf40d4341640ddd6fea5b05ecbbe93b06d5487 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 2 Dec 2025 09:10:06 +0100 Subject: [PATCH 1/5] gen-manifests: add -print-only We need a `-print-only` option so that we can see what gen-manifests would generate. This will be input for a pytest based framework (but seems generally be useful). This also tweaks the outputs so that any status message now goes to stderr so that one can parse stdout for the list of image manifests that would be generated. --- cmd/gen-manifests/main.go | 38 +++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/cmd/gen-manifests/main.go b/cmd/gen-manifests/main.go index d27006bea1..d264ac8326 100644 --- a/cmd/gen-manifests/main.go +++ b/cmd/gen-manifests/main.go @@ -424,6 +424,10 @@ func main() { flag.Var(&imgTypes, "types", "comma-separated list of image types (globs supported)") flag.Var(&bootcRefs, "bootc-refs", "comma-separated list of bootc-refs") + // print-only + var printOnly bool + flag.BoolVar(&printOnly, "print-only", false, "print what manifests would be generate") + flag.Parse() testedRepoRegistry, err := testrepos.New() @@ -443,7 +447,7 @@ func main() { var configs *BuildConfigs opts := &buildconfig.Options{AllowUnknownFields: buildconfigAllowUnknown} if configPath != "" { - fmt.Println("'-config' was provided, thus ignoring '-config-list' option") + fmt.Fprintln(os.Stderr, "'-config' was provided, thus ignoring '-config-list' option") configs = loadImgConfig(configPath, opts) } else { configs = loadConfigList(configMapPath, opts) @@ -460,7 +464,7 @@ func main() { tmpdirRoot := filepath.Join(os.TempDir(), "gen-manifests-tmpdir") defer os.RemoveAll(tmpdirRoot) - fmt.Println("Collecting jobs") + fmt.Fprintln(os.Stderr, "Collecting jobs") distros, invalidDistros := distros.ResolveArgValues(testedRepoRegistry.ListDistros()) if len(invalidDistros) > 0 { @@ -503,7 +507,7 @@ func main() { if len(repos) == 0 { fmt.Printf("no repositories defined for %s/%s/%s\n", distroName, archName, imgTypeName) if skipNorepos { - fmt.Println("Skipping") + fmt.Fprintln(os.Stderr, "Skipping") continue } panic("no repositories found, pass --skip-norepos to skip") @@ -523,8 +527,12 @@ func main() { continue } - job := makeManifestJob(itConfig, imgType, distribution, repos, archName, cacheRoot, outputDir, contentResolve, metadata, tmpdirRoot) - jobs = append(jobs, job) + if printOnly { + fmt.Printf("%s,%s,%s,%s\n", distribution.Name(), archName, imgType.Name(), itConfig.Name) + } else { + job := makeManifestJob(itConfig, imgType, distribution, repos, archName, cacheRoot, outputDir, contentResolve, metadata, tmpdirRoot) + jobs = append(jobs, job) + } } } } @@ -577,9 +585,13 @@ func main() { continue } - var repos []rpmmd.RepoConfig - job := makeManifestJob(itConfig, imgType, distribution, repos, archName, cacheRoot, outputDir, contentResolve, metadata, tmpdirRoot) - jobs = append(jobs, job) + if printOnly { + fmt.Printf("%s,%s,%s,%s\n", distribution.Name(), archName, imgType.Name(), itConfig.Name) + } else { + var repos []rpmmd.RepoConfig + job := makeManifestJob(itConfig, imgType, distribution, repos, archName, cacheRoot, outputDir, contentResolve, metadata, tmpdirRoot) + jobs = append(jobs, job) + } } } } @@ -669,17 +681,17 @@ func main() { } nJobs := len(jobs) - fmt.Printf("Collected %d jobs\n", nJobs) + fmt.Fprintf(os.Stderr, "Collected %d jobs\n", nJobs) // nolint:gosec wq := newWorkerQueue(uint32(nWorkers), uint32(nJobs)) wq.start() - fmt.Printf("Initialised %d workers\n", nWorkers) - fmt.Printf("Submitting %d jobs... ", nJobs) + fmt.Fprintf(os.Stderr, "Initialised %d workers\n", nWorkers) + fmt.Fprintf(os.Stderr, "Submitting %d jobs... ", nJobs) for _, j := range jobs { wq.submitJob(j) } - fmt.Println("done") + fmt.Fprintln(os.Stderr, "done") errs := wq.wait() exit := 0 if nErrs := len(errs); nErrs > 0 { @@ -697,6 +709,6 @@ func main() { } exit = 1 } - fmt.Printf("RPM metadata cache kept in %s\n", cacheRoot) + fmt.Fprintf(os.Stderr, "RPM metadata cache kept in %s\n", cacheRoot) os.Exit(exit) } From 5b3b4469bc790453ed03d07e91dd65eface95395 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 26 Nov 2025 13:39:05 +0100 Subject: [PATCH 2/5] test: add/use a pytest wrapper for the build/boot tests This is a tiny wrapper around the build/boot tests scripts to run them via pytest. Why another layer of indirection you ask? 1. it makes it easy to enumerate all tests one can run via `pytest --collect-only` 2. it makes it easy to run selected tests: `pytest -k rhel-10.2` 3. it provides a uniform way to that is the same for all pytest projects 4. with a sufficiently small number of tests (via -k) we could do --parallel 5. it abstracts away from the underlying scripts, we can (potentially) change how we run this without changing the higher layers Now that tests are easy to run locally it seems like a really nice feature. --- test/scripts/generate-build-config | 5 +-- test/scripts/generate-ostree-build-config | 5 +-- test/scripts/install-dependencies | 1 + test/test_build_and_boot.py | 49 +++++++++++++++++++++++ 4 files changed, 54 insertions(+), 6 deletions(-) create mode 100644 test/test_build_and_boot.py diff --git a/test/scripts/generate-build-config b/test/scripts/generate-build-config index bd86996945..4ec2742f2c 100755 --- a/test/scripts/generate-build-config +++ b/test/scripts/generate-build-config @@ -13,9 +13,8 @@ build/{distro}/{arch}/{image_type}/{config_name}: script: - sudo ./test/scripts/setup-osbuild-repo - sudo ./test/scripts/install-dependencies - - ./test/scripts/build-image "{distro}" "{image_type}" "{config}" - - ./test/scripts/boot-image "{image_path}" - - ./test/scripts/upload-results "{distro}" "{image_type}" "{config}" + - sudo -E pytest -k "{distro}-{arch}-{image_type}-{config_name}" ./test + - sudo -E ./test/scripts/upload-results "{distro}" "{image_type}" "{config}" extends: .terraform tags: - {tag} diff --git a/test/scripts/generate-ostree-build-config b/test/scripts/generate-ostree-build-config index 8afd0d9d1d..e2c96e97b7 100755 --- a/test/scripts/generate-ostree-build-config +++ b/test/scripts/generate-ostree-build-config @@ -15,9 +15,8 @@ build/{distro}/{arch}/{image_type}/{config_name}: - sudo ./test/scripts/install-dependencies - {dl_container} - {start_container} - - ./test/scripts/build-image "{distro}" "{image_type}" "{config}" - - ./test/scripts/boot-image "{image_path}" - - ./test/scripts/upload-results "{distro}" "{image_type}" "{config}" + - sudo -E pytest -k "{distro}-{arch}-{image_type}-{config_name}" ./test + - sudo -E ./test/scripts/upload-results "{distro}" "{image_type}" "{config}" extends: .terraform variables: RUNNER: {runner}-{arch} diff --git a/test/scripts/install-dependencies b/test/scripts/install-dependencies index 5d03a68e9b..fa154394a2 100755 --- a/test/scripts/install-dependencies +++ b/test/scripts/install-dependencies @@ -22,6 +22,7 @@ dnf -y install \ podman \ python3 \ python3-pip \ + python3-pytest \ yamllint \ xz diff --git a/test/test_build_and_boot.py b/test/test_build_and_boot.py new file mode 100644 index 0000000000..b7bc6ad9f7 --- /dev/null +++ b/test/test_build_and_boot.py @@ -0,0 +1,49 @@ +import platform +import subprocess + +import pytest +import scripts.imgtestlib as testlib + +def _test_cases(): + # XXX: make testcase a data class + all_test_cases = subprocess.check_output([ + "go", "run", "-buildvcs=false", "./cmd/gen-manifests", + # we may consider cross arch tests here at some point but for now + # assume we run native + "-arches", platform.uname().machine, + "-print-only", + ], text=True).strip().split("\n") + boot_tests = set() + for tcase in all_test_cases: + distro, arch, image_type, config_name = tcase.split(",") + if not (image_type in testlib.CAN_BOOT_TEST["*"] or image_type in testlib.CAN_BOOT_TEST.get(arch, [])): + continue + # XXX: we need to filter further here, i.e. not all qemu tests can be run currently, e.g. + # if sshd is missing (see boot_image.ensure_can_run_qemu_test - however this needs + # a build/ dir so we cannot filter right now without also building all manifests. This + # should instead be refactored so that we have a testcase class and we can filter more + # flexible on that and exclude e.g. all image types with "minmal" and "minimal-pkgs" + # as their config_name + boot_tests.add(tcase) + return { + "build_only": set(all_test_cases)-boot_tests, + "build_and_boot": boot_tests, + } + + +@pytest.mark.parametrize("distro,arch,image_type,config_name", [tcase.split(",") for tcase in _test_cases()["build_only"]]) +def test_build_only(distro,arch,image_type,config_name): + subprocess.check_call( + ["./test/scripts/build-image", distro, image_type, f"test/configs/{config_name}.json"]) + build_dir = os.path.join("build", testlib.gen_build_name(distro, arch, image_type, config_name)) + subprocess.check_call( + ["./test/scripts/boot-image", build_dir]) + + +@pytest.mark.parametrize("distro,arch,image_type,config_name", [tcase.split(",") for tcase in _test_cases()["build_and_boot"]]) +def test_build_and_boot(distro,arch,image_type,config_name): + subprocess.check_call( + ["./test/scripts/build-image", distro, image_type, f"test/configs/{config_name}.json"]) + build_dir = os.path.join("build", testlib.gen_build_name(distro, arch, image_type, config_name)) + subprocess.check_call( + ["./test/scripts/boot-image", build_dir]) From 5bf529056b27fa6e7e37ec5361a24bbaff90033e Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 27 Nov 2025 10:18:53 +0100 Subject: [PATCH 3/5] test: skip build/boot tests as non-root This skips the build and boot tests for non-root users, they are currently required to run as root. A nice side effect is that the normal workflow of running pytest to run the "normal" tests is not interrupted. We could of course also use pytest and a custom "marker" like "@pytest.mark.integration" if this commit is considered too loose. --- test/test_build_and_boot.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/test_build_and_boot.py b/test/test_build_and_boot.py index b7bc6ad9f7..818bf7ec38 100644 --- a/test/test_build_and_boot.py +++ b/test/test_build_and_boot.py @@ -1,9 +1,13 @@ +import os import platform import subprocess import pytest import scripts.imgtestlib as testlib +if os.getuid() != 0: + pytest.skip(reason="need root to build the images", allow_module_level=True) + def _test_cases(): # XXX: make testcase a data class all_test_cases = subprocess.check_output([ From 88b828c6d9f2974b75c1fc03b4a043cb472a7a57 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 27 Nov 2025 10:24:58 +0100 Subject: [PATCH 4/5] test: rename test_build_and_boot->test_build_integration This makes it nicer to use `pytest -k build_and_boot` to find only our build and boot tests. --- test/{test_build_and_boot.py => test_build_integration.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/{test_build_and_boot.py => test_build_integration.py} (100%) diff --git a/test/test_build_and_boot.py b/test/test_build_integration.py similarity index 100% rename from test/test_build_and_boot.py rename to test/test_build_integration.py From e15c7f5389c3d2e65778360e2d0452f73a394c3e Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 2 Dec 2025 09:25:58 +0100 Subject: [PATCH 5/5] test,workflow: use custom marker to exclude integration tests We need to excude the integration tests in the python-test workflow. So add a custom pytest marker. We could also move the integration tests into their own dir, I have no strong opinion. --- .github/workflows/tests.yml | 2 +- test/conftest.py | 4 ++++ test/test_build_integration.py | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 test/conftest.py diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7a0b1f3315..8ad8006abc 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -229,7 +229,7 @@ jobs: - name: Testing imgtestlib and test scripts run: | pip install . - python3 -m pytest -v + python3 -m pytest -v -k "not images_integration" python-lint: name: "🐍 Lint (test scripts)" diff --git a/test/conftest.py b/test/conftest.py new file mode 100644 index 0000000000..f332d21b83 --- /dev/null +++ b/test/conftest.py @@ -0,0 +1,4 @@ +def pytest_configure(config): + config.addinivalue_line( + "markers", "images_integration" + ) diff --git a/test/test_build_integration.py b/test/test_build_integration.py index 818bf7ec38..b78474ec87 100644 --- a/test/test_build_integration.py +++ b/test/test_build_integration.py @@ -35,6 +35,7 @@ def _test_cases(): } +@pytest.mark.images_integration @pytest.mark.parametrize("distro,arch,image_type,config_name", [tcase.split(",") for tcase in _test_cases()["build_only"]]) def test_build_only(distro,arch,image_type,config_name): subprocess.check_call( @@ -44,6 +45,7 @@ def test_build_only(distro,arch,image_type,config_name): ["./test/scripts/boot-image", build_dir]) +@pytest.mark.images_integration @pytest.mark.parametrize("distro,arch,image_type,config_name", [tcase.split(",") for tcase in _test_cases()["build_and_boot"]]) def test_build_and_boot(distro,arch,image_type,config_name): subprocess.check_call(