Repository: osbuild/osbuild
Pull request #2016 (2025-02-14 13:22:06)

Rework `osbuild-depsolve-dnf` tests (HMS-5509)
The test matrix in `osbuild-depsolve-dnf` got out of hand. We were increasing the parametrization of `test_depsolve()` test case (and others) to test new features. However, for each test case instance, we also ran the same test case 8 times for each possible repo config combination and optional metadata requested in the request (2\*2\*2).

This PR improves the situation by extracting the repo config combo testing to dedicated test cases, which test all combinations with a single test scenario containing multiple repositories.

I've done some additional refactoring along the way to reduce code duplication and improve testing accuracy (e.g., don't set arguments in the request, which should not be used in the test scenario).

From local testing of `python3 -m pytest -vv tools/test/test_depsolve.py`:

- Before: 1083.64s - 00:18:03
- After: 290.32s - 00:04:50

**This means that tests run almost 4 times faster.**

The effect on `Test / Unittest (normal, pyxx)` in GH action is ~20m before vs. ~8m after.

/jira-epic COMPOSER-2405

JIRA: [HMS-5509](https://issues.redhat.com/browse/HMS-5509)

thozza: > Amazing improvement and something that indeed annoyed me during the modularity tests as well (which are exactly the ones that made the time here blow up to be unacceptable, I guess :)).

The time was unacceptable even before modularity. :sweat_smile: The actual reason why I looked at this is that I'm going to add one more dimension for SBOM testing, and that would make it really annoying :innocent: 
thozza: I've addressed @mvo5 's comments...

Patch:
diff --git a/osbuild/monitor.py b/osbuild/monitor.py
index 9ece10f3..0393aab1 100644
--- a/osbuild/monitor.py
+++ b/osbuild/monitor.py
@@ -341,12 +341,32 @@ class JSONSeqMonitor(BaseMonitor):
         self._context.set_stage(module)
         self.log(f"Starting module {module.name}", origin="osbuild.monitor")
 
-    # result is for modules
     def result(self, result: Union[BuildResult, DownloadResult]) -> None:
+        """ Called when the module (stage or download) is finished
+
+        This will stream a log entry that the stage finished and the result
+        is available via the json-seq monitor as well. Note that while the
+        stage output is part of the result it may be abbreviated. To get
+        an entire buildlog the consumer needs to simply log the calls to
+        "log()" which contain more detailed information as well.
+        """
         # we may need to check pipeline ids here in the future
         if self._progress.sub_progress:
             self._progress.sub_progress.incr()
 
+        # Limit the output in the json pipeline to a "reasonable"
+        # length. We ran into an issue from a combination of a stage
+        # that produce tons of output (~256 kb, see issue#1976) and
+        # the consumer that used a golang scanner with a max default
+        # buffer of 64kb before erroring.
+        #
+        # Consumers can collect the individual log lines on their own
+        # if desired via the "log()" method.
+        max_output_len = 31_000
+        if len(result.output) > max_output_len:
+            removed = len(result.output) - max_output_len
+            result.output = f"[...{removed} bytes hidden...]\n{result.output[removed:]}"
+
         self._jsonseq(log_entry(
             f"Finished module {result.name}",
             context=self._context.with_origin("osbuild.monitor"),
diff --git a/stages/org.osbuild.coreos.live-artifacts.mono b/stages/org.osbuild.coreos.live-artifacts.mono
index 302b24d3..a5837deb 100755
--- a/stages/org.osbuild.coreos.live-artifacts.mono
+++ b/stages/org.osbuild.coreos.live-artifacts.mono
@@ -606,10 +606,14 @@ def mkrootfs_metal(paths, workdir, img_metal, fstype, fsoptions, loop_client):
             # Note the filename must be exactly "root.[squash|ero]fs"
             # because the 35coreos-live dracut module requires it.
             if fstype == "erofs":
+                # Set TMPDIR='/var/tmp' to write temporary files into non-tmpfs
+                # (memory backed) storage. This helps reduce memory requirements.
+                # https://github.com/erofs/erofs-utils/issues/13
                 subprocess.check_call(['mkfs.erofs',
                                        *fsoptions.split(' '),
                                        paths["initrd-rootfs/root.erofs"],
-                                       tmp_rootfs_dir])
+                                       tmp_rootfs_dir],
+                                      env=dict(os.environ, TMPDIR='/var/tmp'))
             else:
                 subprocess.check_call(['mksquashfs', tmp_rootfs_dir,
                                        paths["initrd-rootfs/root.squashfs"],


Test patch:
diff --git a/test/mod/test_monitor.py b/test/mod/test_monitor.py
index 148a4957..1eef7553 100644
--- a/test/mod/test_monitor.py
+++ b/test/mod/test_monitor.py
@@ -19,7 +19,7 @@ import osbuild
 import osbuild.meta
 from osbuild.monitor import Context, JSONSeqMonitor, LogMonitor, Progress, log_entry
 from osbuild.objectstore import ObjectStore
-from osbuild.pipeline import Runner
+from osbuild.pipeline import BuildResult, Runner, Stage
 
 from .. import test
 
@@ -442,3 +442,21 @@ def test_json_progress_monitor_handles_racy_writes(tmp_path):
                 json.loads(line)
             except json.decoder.JSONDecodeError:
                 pytest.fail(f"the jsonseq stream is not valid json, got {line}")
+
+
+def test_json_progress_monitor_excessive_output_in_result(tmp_path):
+    index = osbuild.meta.Index(os.curdir)
+    info = index.get_module_info("Stage", "org.osbuild.noop")
+    stage = Stage(info, "source_opts", "build", "base", "options", "source_epoch")
+
+    output_path = tmp_path / "jsonseq.log"
+    output = "beginning-marker-vanishes\n" + "1" * 32_000 + "\nend-marker"
+    build_result = BuildResult(stage, 0, output, {})
+    with output_path.open("w") as fp:
+        mon = JSONSeqMonitor(fp.fileno(), 1)
+        mon.result(build_result)
+    with output_path.open() as fp:
+        line = fp.readline().strip("\x1e")
+        json_result = json.loads(line)
+    assert json_result["result"]["output"].startswith("[...1037 bytes hidden...]\n1111")
+    assert json_result["result"]["output"].endswith("1111\nend-marker")
diff --git a/tools/test/test_depsolve.py b/tools/test/test_depsolve.py
index ba0c7e07..21c5f735 100644
--- a/tools/test/test_depsolve.py
+++ b/tools/test/test_depsolve.py
@@ -40,7 +40,11 @@ def assert_dnf():
         raise RuntimeError("Cannot import dnf")
 
 
-def depsolve(transactions, repos, root_dir, cache_dir, dnf_config, opt_metadata, with_sbom=False) -> Tuple[dict, int]:
+def depsolve(transactions, cache_dir, dnf_config=None, repos=None, root_dir=None,
+             opt_metadata=None, with_sbom=False) -> Tuple[dict, int]:
+    if not repos and not root_dir:
+        raise ValueError("At least one of 'repos' or 'root_dir' must be specified")
+
     req = {
         "command": "depsolve",
         "arch": ARCH,
@@ -48,13 +52,19 @@ def depsolve(transactions, repos, root_dir, cache_dir, dnf_config, opt_metadata,
         "releasever": RELEASEVER,
         "cachedir": cache_dir,
         "arguments": {
-            "root_dir": root_dir,
-            "repos": repos,
-            "optional-metadata": opt_metadata,
             "transactions": transactions,
         }
     }
 
+    if repos:
+        req["arguments"]["repos"] = repos
+
+    if root_dir:
+        req["arguments"]["root_dir"] = root_dir
+
+    if opt_metadata:
+        req["arguments"]["optional-metadata"] = opt_metadata
+
     if with_sbom:
         req["arguments"]["sbom"] = {"type": "spdx"}
 
@@ -72,26 +82,34 @@ def depsolve(transactions, repos, root_dir, cache_dir, dnf_config, opt_metadata,
         return json.loads(p.stdout), p.returncode
 
 
-def dump(repos, root_dir, cache_dir, dnf_config, opt_metadata) -> Tuple[dict, int]:
+def dump(cache_dir, dnf_config, repos=None, root_dir=None, opt_metadata=None) -> Tuple[dict, int]:
+    if not repos and not root_dir:
+        raise ValueError("At least one of 'repos' or 'root_dir' must be specified")
+
     req = {
         "command": "dump",
         "arch": ARCH,
         "module_platform_id": f"platform:el{RELEASEVER}",
         "releasever": RELEASEVER,
         "cachedir": cache_dir,
-        "arguments": {
-            "root_dir": root_dir,
-            "repos": repos,
-            "optional-metadata": opt_metadata,
-        }
+        "arguments": {}
     }
 
+    if repos:
+        req["arguments"]["repos"] = repos
+
+    if root_dir:
+        req["arguments"]["root_dir"] = root_dir
+
+    if opt_metadata:
+        req["arguments"]["optional-metadata"] = opt_metadata
+
     # If there is a config file, write it to a temporary file and pass it to the depsolver
     with TemporaryDirectory() as cfg_dir:
         env = None
         if dnf_config:
             cfg_file = pathlib.Path(cfg_dir) / "solver.json"
-            cfg_file.write_text(dnf_config)
+            json.dump(dnf_config, cfg_file.open("w"))
             env = {"OSBUILD_SOLVER_CONFIG": os.fspath(cfg_file)}
 
         p = sp.run(["./tools/osbuild-depsolve-dnf"], input=json.dumps(req), env=env,
@@ -100,7 +118,10 @@ def dump(repos, root_dir, cache_dir, dnf_config, opt_metadata) -> Tuple[dict, in
         return json.loads(p.stdout), p.returncode
 
 
-def search(search_args, repos, root_dir, cache_dir, dnf_config, opt_metadata) -> Tuple[dict, int]:
+def search(search_args, cache_dir, dnf_config, repos=None, root_dir=None, opt_metadata=None) -> Tuple[dict, int]:
+    if not repos and not root_dir:
+        raise ValueError("At least one of 'repos' or 'root_dir' must be specified")
+
     req = {
         "command": "search",
         "arch": ARCH,
@@ -109,18 +130,24 @@ def search(search_args, repos, root_dir, cache_dir, dnf_config, opt_metadata) ->
         "cachedir": cache_dir,
         "arguments": {
             "search": search_args,
-            "root_dir": root_dir,
-            "repos": repos,
-            "optional-metadata": opt_metadata,
         }
     }
 
+    if repos:
+        req["arguments"]["repos"] = repos
+
+    if root_dir:
+        req["arguments"]["root_dir"] = root_dir
+
+    if opt_metadata:
+        req["arguments"]["optional-metadata"] = opt_metadata
+
     # If there is a config file, write it to a temporary file and pass it to the depsolver
     with TemporaryDirectory() as cfg_dir:
         env = None
         if dnf_config:
             cfg_file = pathlib.Path(cfg_dir) / "solver.json"
-            cfg_file.write_text(dnf_config)
+            json.dump(dnf_config, cfg_file.open("w"))
             env = {"OSBUILD_SOLVER_CONFIG": os.fspath(cfg_file)}
 
         p = sp.run(["./tools/osbuild-depsolve-dnf"], input=json.dumps(req), env=env,
@@ -155,6 +182,43 @@ def tcase_idfn(param):
     return param['id']
 
 
+depsolve_test_case_basic_2pkgs_2repos = {
+    "id": "basic_2pkgs_2repos",
+    "enabled_repos": ["baseos", "custom"],
+    "transactions": [
+        {
+            "package-specs": [
+                "filesystem",
+                "pkg-with-no-deps"
+            ],
+        },
+    ],
+    "results": {
+        "packages": {
+            "basesystem",
+            "bash",
+            "centos-gpg-keys",
+            "centos-stream-release",
+            "centos-stream-repos",
+            "filesystem",
+            "glibc",
+            "glibc-common",
+            "glibc-minimal-langpack",
+            "libgcc",
+            "ncurses-base",
+            "ncurses-libs",
+            "setup",
+            "tzdata",
+            "pkg-with-no-deps",
+        },
+        "reponames": {
+            "baseos",
+            "custom",
+        },
+    },
+}
+
+
 depsolve_test_cases = [
     {
         "id": "basic_1pkg_1repo",
@@ -204,41 +268,6 @@ depsolve_test_cases = [
             "reponames": {"custom"},
         },
     },
-    {
-        "id": "basic_2pkgs_2repos",
-        "enabled_repos": ["baseos", "custom"],
-        "transactions": [
-            {
-                "package-specs": [
-                    "filesystem",
-                    "pkg-with-no-deps"
-                ],
-            },
-        ],
-        "results": {
-            "packages": {
-                "basesystem",
-                "bash",
-                "centos-gpg-keys",
-                "centos-stream-release",
-                "centos-stream-repos",
-                "filesystem",
-                "glibc",
-                "glibc-common",
-                "glibc-minimal-langpack",
-                "libgcc",
-                "ncurses-base",
-                "ncurses-libs",
-                "setup",
-                "tzdata",
-                "pkg-with-no-deps",
-            },
-            "reponames": {
-                "baseos",
-                "custom",
-            },
-        },
-    },
     {
         "id": "basic_pkg_group",
         "enabled_repos": ["baseos", "custom"],
@@ -1088,7 +1117,7 @@ depsolve_test_cases = [
         "error_kind": "RepoError",
         "error_reason_re": r"There was a problem reading a repository: Failed to download metadata.*['\"]broken['\"].*",
     },
-]
+] + [depsolve_test_case_basic_2pkgs_2repos]
 
 
 dump_test_cases = [
@@ -1114,6 +1143,52 @@ dump_test_cases = [
 ]
 
 
+search_test_case_basic_2pkgs_2repos = {
+    "id": "basic_2pkgs_2repos",
+    "enabled_repos": ["baseos", "custom"],
+    "search_args": {
+        "latest": True,
+        "packages": [
+            "zsh",
+            "pkg-with-no-deps",
+        ],
+    },
+    "results": [
+        {
+            "name": "zsh",
+            "summary": "Powerful interactive shell",
+            "description": """The zsh shell is a command interpreter usable as an interactive login
+shell and as a shell script command processor.  Zsh resembles the ksh
+shell (the Korn shell), but includes many enhancements.  Zsh supports
+command line editing, built-in spelling correction, programmable
+command completion, shell functions (with autoloading), a history
+mechanism, and more.""",
+            "url": "http://zsh.sourceforge.net/",
+            "repo_id": "baseos",
+            "epoch": 0,
+            "version": "5.8",
+            "release": "9.el9",
+            "arch": "x86_64",
+            "buildtime": "2022-02-23T13:47:24Z",
+            "license": "MIT",
+        },
+        {
+            'arch': 'noarch',
+            'buildtime': '2024-04-15T18:09:19Z',
+            'description': 'Provides pkg-with-no-deps',
+            'epoch': 0,
+            'license': 'BSD',
+            'name': 'pkg-with-no-deps',
+            'release': '0',
+            'repo_id': 'custom',
+            'summary': 'Provides pkg-with-no-deps',
+            'url': None,
+            'version': '1.0.0',
+        },
+    ],
+}
+
+
 search_test_cases = [
     {
         "id": "1pkg_latest",
@@ -1213,7 +1288,7 @@ mechanism, and more.""",
         "error_kind": "RepoError",
         "error_reason_re": r"There was a problem reading a repository: Failed to download metadata.*['\"]broken['\"].*",
     },
-]
+] + [search_test_case_basic_2pkgs_2repos]
 
 
 def make_dnf_scafolding(base_dir):
@@ -1275,130 +1350,169 @@ def test_gen_config_combos(items_count, expected_combos):
     assert list(gen_config_combos(items_count)) == expected_combos
 
 
+def gen_repo_config(server):
+    """
+    Generate a repository configuration dictionary for the provided server.
+    """
+    return {
+        "id": server["name"],
+        "name": server["name"],
+        "baseurl": server["address"],
+        "check_gpg": False,
+        "sslverify": False,
+        "rhsm": False,
+        "gpgkeys": [TEST_KEY + server["name"]],
+    }
+
+
 def config_combos(tmp_path, servers):
     """
     Return all configurations for the provided repositories, either as config files in a directory or as repository
     configs in the depsolve request, or a combination of both.
     """
     for combo in gen_config_combos(len(servers)):
-        repo_configs = []
-        for idx in combo[0]:  # servers to be configured through request
-            server = servers[idx]
-            repo_configs.append({
-                "id": server["name"],
-                "name": server["name"],
-                "baseurl": server["address"],
-                "check_gpg": False,
-                "sslverify": False,
-                "rhsm": False,
-                "gpgkeys": [TEST_KEY + server["name"]],
-            })
-        root_dir, repos_dir, keys_dir = make_dnf_scafolding(tmp_path)
-        for idx in combo[1]:  # servers to be configured through root_dir
-            server = servers[idx]
-            name = server["name"]
-            # Use the gpgkey to test both the key reading and the variable substitution.
-            # For this test, it doesn't need to be a real key.
-            key_url = f"file:///etc/pki/rpm-gpg/RPM-GPG-KEY-$releasever-$basearch-$customvar-{name}"
-
-            key_path = keys_dir / f"RPM-GPG-KEY-{RELEASEVER}-{ARCH}-{CUSTOMVAR}-{name}"
-            key_path.write_text(TEST_KEY + name, encoding="utf8")
-            parser = configparser.ConfigParser()
-            parser.add_section(name)
-            # Set some options in a specific order in which they tend to be
-            # written in repo files.
-            parser.set(name, "name", name)
-            parser.set(name, "baseurl", server["address"])
-            parser.set(name, "enabled", "1")
-            parser.set(name, "gpgcheck", "1")
-            parser.set(name, "sslverify", "0")
-            parser.set(name, "gpgkey", key_url)
-
-            with (repos_dir / f"{name}.repo").open("w", encoding="utf-8") as fp:
-                parser.write(fp, space_around_delimiters=False)
+        repo_configs = None
+        if len(combo[0]):
+            repo_configs = []
+            for idx in combo[0]:  # servers to be configured through request
+                server = servers[idx]
+                repo_configs.append(gen_repo_config(server))
+
+        root_dir = None
+        if len(combo[1]):
+            root_dir, repos_dir, keys_dir = make_dnf_scafolding(tmp_path)
+            for idx in combo[1]:  # servers to be configured through root_dir
+                server = servers[idx]
+                name = server["name"]
+                # Use the gpgkey to test both the key reading and the variable substitution.
+                # For this test, it doesn't need to be a real key.
+                key_url = f"file:///etc/pki/rpm-gpg/RPM-GPG-KEY-$releasever-$basearch-$customvar-{name}"
+
+                key_path = keys_dir / f"RPM-GPG-KEY-{RELEASEVER}-{ARCH}-{CUSTOMVAR}-{name}"
+                key_path.write_text(TEST_KEY + name, encoding="utf8")
+                parser = configparser.ConfigParser()
+                parser.add_section(name)
+                # Set some options in a specific order in which they tend to be
+                # written in repo files.
+                parser.set(name, "name", name)
+                parser.set(name, "baseurl", server["address"])
+                parser.set(name, "enabled", "1")
+                parser.set(name, "gpgcheck", "1")
+                parser.set(name, "sslverify", "0")
+                parser.set(name, "gpgkey", key_url)
+
+                with (repos_dir / f"{name}.repo").open("w", encoding="utf-8") as fp:
+                    parser.write(fp, space_around_delimiters=False)
+            root_dir = os.fspath(root_dir)
 
         # for each combo, let's also enable or disable filelists (optional-metadata)
         for opt_metadata in ([], ["filelists"]):
-            yield repo_configs, os.fspath(root_dir), opt_metadata
+            yield repo_configs, root_dir, opt_metadata
+
+
+def get_test_case_repo_servers(test_case, repo_servers):
+    """
+    Return a list of repository servers for the test case.
+    """
+    repo_servers_copy = repo_servers.copy()
+    # filter to only include enabled repositories
+    repo_servers_copy = [r for r in repo_servers_copy if r["name"] in test_case["enabled_repos"]]
+    repo_servers_copy.extend(test_case.get("additional_servers", []))
+    return repo_servers_copy
+
+
+def get_test_case_repo_configs(test_case, repo_servers):
+    """
+    Return a list of repository configurations for the test case.
+    """
+    return [gen_repo_config(server) for server in get_test_case_repo_servers(test_case, repo_servers)]
+
+
+@pytest.mark.parametrize("test_case,repo_servers,expected", [
+    (
+        {"enabled_repos": ["baseos", "custom"], "additional_servers": []},
+        [{"name": "baseos", "address": "file:///baseos"}, {"name": "custom", "address": "file:///custom"}],
+        [{"name": "baseos", "address": "file:///baseos"}, {"name": "custom", "address": "file:///custom"}]
+    ),
+    (
+        {"enabled_repos": ["baseos"], "additional_servers": []},
+        [{"name": "baseos", "address": "file:///baseos"}, {"name": "custom", "address": "file:///custom"}],
+        [{"name": "baseos", "address": "file:///baseos"}]
+    ),
+    (
+        {
+            "enabled_repos": ["baseos", "custom"],
+            "additional_servers": [{"name": "broken", "address": "file:///broken"}]
+        },
+        [{"name": "baseos", "address": "file:///baseos"}, {"name": "custom", "address": "file:///custom"}],
+        [
+            {"name": "baseos", "address": "file:///baseos"},
+            {"name": "custom", "address": "file:///custom"},
+            {"name": "broken", "address": "file:///broken"},
+        ]
+    ),
+    (
+        {
+            "enabled_repos": ["baseos"], "additional_servers": [{"name": "broken", "address": "file:///broken"}]
+        },
+        [{"name": "baseos", "address": "file:///baseos"}, {"name": "custom", "address": "file:///custom"}],
+        [
+            {"name": "baseos", "address": "file:///baseos"},
+            {"name": "broken", "address": "file:///broken"},
+        ]
+    ),
+    (
+        {
+            "enabled_repos": [],
+            "additional_servers": [{"name": "broken", "address": "file:///broken"}]
+        },
+        [{"name": "baseos", "address": "file:///baseos"}, {"name": "custom", "address": "file:///custom"}],
+        [
+            {"name": "broken", "address": "file:///broken"},
+        ]
+    ),
+])
+def test_get_test_case_repo_servers(test_case, repo_servers, expected):
+    assert get_test_case_repo_servers(test_case, repo_servers) == expected
 
 
-# pylint: disable=too-many-branches
-@pytest.mark.parametrize("test_case", depsolve_test_cases, ids=tcase_idfn)
-@pytest.mark.parametrize("with_sbom", [False, True])
 @pytest.mark.parametrize("dnf_config, detect_fn", [
     ({}, assert_dnf),
     ({"use_dnf5": False}, assert_dnf),
     ({"use_dnf5": True}, assert_dnf5),
 ], ids=["no-config", "dnf4", "dnf5"])
-def test_depsolve(tmp_path, repo_servers, dnf_config, detect_fn, with_sbom, test_case):
+def test_depsolve_config_combos(tmp_path, repo_servers, dnf_config, detect_fn):
+    """
+    Test all possible configurations of repository configurations for the depsolve function.
+    Test on a single test case which installs two packages from two repositories.
+    """
     try:
         detect_fn()
     except RuntimeError as e:
         pytest.skip(str(e))
 
-    # pylint: disable=fixme
-    # TODO: remove this once dnf5 implementation is fixed
-    dnf5_broken_test_cases = [
-        "basic_pkg_group_with_excludes",
-        "install_pkg_excluded_in_another_transaction",
-        "error_pkg_not_in_enabled_repos",
-        "basic_module",
-    ]
-
-    if dnf_config.get("use_dnf5", False) and test_case["id"] in dnf5_broken_test_cases:
-        pytest.skip("This test case is known to be broken with dnf5")
-
+    test_case = depsolve_test_case_basic_2pkgs_2repos
     transactions = test_case["transactions"]
+    tc_repo_servers = get_test_case_repo_servers(test_case, repo_servers)
 
-    repo_servers_copy = repo_servers.copy()
-
-    # filter to only include enabled repositories
-    repo_servers_copy = [r for r in repo_servers_copy if r["name"] in test_case["enabled_repos"]]
-
-    if "additional_servers" in test_case:
-        repo_servers_copy.extend(test_case["additional_servers"])
-
-    for repo_configs, root_dir, opt_metadata in config_combos(tmp_path, repo_servers_copy):
+    for repo_configs, root_dir, opt_metadata in config_combos(tmp_path, tc_repo_servers):
         with TemporaryDirectory() as cache_dir:
-            res, exit_code = depsolve(transactions, repo_configs, root_dir,
-                                      cache_dir, dnf_config, opt_metadata, with_sbom)
-
-            if test_case.get("error", False):
-                assert exit_code != 0
-                assert res["kind"] == test_case["error_kind"]
-                assert re.match(test_case["error_reason_re"], res["reason"], re.DOTALL)
-                continue
+            res, exit_code = depsolve(
+                transactions, cache_dir, dnf_config, repo_configs, root_dir, opt_metadata)
 
             assert exit_code == 0
             assert {pkg["name"] for pkg in res["packages"]} == test_case["results"]["packages"]
             assert res["repos"].keys() == test_case["results"]["reponames"]
 
-            # modules is optional here as the dnf5 depsolver never returns any modules
-            assert res.get("modules", {}).keys() == test_case["results"].get("modules", set())
-
             for repo in res["repos"].values():
                 assert repo["gpgkeys"] == [TEST_KEY + repo["id"]]
                 assert repo["sslverify"] is False
-            if with_sbom:
-                assert "sbom" in res
-
-                spdx_2_3_1_schema_file = './test/data/spdx/spdx-schema-v2.3.1.json'
-                with open(spdx_2_3_1_schema_file, encoding="utf-8") as f:
-                    spdx_schema = json.load(f)
-                validator = jsonschema.Draft4Validator
-                validator.check_schema(spdx_schema)
-                spdx_validator = validator(spdx_schema)
-                spdx_validator.validate(res["sbom"])
-
-                assert {pkg["name"] for pkg in res["sbom"]["packages"]} == test_case["results"]["packages"]
-            else:
-                assert "sbom" not in res
 
             # if opt_metadata includes 'filelists', then each repository 'repodata' must include a file that matches
             # *filelists*
             n_filelist_files = len(glob(f"{cache_dir}/*/repodata/*filelists*"))
             if "filelists" in opt_metadata:
-                assert n_filelist_files == len(repo_servers_copy)
+                assert n_filelist_files == len(tc_repo_servers)
             else:
                 assert n_filelist_files == 0
 
@@ -1409,11 +1523,112 @@ def test_depsolve(tmp_path, repo_servers, dnf_config, detect_fn, with_sbom, test
                 assert res["solver"] == "dnf"
 
 
+@pytest.mark.parametrize("with_sbom", [False, True])
+@pytest.mark.parametrize("dnf_config, detect_fn", [
+    ({}, assert_dnf),
+    ({"use_dnf5": False}, assert_dnf),
+    ({"use_dnf5": True}, assert_dnf5),
+], ids=["no-config", "dnf4", "dnf5"])
+def test_depsolve_sbom(tmp_path, repo_servers, dnf_config, detect_fn, with_sbom):
+    try:
+        detect_fn()
+    except RuntimeError as e:
+        pytest.skip(str(e))
+
+    test_case = depsolve_test_case_basic_2pkgs_2repos
+    transactions = test_case["transactions"]
+    repo_configs = get_test_case_repo_configs(test_case, repo_servers)
+
+    res, exit_code = depsolve(transactions, tmp_path.as_posix(), dnf_config, repo_configs, with_sbom=with_sbom)
+
+    assert exit_code == 0
+    assert {pkg["name"] for pkg in res["packages"]} == test_case["results"]["packages"]
+    assert res["repos"].keys() == test_case["results"]["reponames"]
+
+    for repo in res["repos"].values():
+        assert repo["gpgkeys"] == [TEST_KEY + repo["id"]]
+        assert repo["sslverify"] is False
+    if with_sbom:
+        assert "sbom" in res
+
+        spdx_2_3_1_schema_file = './test/data/spdx/spdx-schema-v2.3.1.json'
+        with open(spdx_2_3_1_schema_file, encoding="utf-8") as f:
+            spdx_schema = json.load(f)
+        validator = jsonschema.Draft4Validator
+        validator.check_schema(spdx_schema)
+        spdx_validator = validator(spdx_schema)
+        spdx_validator.validate(res["sbom"])
+
+        assert {pkg["name"] for pkg in res["sbom"]["packages"]} == test_case["results"]["packages"]
+    else:
+        assert "sbom" not in res
+
+    use_dnf5 = dnf_config.get("use_dnf5", False)
+    if use_dnf5:
+        assert res["solver"] == "dnf5"
+    else:
+        assert res["solver"] == "dnf"
+
+
+# pylint: disable=too-many-branches
+@pytest.mark.parametrize("test_case", depsolve_test_cases, ids=tcase_idfn)
+@pytest.mark.parametrize("dnf_config, detect_fn", [
+    ({}, assert_dnf),
+    ({"use_dnf5": False}, assert_dnf),
+    ({"use_dnf5": True}, assert_dnf5),
+], ids=["no-config", "dnf4", "dnf5"])
+def test_depsolve(tmp_path, repo_servers, dnf_config, detect_fn, test_case):
+    try:
+        detect_fn()
+    except RuntimeError as e:
+        pytest.skip(str(e))
+
+    # pylint: disable=fixme
+    # TODO: remove this once dnf5 implementation is fixed
+    dnf5_broken_test_cases = [
+        "basic_pkg_group_with_excludes",
+        "install_pkg_excluded_in_another_transaction",
+        "error_pkg_not_in_enabled_repos",
+        "basic_module",
+    ]
+
+    if dnf_config.get("use_dnf5", False) and test_case["id"] in dnf5_broken_test_cases:
+        pytest.skip("This test case is known to be broken with dnf5")
+
+    transactions = test_case["transactions"]
+    repo_configs = get_test_case_repo_configs(test_case, repo_servers)
+
+    res, exit_code = depsolve(transactions, tmp_path.as_posix(), dnf_config, repo_configs)
+
+    if test_case.get("error", False):
+        assert exit_code != 0
+        assert res["kind"] == test_case["error_kind"]
+        assert re.match(test_case["error_reason_re"], res["reason"], re.DOTALL)
+        return
+
+    assert exit_code == 0
+    assert {pkg["name"] for pkg in res["packages"]} == test_case["results"]["packages"]
+    assert res["repos"].keys() == test_case["results"]["reponames"]
+
+    # modules is optional here as the dnf5 depsolver never returns any modules
+    assert res.get("modules", {}).keys() == test_case["results"].get("modules", set())
+
+    for repo in res["repos"].values():
+        assert repo["gpgkeys"] == [TEST_KEY + repo["id"]]
+        assert repo["sslverify"] is False
+
+    use_dnf5 = dnf_config.get("use_dnf5", False)
+    if use_dnf5:
+        assert res["solver"] == "dnf5"
+    else:
+        assert res["solver"] == "dnf"
+
+
 @pytest.mark.parametrize("test_case", dump_test_cases, ids=tcase_idfn)
 @pytest.mark.parametrize("dnf_config, detect_fn", [
     (None, assert_dnf),
-    ('{"use_dnf5": false}', assert_dnf),
-    ('{"use_dnf5": true}', assert_dnf5),
+    ({"use_dnf5": False}, assert_dnf),
+    ({"use_dnf5": True}, assert_dnf5),
 ], ids=["no-config", "dnf4", "dnf5"])
 def test_dump(tmp_path, repo_servers, dnf_config, detect_fn, test_case):
     try:
@@ -1421,17 +1636,11 @@ def test_dump(tmp_path, repo_servers, dnf_config, detect_fn, test_case):
     except RuntimeError as e:
         pytest.skip(str(e))
 
-    repo_servers_copy = repo_servers.copy()
-
-    # filter to only include enabled repositories
-    repo_servers_copy = [r for r in repo_servers_copy if r["name"] in test_case["enabled_repos"]]
-
-    if "additional_servers" in test_case:
-        repo_servers_copy.extend(test_case["additional_servers"])
+    tc_repo_servers = get_test_case_repo_servers(test_case, repo_servers)
 
-    for repo_configs, root_dir, opt_metadata in config_combos(tmp_path, repo_servers_copy):
+    for repo_configs, root_dir, opt_metadata in config_combos(tmp_path, tc_repo_servers):
         with TemporaryDirectory() as cache_dir:
-            res, exit_code = dump(repo_configs, root_dir, cache_dir, dnf_config, opt_metadata)
+            res, exit_code = dump(cache_dir, dnf_config, repo_configs, root_dir, opt_metadata)
 
             if test_case.get("error", False):
                 assert exit_code != 0
@@ -1455,67 +1664,102 @@ def test_dump(tmp_path, repo_servers, dnf_config, detect_fn, test_case):
             # *filelists*
             n_filelist_files = len(glob(f"{cache_dir}/*/repodata/*filelists*"))
             if "filelists" in opt_metadata:
-                assert n_filelist_files == len(repo_servers_copy)
+                assert n_filelist_files == len(tc_repo_servers)
             else:
                 assert n_filelist_files == 0
 
 
-@pytest.mark.parametrize("test_case", search_test_cases, ids=tcase_idfn)
 @pytest.mark.parametrize("dnf_config, detect_fn", [
     (None, assert_dnf),
-    ('{"use_dnf5": false}', assert_dnf),
-    ('{"use_dnf5": true}', assert_dnf5),
+    ({"use_dnf5": False}, assert_dnf),
+    ({"use_dnf5": True}, assert_dnf5),
 ], ids=["no-config", "dnf4", "dnf5"])
-def test_search(tmp_path, repo_servers, dnf_config, detect_fn, test_case):
+def test_search_config_combos(tmp_path, repo_servers, dnf_config, detect_fn):
+    """
+    Test all possible configurations of repository configurations for the search function.
+    Test on a single test case which searches for two packages from two repositories.
+    """
     try:
         detect_fn()
     except RuntimeError as e:
         pytest.skip(str(e))
 
-    repo_servers_copy = repo_servers.copy()
-
-    # filter to only include enabled repositories
-    repo_servers_copy = [r for r in repo_servers_copy if r["name"] in test_case["enabled_repos"]]
-
-    if "additional_servers" in test_case:
-        repo_servers_copy.extend(test_case["additional_servers"])
-
+    test_case = search_test_case_basic_2pkgs_2repos
+    tc_repo_servers = get_test_case_repo_servers(test_case, repo_servers)
     search_args = test_case["search_args"]
 
-    for repo_configs, root_dir, opt_metadata in config_combos(tmp_path, repo_servers_copy):
+    for repo_configs, root_dir, opt_metadata in config_combos(tmp_path, tc_repo_servers):
         with TemporaryDirectory() as cache_dir:
-            res, exit_code = search(search_args, repo_configs, root_dir, cache_dir, dnf_config, opt_metadata)
-
-            if test_case.get("error", False):
-                assert exit_code != 0
-                assert res["kind"] == test_case["error_kind"]
-                assert re.match(test_case["error_reason_re"], res["reason"], re.DOTALL)
-                continue
+            res, exit_code = search(search_args, cache_dir, dnf_config, repo_configs, root_dir, opt_metadata)
 
             assert exit_code == 0
-            assert res == test_case["results"]
+            for res, exp in zip(res, test_case["results"]):
+                # if the url in the package is empty, DNF4 returns None, DNF5 returns an empty string
+                exp = exp.copy()
+                exp_url = exp.pop("url")
+                res_url = res.pop("url")
+                if exp_url is None and dnf_config and dnf_config.get("use_dnf5", False):
+                    assert res_url == ""
+                else:
+                    assert res_url == exp_url
+                assert res == exp
 
             # if opt_metadata includes 'filelists', then each repository 'repodata' must include a file that matches
             # *filelists*
             n_filelist_files = len(glob(f"{cache_dir}/*/repodata/*filelists*"))
             if "filelists" in opt_metadata:
-                assert n_filelist_files == len(repo_servers_copy)
+                assert n_filelist_files == len(tc_repo_servers)
             else:
                 assert n_filelist_files == 0
 
 
+@pytest.mark.parametrize("test_case", search_test_cases, ids=tcase_idfn)
 @pytest.mark.parametrize("dnf_config, detect_fn", [
     (None, assert_dnf),
-    ('{"use_dnf5": false}', assert_dnf),
-    ('{"use_dnf5": true}', assert_dnf5),
+    ({"use_dnf5": False}, assert_dnf),
+    ({"use_dnf5": True}, assert_dnf5),
 ], ids=["no-config", "dnf4", "dnf5"])
-def test_depsolve_result_api(tmp_path, repo_servers, dnf_config, detect_fn):
+def test_search(tmp_path, repo_servers, dnf_config, detect_fn, test_case):
     try:
         detect_fn()
     except RuntimeError as e:
         pytest.skip(str(e))
 
-    root_dir = tmp_path.as_posix()
+    repo_configs = get_test_case_repo_configs(test_case, repo_servers)
+    search_args = test_case["search_args"]
+
+    res, exit_code = search(search_args, tmp_path.as_posix(), dnf_config, repo_configs)
+
+    if test_case.get("error", False):
+        assert exit_code != 0
+        assert res["kind"] == test_case["error_kind"]
+        assert re.match(test_case["error_reason_re"], res["reason"], re.DOTALL)
+        return
+
+    assert exit_code == 0
+    for res, exp in zip(res, test_case["results"]):
+        # if the url in the package is empty, DNF4 returns None, DNF5 returns an empty string
+        exp = exp.copy()
+        exp_url = exp.pop("url")
+        res_url = res.pop("url")
+        if exp_url is None and dnf_config and dnf_config.get("use_dnf5", False):
+            assert res_url == ""
+        else:
+            assert res_url == exp_url
+        assert res == exp
+
+
+def test_depsolve_result_api(tmp_path, repo_servers):
+    """
+    Test the result of depsolve() API.
+
+    Note tha this test runs only with dnf4, as the dnf5 depsolver does not support modules.
+    """
+    try:
+        assert_dnf()
+    except RuntimeError as e:
+        pytest.skip(str(e))
+
     cache_dir = (tmp_path / "depsolve-cache").as_posix()
     transactions = [
         {
@@ -1524,20 +1768,8 @@ def test_depsolve_result_api(tmp_path, repo_servers, dnf_config, detect_fn):
         },
     ]
 
-    repo_configs = []
-    for server in repo_servers:
-        repo_configs.append({
-            "id": server["name"],
-            "name": server["name"],
-            "baseurl": server["address"],
-            "gpgkeys": [TEST_KEY + server["name"]],
-        })
-    dnf_config = {}
-    opt_metadata = []
-    with_sbom = False
-
-    res, exit_code = depsolve(transactions, repo_configs, root_dir,
-                              cache_dir, dnf_config, opt_metadata, with_sbom)
+    repo_configs = [gen_repo_config(server) for server in repo_servers]
+    res, exit_code = depsolve(transactions, cache_dir, repos=repo_configs)
 
     assert exit_code == 0
     # If any of  this changes, increase: