#1607 Add test_reuse_components_if_added
Closed 3 years ago by jobrauer. Opened 4 years ago by jobrauer.
jobrauer/fm-orchestrator test_reuse_components_if_added  into  master

@@ -13,6 +13,10 @@ 

  # Items in here are mapped by their name to the tests that use them.

  # For example test_scratch_build will use scratch_build.

  testdata:

+   # 'dummy' RPMs to be used by tests (modulemd format)

+   rpms:

+     - {attr: {rationale: "...", ref: "rhel-8.0", buildorder: 1}}

+     # ...

    scratch_build:

      # MBS build id to be reused for this test.

      # When specified no new build is started for this test,
@@ -96,3 +100,21 @@ 

      # with koji_tag_with_modules from testmodule's platform (stream 8.1.0.z)

      module: testmodule

      branch: test-reuse-tagged-module

+   reuse_components_if_added:

+     # Test expects no RPM components in modulemd

+     module: testmodule

+     branch: test-reuse-components-if-added

+     scenarios:

+       # indexes of RPMs in ['testdata'] section of this file

+       - {first_build: [0, 2], second_build: [0, 1, 2], rebuild_strategy: "only-changed",

+          expected_reused: [0, 2], expected_rebuilt: [1]}

+       # ...

+   reuse_components_if_removed:

+     # Test expects no RPM components in modulemd

+     module: testmodule

+     branch: test-reuse-components-if-removed

+     scenarios:

+       # indexes of RPMs in ['testdata'] section of this file

+       - {first_build: [0, 1], second_build: [0], expected_reused: [0], expected_rebuilt: [],

+          rebuild_strategy: "only-changed"}

+       # ...

@@ -0,0 +1,112 @@ 

+ import copy

+ import pytest

+ 

+ 

+ def extracted_test_method(repo, pkg_util,

+                           rebuild_strategy,

+                           first_build_rpms, second_build_rpms,

+                           expected_reused, expected_rebuilt):

+     """

+         :param utils.Repo repo: repo fixture

+         :param utils.PackagingUtility pkg_util: pkg_util fixture

+         :param str rebuild_strategy: rebuild strategy for the 2nd build

+         :param dict first_build_rpms: dictionary of RPMs (modulemd format)

+         :param dict second_build_rpms: dictionary of RPMs (modulemd format)

+         :param dict expected_reused: dictionary of RPMs (modulemd format)

+         :param dict expected_rebuilt: dictionary of RPMs (modulemd format)

+ 

+         Prerequisites:

+             EMPTY (no components/RPMs) module MD file

+         Steps:

+             1) Make changes to the modulemd according to test scenario (first build)

+             2) Make a clean build (rebuild strategy : all).

+             3) Add/remove RPMs according to test scenario (second build)

+             4) Rebuild, rebuild strategy according to test scenario

+             5) Revert the initial change.

+         """

+     # Save initial state

+     original_metadata = repo.modulemd

+ 

+     # Prepare initial build metadata & push

+     tmp_metadata = copy.deepcopy(original_metadata)

+     tmp_metadata["data"]["components"]["rpms"] = first_build_rpms

+     repo.write_to_modulemd(tmp_metadata)

+     repo.add_all_commit_and_push(f'Add "{first_build_rpms.keys()}"')

+ 

+     try:

+         # Make an initial build (to be later reused)

+         builds = pkg_util.run("--watch", "--optional", "rebuild_strategy=all")

+         assert len(builds) == 1, "Initial build failed!"

+ 

+         # Prepare 2nd build metadata & push

+         tmp_metadata["data"]["components"]["rpms"] = second_build_rpms

+         repo.write_to_modulemd(tmp_metadata)

+         repo.add_all_commit_and_push(f'Add "{second_build_rpms.keys()}"')

+ 

+         # Make a new build

+         builds = pkg_util.run("--watch", "--optional", f"rebuild_strategy={rebuild_strategy}")

+         assert len(builds) == 1, "Second (re)build failed!"

+ 

+         build = builds[0]

+         # we don"t care about module-build-macros

+         build_components = [c for c in build.components() if c["package"] != "module-build-macros"]

+ 

+         # Partition components by "reused" state - package name only

+         reused_msg = "Reused component from previous module build"

+         actually_reused = {

+             c["package"] for c in build_components if c["state_reason"] == reused_msg

+         }

+         actually_rebuilt = {

+             c["package"] for c in build_components if c["state_reason"] != reused_msg

+         }

+ 

+         assert actually_reused == expected_reused.keys()

+         assert actually_rebuilt == expected_rebuilt.keys()

+ 

+     finally:  # Revert the change

+         repo.write_to_modulemd(original_metadata)

+         repo.add_all_commit_and_push(f"Revert to empty md file")

+ 

+ 

+ # Same test logic is split into 2 methods (different repositories) to enable parallel execution.

+ # In case of more scenarios and longer execution times, split into more functions.

+ def test_reuse_components_if_added(pkg_util, test_env, scenario, repo):

+     """

+     Test case wrapper for parametrization based on test.env.yaml

+     """

+     test_rpms = test_env.get("testdata").get("rpms")

+     if not test_rpms:

+         pytest.skip("No RPMs in test.env.yaml: ['testdata']['rpms']")

+     if not scenario.get('scenarios'):

+         pytest.skip("No scenarios for test branch '{}'".format(scenario["branch"]))

+ 

+     for index, test_scenario in enumerate(scenario['scenarios']):

+         try:

+             first_build_rpms = [test_rpms[i] for i in test_scenario["first_build"]]

+             first_build_rpms = {k: v for rpm in first_build_rpms for k, v in rpm.items()}

+ 

+             second_build_rpms = [test_rpms[i] for i in test_scenario["second_build"]]

+             second_build_rpms = {k: v for rpm in second_build_rpms for k, v in rpm.items()}

+ 

+             expected_reused = [test_rpms[i] for i in test_scenario["expected_reused"]]

+             expected_reused = {k: v for rpm in expected_reused for k, v in rpm.items()}

+ 

+             expected_rebuilt = [test_rpms[i] for i in test_scenario["expected_rebuilt"]]

+             expected_rebuilt = {k: v for rpm in expected_rebuilt for k, v in rpm.items()}

+ 

+             rebuild_strategy = test_scenario["rebuild_strategy"]

+         except IndexError:

+             pytest.skip(f"Index error, see test.env.yaml '{scenario['branch']}', scenario {index}")

+         else:

+             # TC itself

+             extracted_test_method(repo, pkg_util, rebuild_strategy,

+                                   first_build_rpms, second_build_rpms,

+                                   expected_reused, expected_rebuilt)

+ 

+ 

+ def test_reuse_components_if_removed(pkg_util, test_env, scenario, repo):

+     """

+     Test case wrapper for parametrization based on test.env.yaml

+     """

+ 

+     test_reuse_components_if_added(pkg_util, test_env, scenario, repo)

file modified
+31 -1
@@ -119,6 +119,11 @@ 

          self._version = None

  

      @property

+     def modulemd_file(self):

+         """Modulemd file absolute path"""

+         return os.path.join(os.path.abspath(os.curdir), self.module_name + ".yaml")

+ 

+     @property

      def modulemd(self):

          """Modulemd file as read from the repo

  
@@ -155,7 +160,24 @@ 

          elif self._version == 2:

              return self._modulemd["data"]["dependencies"][0]["buildrequires"].get("platform")

  

-     def bump(self):

+     def write_to_modulemd(self, content_yaml):

+         """

+         Write new content (replace) to the modulemd file

+ 

+         :param dict content_yaml: module metadata dictionary

+         """

+         with open(self.modulemd_file, 'w') as md_file:

+             yaml.dump(content_yaml, md_file, sort_keys=False)

+ 

+     @staticmethod

+     def add_all_commit_and_push(message="Bump"):

+         """Add all current changes and commit with a custom message"""

+         git("add", "--all")

+         git("commit", "-m", message)

+         git("push")

+ 

+     @staticmethod

+     def bump():

          """Create a "bump" commit"""

          args = [

              "--allow-empty",
@@ -230,6 +252,14 @@ 

              "utf-8")

          return stdout

  

+     def get_build(self, build_id):

+         """

+         :param int build_id: build integer ID

+         :return: a module build object

+         :rtype: Build

+         """

+         return Build(self._mbs_api, build_id)

+ 

  

  class Build:

      """Wrapper class to work with module builds

FACTORY-6005: add test_reuse_components_if_added

This is testing: https://pagure.io/fm-orchestrator/pull-request/1475#

@mikem @breilly
Hi, I added a new TC with 3 variations. However the last 'scenario':

"having 2 rpms previously built (batch 1, 2), adding a third one (batch 3) and rebuilding with changed-and-after"

still rebuilds the the 2nd rpm (batch 2) - the 1st one gets reused as expected. There are no log entries between 'Starting build of next batch X' and 'submitted build of' so I got a hard time following the code without the inner workings knowledge...

Am I doing something wrong? Please have a look at builds:
1) strategy 'all': https://brewweb.stage.engineering.redhat.com/brew/buildinfo?buildID=1806425
2) immediately after - strategy 'changed-and-after': https://brewweb.stage.engineering.redhat.com/brew/buildinfo?buildID=1806435

Thanks a lot.

Why are you using type hinting? It is not used elsewhere in the code base.

I would say: do not use type hinting unless we set guidelines for using it for this project.

Ah, I guess you've done this in order to use the dataclasses module. I'm not sure the benefit of using this module, which is used nowhere else in the code.

rebased onto ed5cc85e840e1dbc85909db1af1473eca9debdc5

4 years ago

@mikem thanks for your comments, I just thought having an object was more comfortable than using plain dictionaries. But you're right, it's unnecessary - removed. Type hinting as well - I will try to adhere more to the projects current style.

pretty please pagure-ci rebuild

4 years ago

1 new commit added

  • Move test scenarios to test.env.yaml
4 years ago

pretty please pagure-ci rebuild

4 years ago

pretty please pagure-ci rebuild

4 years ago

1 new commit added

  • Small cleanup
4 years ago

pretty please pagure-ci rebuild

4 years ago

pretty please pagure-ci rebuild

4 years ago

pretty please pagure-ci rebuild

4 years ago

rebased onto c81cd223b2b25f55aa90c517d6c061cfdb91a6ad

4 years ago

rebased onto 7f849c1

3 years ago

@mikem overlooked import removed. I can't see any more type hinting. I hope it is alright now. If yes, can we merge?

In any case, my question from above is still unanswered, I'm not sure whether to include this scenario.

"""
having 2 rpms previously built (batch 1, 2), adding a third one (batch 3) and rebuilding with changed-and-after"

still rebuilds the the 2nd rpm (batch 2) - the 1st one gets reused as expected. There are no log entries between 'Starting build of next batch X' and 'submitted build of' so I got a hard time following the code without the inner workings knowledge...

Am I doing something wrong? Please have a look at builds:
1) strategy 'all': https://brewweb.stage.engineering.redhat.com/brew/buildinfo?buildID=1806425
2) immediately after - strategy 'changed-and-after': https://brewweb.stage.engineering.redhat.com/brew/buildinfo?buildID=1806435
"""

Pull-Request has been closed by jobrauer

3 years ago