#4 Additional changes to support buildrequires in commands
Merged 4 months ago by cqi. Opened 4 months ago by cqi.
cqi/ursa-major remove-module  into  master

file modified
+75 -51

@@ -33,6 +33,58 @@ 

  gi.require_version('Modulemd', '1.0')  # noqa

  from gi.repository import Modulemd

  

+ TESTS_DIR = os.path.dirname(os.path.realpath(__file__))

+ TEST_DATA_DIR = os.path.join(TESTS_DIR, 'test_data')

+ 

+ 

+ def make_mmd(name, stream, version, context, requires=None, buildrequires=None):

+     """Creates new Modulemd.Module instance

+ 

+     :param name: module name

+     :param stream: module stream

+     :param version: module version

+     :param context: module context

+     :param requires: Dict of requires, example:

+         {'platform': ['rhel-8.0'], 'python3': 'master'}

+     :param buildrequires: Dict of build_requires, example:

+         {'platform': 'rhel-8.0', 'bootstrap': ['rhel-8.0']}

+     :rtype: Modulemd.Module instance

+     """

+     mmd = Modulemd.Module()

+     mmd.set_name(name)

+     mmd.set_stream(stream)

+     mmd.set_version(int(version))

+     mmd.set_context(context)

+     # required options

+     mmd.set_mdversion(2)

+     mmd.set_summary("A test module in all its beautiful beauty.")

+     description = ("This module demonstrates how to write simple "

+                    "modulemd files And can be used for testing "

+                    "the build and release pipeline.")

+     mmd.set_description(description)

+     licenses = Modulemd.SimpleSet()

+     licenses.add("GPL")

+     mmd.set_module_licenses(licenses)

+ 

+     deps_list = []

+     requires = requires or {}

+     buildrequires = buildrequires or {}

+ 

+     deps = Modulemd.Dependencies()

+     for req_name, req_streams in requires.items():

+         if not isinstance(req_streams, list):

+             req_streams = [req_streams]

+         deps.add_requires(req_name, req_streams)

+ 

+     for req_name, req_streams in buildrequires.items():

+         if not isinstance(req_streams, list):

+             req_streams = [req_streams]

+         deps.add_buildrequires(req_name, req_streams)

+     deps_list.append(deps)

+     mmd.set_dependencies(deps_list)

+ 

+     return mmd

+ 

  

  class MockResponse:

      def __init__(self, json_data, status_code):

@@ -45,57 +97,29 @@ 

  

  class UrsaMajorTestCase(unittest.TestCase):

      def load_json_from_file(self, filename):

-         basedir = os.path.dirname(os.path.realpath(__file__))

-         test_data_dir = os.path.join(basedir, 'test_data')

-         test_data_file = os.path.join(test_data_dir, filename)

+         test_data_file = os.path.join(TEST_DATA_DIR, filename)

          with open(test_data_file, 'r') as f:

              return json.load(f)

  

-     def make_mmd(self, name, stream, version, context,

-                  requires=None, buildrequires=None):

-         """Creates new Modulemd.Module instance

- 

-         :param name: module name

-         :param stream: module stream

-         :param version: module version

-         :param context: module context

-         :param requires: Dict of requires, example:

-             {'platform': ['rhel-8.0'], 'python3': 'master'}

-         :param buildrequires: Dict of build_requires, example:

-             {'platform': 'rhel-8.0', 'bootstrap': ['rhel-8.0']}

-         :rtype: Modulemd.Module instance

-         """

-         mmd = Modulemd.Module()

-         mmd.set_name(name)

-         mmd.set_stream(stream)

-         mmd.set_version(int(version))

-         mmd.set_context(context)

-         # required options

-         mmd.set_mdversion(2)

-         mmd.set_summary("A test module in all its beautiful beauty.")

-         description = ("This module demonstrates how to write simple "

-                        "modulemd files And can be used for testing "

-                        "the build and release pipeline.")

-         mmd.set_description(description)

-         licenses = Modulemd.SimpleSet()

-         licenses.add("GPL")

-         mmd.set_module_licenses(licenses)

- 

-         deps_list = []

-         requires = requires or {}

-         buildrequires = buildrequires or {}

- 

-         deps = Modulemd.Dependencies()

-         for req_name, req_streams in requires.items():

-             if not isinstance(req_streams, list):

-                 req_streams = [req_streams]

-             deps.add_requires(req_name, req_streams)

- 

-         for req_name, req_streams in buildrequires.items():

-             if not isinstance(req_streams, list):

-                 req_streams = [req_streams]

-             deps.add_buildrequires(req_name, req_streams)

-         deps_list.append(deps)

-         mmd.set_dependencies(deps_list)

- 

-         return mmd

+ 

+ def make_mbs_response(module_builds):

+     """Helper function to make fake MBS response JSON

+ 

+     When calling this method, developer only needs to pass a list of module

+     builds. Finally, a full response JSON data with only one page data set is

+     returned.

+     """

+     return {

+         'items': module_builds,

+         'meta': {

+             'prev': None,

+             'last': None,

+             'next': None,

+             'total': 1,

+             'per_page': 10,

+             'first': 'https://mbs.example.com/module-build-service/1/module-builds/'

+                      '?per_page=10&page=1',

+             'pages': 1,

+             'page': 1

+         }

+     }

@@ -0,0 +1,492 @@ 

+ # -*- coding: utf-8 -*-

+ # Copyright (c) 2019  Red Hat, Inc.

+ #

+ # Permission is hereby granted, free of charge, to any person obtaining a copy

+ # of this software and associated documentation files (the "Software"), to deal

+ # in the Software without restriction, including without limitation the rights

+ # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell

+ # copies of the Software, and to permit persons to whom the Software is

+ # furnished to do so, subject to the following conditions:

+ #

+ # The above copyright notice and this permission notice shall be included in all

+ # copies or substantial portions of the Software.

+ #

+ # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR

+ # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,

+ # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE

+ # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER

+ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,

+ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE

+ # SOFTWARE.

+ #

+ # Written by Chenxiong Qi <cqi@redhat.com>

+ 

+ import json

+ import os

+ import pytest

+ import tempfile

+ 

+ from mock import patch, Mock

+ from six.moves.configparser import ConfigParser

+ from tests import TEST_DATA_DIR, make_mbs_response, make_mmd

+ from ursa_major.cli import main

+ from ursa_major.handlers.add_module import AddModuleHandler, ModuleConfig

+ 

+ CONFIG_FILE = os.path.join(TEST_DATA_DIR, 'ursa-major-test.conf')

+ URSA_MAJOR_TEST_CONFIG = ConfigParser()

+ URSA_MAJOR_TEST_CONFIG.read(CONFIG_FILE)

+ 

+ 

+ def get_tag_side_effect(info):

+     return {

+         'f30-build': {'id': 99, 'name': 'f30-build'},

+         'module-mariadb-10.4-3020190313091759-a5b0195c': {

+             'id': 100,

+             'name': 'module-mariadb-10.4-3020190313091759-a5b0195c',

+         },

+         'module-mariadb-10.4-3020190304180835-a5b0195c': {

+             'id': 101,

+             'name': 'module-mariadb-10.4-3020190304180835-a5b0195c',

+         },

+     }[info]

+ 

+ 

+ class TestAddModule:

+ 

+     def setup_method(self):

+         fd, self.tag_config_file = tempfile.mkstemp()

+         os.close(fd)

+ 

+         self.ClientSession_patcher = patch('koji.ClientSession')

+         self.mock_ClientSession = self.ClientSession_patcher.start()

+         self.koji_session = self.mock_ClientSession.return_value

+ 

+         self.get_patcher = patch('requests.get')

+         self.mock_get = self.get_patcher.start()

+ 

+     def teardown_method(self):

+         self.get_patcher.stop()

+         self.ClientSession_patcher.stop()

+ 

+         os.unlink(self.tag_config_file)

+ 

+     def write_tag_config_file(self, content):

+         with open(self.tag_config_file, 'w') as f:

+             json.dump(content, f)

+ 

+     def read_tag_config_file(self):

+         with open(self.tag_config_file, 'r') as f:

+             return json.load(f)

+ 

+     def add_module(self,

+                    command_options=None,

+                    tag_configs_content=None,

+                    update_inheritance_only=None,

+                    update_tag_config_only=None,

+                    mock_inheritance_data=None,

+                    mock_mbs_modules=None):

+         """

+         Run add-module command for test. Behavior could be changed by arguments

+         for specific tests.

+         """

+         self.koji_session.getInheritanceData.return_value = mock_inheritance_data or [

+             {'parent_id': 20,

+              'name': 'module-ant-1.10-20181122140939-819b5873',

+              'priority': 10},

+         ]

+         self.koji_session.getTag.side_effect = get_tag_side_effect

+ 

+         self.mock_get.return_value = Mock(status_code=200)

+         self.mock_get.return_value.json.return_value = make_mbs_response(mock_mbs_modules or [

+             {

+                 'koji_tag': 'module-mariadb-10.4-3020190313091759-a5b0195c',

+                 'modulemd': make_mmd(name='mariadb', stream='10.4',

+                                      version='3020190313091759', context='a5b0195c',

+                                      requires={'platform': 'f30'},

+                                      buildrequires={'platform': 'f30'}).dumps(),

+             },

+             {

+                 'koji_tag': 'module-mariadb-10.4-3020190304180835-a5b0195c',

+                 'modulemd': make_mmd(name='mariadb', stream='10.4',

+                                      version='3020190304180835', context='a5b0195c',

+                                      requires={'platform': 'f30'},

+                                      buildrequires={'platform': 'f30'}).dumps(),

+             },

+         ])

+ 

+         # For this test, we write an empty tag config file.

+         self.write_tag_config_file(tag_configs_content or {})

+ 

+         cli_cmd_options = command_options or [

+             '--name', 'mariadb', '--stream', '10.4', '--priority', '50',

+             '--require', 'platform:f30',

+             '--buildrequire', 'platform:f30',

+             '--tag', 'f30-build',

+             '--tag-config-file', self.tag_config_file

+         ]

+         if update_inheritance_only:

+             cli_cmd_options.append('--update-inheritance-only')

+         if update_tag_config_only:

+             cli_cmd_options.append('--update-config-only')

+ 

+         config_file = os.path.join(TEST_DATA_DIR, 'ursa-major-test.conf')

+         cli_cmd = ['ursa-major', 'add-module', '--config', config_file] + cli_cmd_options

+ 

+         with patch('sys.argv', new=cli_cmd):

+             main()

+ 

+     def test_common_add_module(self):

+         """Test add a module to tag config file and its koji_tag to tag inheritance"""

+         self.add_module()

+ 

+         self.koji_session.setInheritanceData.assert_called_once_with(99, [

+             {

+                 'parent_id': 20,

+                 'name': 'module-ant-1.10-20181122140939-819b5873',

+                 'priority': 10

+             },

+             {

+                 'parent_id': 100,

+                 'priority': 50,

+                 'maxdepth': None,

+                 'intransitive': False,

+                 'noconfig': False,

+                 'pkg_filter': ''

+             },

+         ])

+ 

+         expected = [{

+             'name': 'mariadb',

+             'stream': '10.4',

+             'requires': {'platform': 'f30'},

+             'buildrequires': {'platform': 'f30'},

+             'priority': 50

+         }]

+         tag_configs = self.read_tag_config_file()

+         assert expected == tag_configs['f30-build']['modules']

+ 

+     def test_add_module_koji_tag_only(self):

+         self.add_module(tag_configs_content={},

+                         update_inheritance_only=True)

+ 

+         self.koji_session.setInheritanceData.assert_called_once_with(99, [

+             {

+                 'parent_id': 20,

+                 'name': 'module-ant-1.10-20181122140939-819b5873',

+                 'priority': 10

+             },

+             {

+                 'parent_id': 100,

+                 'priority': 50,

+                 'maxdepth': None,

+                 'intransitive': False,

+                 'noconfig': False,

+                 'pkg_filter': ''

+             },

+         ])

+ 

+         tag_configs = self.read_tag_config_file()

+         assert {} == tag_configs, 'Tag config file should keep unchanged.'

+ 

+     def test_add_module_but_only_update_tag_config_file(self):

+         self.add_module(tag_configs_content={},

+                         update_tag_config_only=True)

+ 

+         self.koji_session.setInheritanceData.assert_not_called()

+ 

+         expected = [{

+             'name': 'mariadb',

+             'stream': '10.4',

+             'requires': {'platform': 'f30'},

+             'buildrequires': {'platform': 'f30'},

+             'priority': 50

+         }]

+ 

+         tag_configs = self.read_tag_config_file()

+         assert expected == tag_configs['f30-build']['modules']

+ 

+     @patch('ursa_major.handlers.add_module.AddModuleHandler.write_tag_config')

+     def test_specified_same_module_config(

+             self, write_tag_config):

+         """

+         Both tag inheritance and tag config file are not updated because same

+         module metadata is specified from command line.

+         """

+         self.add_module(tag_configs_content={

+             'f30-build': {

+                 'modules': [

+                     # Compare with the metadata specified from command line,

+                     # all of them are same.

+                     {

+                         'name': 'mariadb',

+                         'stream': '10.4',

+                         'requires': {'platform': 'f30'},

+                         'buildrequires': {'platform': 'f30'},

+                         'priority': 50,

+                     }

+                 ],

+                 'owners': ['owner@example.com']

+             }

+         })

+ 

+         self.koji_session.setInheritanceData.assert_not_called()

+         write_tag_config.assert_not_called()

+ 

+     def test_remove_old_koji_tag_and_add_latest_koji_tag_to_inheritance(self):

+         self.add_module(mock_inheritance_data=[

+             # Noisy koji_tag, which should not be changed.

+             {

+                 'parent_id': 5,

+                 'name': 'module-ant-1.10-20181122140939-819b5873',

+                 'priority': 2,

+             },

+             # This tag will be removed because this old koji_tag exists in the modules

+             # returned from MBS.

+             {

+                 'parent_id': 101,

+                 'name': 'module-mariadb-10.4-3020190304180835-a5b0195c',

+                 'priority': 10,

+             },

+         ])

+ 

+         self.koji_session.setInheritanceData.assert_called_once_with(99, [

+             {

+                 'parent_id': 5,

+                 'name': 'module-ant-1.10-20181122140939-819b5873',

+                 'priority': 2,

+             },

+             {

+                 'parent_id': 101,

+                 'name': 'module-mariadb-10.4-3020190304180835-a5b0195c',

+                 'priority': 10,

+                 'delete link': True,

+             },

+             {

+                 'parent_id': 100,

+                 'priority': 50,

+                 'maxdepth': None,

+                 'intransitive': False,

+                 'noconfig': False,

+                 'pkg_filter': ''

+             },

+         ])

+ 

+     def test_add_module_with_buildrequires(self):

+         """

+         Add a module with buildrequires specified from command line but no

+         buildrequires was specified before and the corresponding module config

+         does not have buildrequires in tag config file.

+ 

+         The expected result is to remove the koij_tag of the exisitng module

+         within tag config file, and find latest module with the specified

+         buildrequires by CLI option --buildrequires, and add its koji_tag to

+         tag inheritance eventually.

+         """

+         self.add_module(

+             mock_inheritance_data=[

+                 # Noisy tag in inheritance which should not be changed.

+                 {

+                     'parent_id': 20,

+                     'name': 'module-ant-1.10-20181122140939-819b5873',

+                     'priority': 10,

+                 },

+                 # The is is the koji_tag that was added when buildrequires is

+                 # not specified in tag config. So, it will be removed and

+                 # koji_tag of latest mariadb:10.4 will be added to tag

+                 # inheritance.

+                 {

+                     'parent_id': 101,

+                     'name': 'module-mariadb-10.4-3020190304180835-a5b0195c',

+                     'priority': 20,

+                 },

+             ],

+             tag_configs_content={

+                 'f30-build': {

+                     'modules': [

+                         # Compare with the metadata specified from command line,

+                         # all of them are same.

+                         {

+                             'name': 'mariadb',

+                             'stream': '10.4',

+                             'requires': {'platform': 'f30'},

+                             'priority': 50,

+                             # Note that, this module was added without

+                             # specifying buildrequires, whereas it is given by

+                             # option --buildrequire from command line. This is

+                             # just the difference.

+                         }

+                     ],

+                     'owners': ['owner@example.com']

+                 }

+             },

+         )

+ 

+         self.koji_session.setInheritanceData.assert_called_once_with(99, [

+             {

+                 'parent_id': 20,

+                 'name': 'module-ant-1.10-20181122140939-819b5873',

+                 'priority': 10,

+             },

+             {

+                 'parent_id': 101,

+                 'name': 'module-mariadb-10.4-3020190304180835-a5b0195c',

+                 'priority': 20,

+                 'delete link': True,

+             },

+             # This is the koji_tag of the latest module mariadb:10.4 to add

+             {

+                 'parent_id': 100,

+                 'priority': 50,

+                 'maxdepth': None,

+                 'intransitive': False,

+                 'noconfig': False,

+                 'pkg_filter': ''

+             },

+         ])

+ 

+         expected = {

+             'f30-build': {

+                 'modules': [

+                     {

+                         'name': 'mariadb',

+                         'stream': '10.4',

+                         'requires': {'platform': 'f30'},

+                         'buildrequires': {'platform': 'f30'},

+                         'priority': 50,

+                     }

+                 ],

+                 'owners': ['owner@example.com']

+             }

+         }

+         assert expected == self.read_tag_config_file()

+ 

+ 

+ class TestAddModuleFunctions:

+ 

+     # NOTE: once MBS supports filtering modules builds by requires or

+     # buildrequires, the test data should be updated because it is unnecessary

+     # to filter module builds locally by Ursa-Major itself.

+ 

+     @pytest.mark.parametrize(

+         'module_config,module_builds,expected_koji_tags', [

+             # None of these koji_tags exist in tag inheritance.

+             (ModuleConfig({'name': 'mariadb', 'stream': '10.3',

+                            'version': '3120190304180825', 'context': 'f636be4b'}),

+              [{'id': 1,

+                'name': 'mariadb',

+                'koji_tag': 'module-mariadb-10.3-3020190301191548-a5b0195c'},

+               {'id': 2,

+                'name': 'mariadb',

+                'koji_tag': 'module-mariadb-10.3-3120190304180825-f636be4b'}],

+              []),

+ 

+             # koji_tag exists in tag inheritance

+             (ModuleConfig({'name': 'mariadb', 'stream': '10.4',

+                            'version': '3120190304180825', 'context': 'f636be4b'}),

+              [{'id': 1,

+                'name': 'mariadb',

+                'koji_tag': 'module-mariadb-10.4-3020190313091759-a5b0195c'},

+               {'id': 2,

+                'name': 'mariadb',

+                # This older koji_tag was added previously

+                'koji_tag': 'module-mariadb-10.4-3020190304180835-a5b0195c'}],

+              ['module-mariadb-10.4-3020190304180835-a5b0195c']),

+ 

+             # Modules are filtered by requires.

+             # None of these koji_tags exist in tag inheritance.

+             (ModuleConfig({'name': 'mariadb', 'stream': '10.3',

+                            'version': '3120190304180825', 'context': 'f636be4b',

+                            'requires': {'platform': 'f30'}}),

+              [{'id': 1,

+                'name': 'mariadb',

+                'koji_tag': 'module-mariadb-10.3-3020190301191548-a5b0195c',

+                'modulemd': make_mmd(name='mariadb', stream='10.3',

+                                     version='3020190301191548', context='a5b0195c',

+                                     requires={'platform': 'f30'},

+                                     buildrequires={'platform': 'f30'}).dumps(),

+                },

+               {'id': 2,

+                'name': 'mariadb',

+                'koji_tag': 'module-mariadb-10.3-3120190304180825-f636be4b',

+                'modulemd': make_mmd(name='mariadb', stream='10.3',

+                                     version='3120190304180825', context='f636be4b',

+                                     requires={'platform': 'f29'},

+                                     buildrequires={'platform': 'f29'}).dumps(),

+                }],

+              []),

+ 

+             # koji_tag exists in tag inheritance and module builds are

+             # filtered by both requires and buildrequires.

+             (ModuleConfig({'name': 'mariadb', 'stream': '10.4',

+                            'version': '3120190304180825', 'context': 'f636be4b',

+                            'requires': {'platform': 'f30'},

+                            'buildrequires': {'platform': 'f30'}}),

+              [{'id': 1,

+                'name': 'mariadb',

+                'koji_tag': 'module-mariadb-10.4-3020190313091759-a5b0195c',

+                'modulemd': make_mmd(name='mariadb', stream='10.4',

+                                     version='3020190313091759', context='a5b0195c',

+                                     requires={'platform': 'f29'},

+                                     buildrequires={'platform': 'f29'}).dumps(),

+                },

+               {'id': 2,

+                'name': 'mariadb',

+                # This is the koji_tag for platform:f30

+                'koji_tag': 'module-mariadb-10.4-3020190304180835-a5b0195c',

+                'modulemd': make_mmd(name='mariadb', stream='10.4',

+                                     version='3020190304180835', context='a5b0195c',

+                                     requires={'platform': 'f30'},

+                                     buildrequires={'platform': 'f30'}).dumps(),

+                }],

+              ['module-mariadb-10.4-3020190304180835-a5b0195c']),

+ 

+             # No module builds match the platform:f30

+             (ModuleConfig({'name': 'mariadb', 'stream': '10.4',

+                            'version': '3120190304180825', 'context': 'f636be4b',

+                            'requires': {'platform': 'f30'},

+                            'buildrequires': {'platform': 'f30'}}),

+              [{'id': 1,

+                'name': 'mariadb',

+                'koji_tag': 'module-mariadb-10.4-3020190313091759-a5b0195c',

+                'modulemd': make_mmd(name='mariadb', stream='10.4',

+                                     version='3020190313091759', context='a5b0195c',

+                                     requires={'platform': 'f29'},

+                                     buildrequires={'platform': 'f29'}).dumps(),

+                },

+               # Although this koji_tag was added to tag inheritance, but no

+               # koji_tag is found that matches platform:f30.

+               {'id': 2,

+                'name': 'mariadb',

+                'koji_tag': 'module-mariadb-10.4-3020190304180835-a5b0195c',

+                'modulemd': make_mmd(name='mariadb', stream='10.4',

+                                     version='3020190304180835', context='a5b0195c',

+                                     requires={'platform': 'f28'},

+                                     buildrequires={'platform': 'f28'}).dumps(),

+                }],

+              []),

+         ])

+     @patch('ursa_major.koji_service.koji')

+     @patch('requests.get')

+     def test_find_module_tags_from_koji_inheritance(

+             self, get, koji, module_config, module_builds, expected_koji_tags):

+         koji_session = koji.ClientSession.return_value

+         koji_session.getInheritanceData.return_value = [

+             {'parent_id': 100,

+              'name': 'module-ant-1.10-20181122140939-819b5873'},

+             {'parent_id': 101,

+              'name': '"module-hyperfine-latest-3120190318171218-f636be4b'},

+             {'parent_id': 102,

+              'name': 'module-exa-latest-2820190316115637-8de94f32'},

+             {'parent_id': 103,

+              'name': 'module-mariadb-10.4-3020190304180835-a5b0195c'},

+         ]

+ 

+         get.return_value = Mock(status_code=200)

+         get.return_value.json.return_value = make_mbs_response(module_builds)

+ 

+         handler = AddModuleHandler(config=URSA_MAJOR_TEST_CONFIG)

+         handler.connect_koji()

+         handler.connect_mbs()

+         found_koji_tags = handler.find_module_tags_from_koji_inheritance(

+             'f30-build', module_config)

+         assert expected_koji_tags == found_koji_tags

file modified
+10 -11

@@ -29,7 +29,7 @@ 

  import tempfile

  from argparse import Namespace

  

- from tests import UrsaMajorTestCase, MockResponse

+ from tests import UrsaMajorTestCase, MockResponse, make_mmd

  from ursa_major.mbs import MBS

  from ursa_major.handlers.add_tag import ModuleInfo, AddTagHandler

  

@@ -139,7 +139,7 @@ 

          session = koji.ClientSession.return_value

          session.getInheritanceData.side_effect = self.fake_get_inheritance_data

  

-         self.handler._mbs.get_module_mmd.return_value = self.make_mmd(

+         self.handler._mbs.get_module_mmd.return_value = make_mmd(

              'autotools', 'rhel-8.0', '20180101', '123456',

              {'platform': ['rhel-8.0']})

  

@@ -215,8 +215,7 @@ 

              koji_tag='module-d243299e85d7e9aa'

          ))

          mock_mbs = mock.MagicMock()

-         fake_mmd = self.make_mmd("testmodule", "master",

-                                  "20180101", "1234567")

+         fake_mmd = make_mmd("testmodule", "master", "20180101", "1234567")

          mock_mbs.get_module_mmd.return_value = fake_mmd

          self.handler.connect_koji = mock.MagicMock()

          self.handler.connect_mbs = mock.MagicMock()

@@ -239,10 +238,10 @@ 

          message = self.load_json_from_file(message_file)

          mock_mbs = mock.MagicMock()

          fake_requires = {"platform": ["el8"]}

-         fake_mmd = self.make_mmd("testmodule", "rhel-8.0",

-                                  "20180409051516", "9e5fe74b",

-                                  requires=fake_requires,

-                                  buildrequires=fake_requires)

+         fake_mmd = make_mmd("testmodule", "rhel-8.0",

+                             "20180409051516", "9e5fe74b",

+                             requires=fake_requires,

+                             buildrequires=fake_requires)

          mock_mbs.get_module_mmd.return_value = fake_mmd

          modinfo = ModuleInfo.from_mbs_message(mock_mbs, message)

          matched_config = AddTagHandler.get_module_config(fake_configs, modinfo)

@@ -420,9 +419,9 @@ 

          message = self.load_json_from_file("testmodule_ready_message.json")

          mock_mbs = mock.MagicMock()

          fake_requires = {"platform": ["el8"]}

-         fake_mmd = self.make_mmd("testmodule", "rhel-8.0",

-                                  "20180409051516", "9e5fe74b",

-                                  requires=fake_requires)

+         fake_mmd = make_mmd("testmodule", "rhel-8.0",

+                             "20180409051516", "9e5fe74b",

+                             requires=fake_requires)

          mock_mbs.get_module_mmd.return_value = fake_mmd

          modinfo = ModuleInfo.from_mbs_message(mock_mbs, message)

  

@@ -0,0 +1,180 @@ 

+ # -*- coding: utf-8 -*-

+ # Copyright (c) 2019  Red Hat, Inc.

+ #

+ # Permission is hereby granted, free of charge, to any person obtaining a copy

+ # of this software and associated documentation files (the "Software"), to deal

+ # in the Software without restriction, including without limitation the rights

+ # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell

+ # copies of the Software, and to permit persons to whom the Software is

+ # furnished to do so, subject to the following conditions:

+ #

+ # The above copyright notice and this permission notice shall be included in all

+ # copies or substantial portions of the Software.

+ #

+ # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR

+ # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,

+ # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE

+ # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER

+ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,

+ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE

+ # SOFTWARE.

+ #

+ # Written by Chenxiong Qi <cqi@redhat.com>

+ 

+ import json

+ import os

+ import pytest

+ import tempfile

+ 

+ from mock import call, patch, Mock

+ from tests import TEST_DATA_DIR, make_mbs_response, make_mmd

+ from ursa_major.cli import main

+ 

+ 

+ class TestCheckConfig:

+ 

+     def setup_method(self):

+         fd, self.tag_config_file = tempfile.mkstemp()

+         os.close(fd)

+ 

+         self.ClientSession_patcher = patch('koji.ClientSession')

+         self.mock_ClientSession = self.ClientSession_patcher.start()

+         self.koji_session = self.mock_ClientSession.return_value

+ 

+         self.get_patcher = patch('requests.get')

+         self.mock_get = self.get_patcher.start()

+ 

+     def teardown_method(self):

+         self.get_patcher.stop()

+         os.unlink(self.tag_config_file)

+ 

+     def write_tag_config_file(self, content):

+         with open(self.tag_config_file, 'w') as f:

+             json.dump(content, f)

+ 

+     def check_config(self):

+         config_file = os.path.join(TEST_DATA_DIR, 'ursa-major-test.conf')

+         cli_cmd = [

+             'ursa-major', 'check-config',

+             '--config', config_file,

+             '--tag-config-file', self.tag_config_file

+         ]

+ 

+         with patch('sys.argv', new=cli_cmd):

+             main()

+ 

+     @pytest.mark.parametrize('modules,error_log', [

+         ([

+             [{'name': 'mariadb'}],

+             'Missing stream in module config: {}'.format({u'name': u'mariadb'})

+         ]),

+         ([

+             [{'stream': '10.4', 'priority': 10}],

+             'Missing name in module config: {}'.format(

+                 {u'stream': u'10.4', u'priority': 10})

+         ]),

+         ([

+             [{'name': 'mariadb', 'stream': '10.4'}],

+             'Missing priority in module config: {}'.format(

+                 {u'name': u'mariadb', u'stream': u'10.4'})

+         ]),

+     ])

+     def test_missing_mandatory_fields(self, modules, error_log):

+         self.write_tag_config_file({

+             'f30-build': {

+                 'modules': modules,

+                 'owners': ['owner@example.com']

+             }

+         })

+ 

+         self.koji_session.getInheritanceData.return_value = [

+             {

+                 'parent': 10,

+                 'name': 'module-ant-1.10-20181122140939-819b5873',

+                 'priority': 10

+             }

+         ]

+ 

+         with patch('ursa_major.handlers.check_config.log') as log:

+             # This is only for making the program terminate and raise SystemExit

+             log.error.counter = 3

+ 

+             with pytest.raises(SystemExit):

+                 self.check_config()

+ 

+             log.error.assert_has_calls([call(error_log)])

+ 

+     def test_show_error_if_cannot_get_tag_inheritance_data(self):

+         self.write_tag_config_file({

+             'f30-build': {

+                 'modules': [],

+                 'owners': ['owner@example.com']

+             }

+         })

+ 

+         # This test requires an error raised from Koji API getInheritanceData

+         # So, just raise any error

+         self.koji_session.getInheritanceData.side_effect = ValueError

+ 

+         with patch('ursa_major.handlers.check_config.log') as log:

+             # This is only for making the program terminate and raise SystemExit

+             log.error.counter = 3

+ 

+             with pytest.raises(SystemExit):

+                 self.check_config()

+ 

+             log.error.assert_has_calls([

+                 call('Unable to get inheritance data of tag %s: %s', u'f30-build', '')

+             ], any_order=True)

+ 

+     def test_verify_valid_module_config(self):

+         self.write_tag_config_file({

+             'f30-build': {

+                 'modules': [

+                     {

+                         'name': 'mariadb', 'stream': '10.4', 'priority': 30,

+                         'requires': {'platform': 'f30'},

+                         'buildrequires': {'platform': 'f30'}

+                     },

+                     {'name': 'ffsend', 'stream': 'latest', 'priority': 40},

+                 ],

+                 'owners': ['owner@example.com']

+             }

+         })

+ 

+         self.mock_get.return_value = Mock(status_code=200)

+         self.mock_get.return_value.json.return_value = make_mbs_response([

+             {

+                 'id': 3617,

+                 'name': 'mariadb',

+                 'koji_tag': 'module-mariadb-10.4-3020190313091759-a5b0195c',

+                 'modulemd': make_mmd(name='mariadb', stream='10.4',

+                                      version='3020190313091759', context='a5b0195c',

+                                      requires={'platform': 'f30'},

+                                      buildrequires={'platform': 'f30'}).dumps(),

+             },

+             {

+                 'id': 3640,

+                 'name': 'ffsend',

+                 'koji_tag': 'module-ffsend-latest-3020190316024948-a5b0195c',

+                 'modulemd': make_mmd(name='mariadb', stream='10.4',

+                                      version='3020190316024948', context='a5b0195c',

+                                      requires={'platform': 'f30'},

+                                      buildrequires={'platform': 'f30'}).dumps(),

+             }

+         ])

+ 

+         self.koji_session.getInheritanceData.return_value = [

+             {

+                 'parent': 10,

+                 'name': 'module-ant-1.10-20181122140939-819b5873',

+                 'priority': 10

+             }

+         ]

+ 

+         with patch('ursa_major.handlers.check_config.log') as log:

+             # This is only for making the program terminate normally

+             log.error.counter = 0

+ 

+             self.check_config()

+             log.error.assert_not_called()

@@ -0,0 +1,19 @@ 

+ [main]

+ log_level = info

+ 

+ [koji]

+ profile = koji

+ 

+ [mbs]

+ server_url = https://mbs.localhost/

+ 

+ [mail]

+ mail_processing = true

+ mail_log_level = info

+ mail_server = smtp.example.com

+ mail_from = ursa-major@example.com

+ mail_replyto = ursa-major@example.com

+ # email addresses seperated by ','

+ mail_always_cc = ursa-major-admin@example.com

+ mail_always_bcc = 

+ mail_subject_prefix = [ursa-major]

@@ -0,0 +1,220 @@ 

+ # -*- coding: utf-8 -*-

+ # Copyright (c) 2019  Red Hat, Inc.

+ #

+ # Permission is hereby granted, free of charge, to any person obtaining a copy

+ # of this software and associated documentation files (the "Software"), to deal

+ # in the Software without restriction, including without limitation the rights

+ # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell

+ # copies of the Software, and to permit persons to whom the Software is

+ # furnished to do so, subject to the following conditions:

+ #

+ # The above copyright notice and this permission notice shall be included in all

+ # copies or substantial portions of the Software.

+ #

+ # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR

+ # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,

+ # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE

+ # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER

+ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,

+ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE

+ # SOFTWARE.

+ #

+ # Written by Chenxiong Qi <cqi@redhat.com>

+ 

+ import json

+ import os

+ import tempfile

+ 

+ from mock import patch, Mock

+ from ursa_major.cli import main

+ from tests import TEST_DATA_DIR, make_mbs_response, make_mmd

+ 

+ 

+ class TestRemoveModule:

+     """Test CLI command remove-module"""

+ 

+     def setup_method(self):

+         fd, self.tag_config_file = tempfile.mkstemp()

+         os.close(fd)

+ 

+     def teardown_method(self):

+         os.unlink(self.tag_config_file)

+ 

+     def write_tag_config_file(self, content):

+         with open(self.tag_config_file, 'w') as f:

+             json.dump(content, f)

+ 

+     def run_cli(self, options):

+         config_file = os.path.join(TEST_DATA_DIR, 'ursa-major-test.conf')

+         cli_cmd = ['ursa-major', 'remove-module', '--config', config_file] + options

+ 

+         with patch('sys.argv', new=cli_cmd):

+             main()

+ 

+     @patch('koji.ClientSession')

+     @patch('ursa_major.mbs.requests.get')

+     def test_module_koji_tag_isnt_present_in_tag_inheritance(self, get, ClientSession):

+         session = ClientSession.return_value

+         session.getInheritanceData.return_value = [

+             {'name': 'module-ant-123'},

+             {'name': 'module-java-456'},

+         ]

+ 

+         get.return_value = Mock(status_code=200)

+         get.return_value.json.return_value = {

+             'items': [

+                 {

+                     # This koji_tag is not present in the tag inheritance, so

+                     # no tag is going to be removed.

+                     'koji_tag': 'module-python-3.6',

+                     'modulemd': make_mmd(name='mariadb', stream='10.4',

+                                          version='3020190313091759', context='a5b0195c',

+                                          requires={'platform': 'f30'},

+                                          buildrequires={'platform': 'f30'}).dumps(),

+                 },

+             ],

+             'meta': {

+                 'prev': None,

+                 'last': None,

+                 'next': None,

+                 'total': 1,

+                 'per_page': 10,

+                 'first': 'https://mbs.example.com/module-build-service/1/module-builds/'

+                          '?per_page=10&page=1&name=testmodule',

+                 'pages': 1,

+                 'page': 1

+             }

+         }

+ 

+         # Write empty tag config file because we do not want to test if the tag

+         # config file is written.

+         self.write_tag_config_file({})

+ 

+         self.run_cli([

+             '--name', 'python', '--stream', '3.6', '--tag', 'f30-build',

+             '--tag-config-file', self.tag_config_file

+         ])

+ 

+         session.setInheritanceData.assert_not_called()

+ 

+     @patch('koji.ClientSession')

+     @patch('ursa_major.mbs.requests.get')

+     def remove_module(self, not_update_file, get, ClientSession):

+         session = ClientSession.return_value

+         session.getInheritanceData.return_value = [

+             {'parent_id': 20,

+              'name': 'module-ant-1.10-20181122140939-819b5873'},

+             {'parent_id': 100,

+              'name': 'module-mariadb-10.4-3020190304180835-a5b0195c'},

+         ]

+         session.getTag.return_value = {

+             'id': 100,

+             'name': 'module-mariadb-10.4-3020190304180835-a5b0195c',

+         }

+ 

+         get.return_value = Mock(status_code=200)

+         get.return_value.json.return_value = make_mbs_response([

+             {

+                 'koji_tag': 'module-mariadb-10.4-3020190313091759-a5b0195c',

+                 'modulemd': make_mmd(name='mariadb', stream='10.4',

+                                      version='3020190313091759', context='a5b0195c',

+                                      requires={'platform': 'f30'},

+                                      buildrequires={'platform': 'f30'}).dumps(),

+             },

+             {

+                 # This koji_tag is present in the inheritance, so it should be removed.

+                 'koji_tag': 'module-mariadb-10.4-3020190304180835-a5b0195c',

+                 'modulemd': make_mmd(name='mariadb', stream='10.4',

+                                      version='3020190304180835', context='a5b0195c',

+                                      requires={'platform': 'f30'},

+                                      buildrequires={'platform': 'f30'}).dumps(),

+             }

+         ])

+ 

+         cmd = [

+             '--name', 'mariadb', '--stream', '10.4',

+             '--require', 'platform:f30', '--buildrequire', 'platform:f30',

+             '--tag', 'f30-build',

+             '--tag-config-file', self.tag_config_file

+         ]

+         if not_update_file:

+             cmd.append('--not-update-config')

+ 

+         self.run_cli(cmd)

+ 

+         data = session.getInheritanceData.return_value[:]

+         data[-1]['delete link'] = True

+         session.setInheritanceData.assert_called_once_with('f30-build', data)

+ 

+     def test_remove_module_and_remove_module_config_from_tag_configs(self):

+         fake_tag_configs = {

+             'f30-build': {

+                 'modules': [

+                     {

+                         'name': 'mariadb',

+                         'stream': '10.4',

+                         'requires': {'platform': 'f30'},

+                         'buildrequires': {'platform': 'f30'},

+                     }

+                 ],

+                 'owners': ['someone@example.com']

+             }

+         }

+         self.write_tag_config_file(fake_tag_configs)

+ 

+         self.remove_module(False)

+ 

+         with open(self.tag_config_file, 'r') as f:

+             tag_configs = json.load(f)

+ 

+         mariadb_module_config = fake_tag_configs['f30-build']['modules'][0]

+         assert mariadb_module_config not in tag_configs['f30-build']['modules']

+ 

+     def test_remove_module_and_not_remove_module_config_from_tag_configs(self):

+         fake_tag_configs = {

+             'f30-build': {

+                 'modules': [

+                     {

+                         'name': 'mariadb',

+                         'stream': '10.4',

+                         'requires': {'platform': 'f30'},

+                         'buildrequires': {'platform': 'f30'},

+                     }

+                 ],

+                 'owners': ['someone@example.com']

+             }

+         }

+         self.write_tag_config_file(fake_tag_configs)

+ 

+         self.remove_module(True)

+ 

+         with open(self.tag_config_file, 'r') as f:

+             tag_configs = json.load(f)

+ 

+         mariadb_module_config = fake_tag_configs['f30-build']['modules'][0]

+         assert mariadb_module_config in tag_configs['f30-build']['modules']

+ 

+     def test_specified_module_isnt_present_in_module_config_file(self):

+         # remove_module removes mariadb, but tag config file only contains ant-1.10

+         fake_tag_configs = {

+             'f30-build': {

+                 'modules': [

+                     {

+                         'name': 'ant',

+                         'stream': '1.10',

+                         'requires': {'platform': 'f30'},

+                         'buildrequires': {'platform': 'f30'},

+                     }

+                 ],

+                 'owners': ['someone@example.com']

+             }

+         }

+         self.write_tag_config_file(fake_tag_configs)

+ 

+         self.remove_module(False)

+ 

+         with open(self.tag_config_file, 'r') as f:

+             tag_configs = json.load(f)

+ 

+         expected = fake_tag_configs['f30-build']['modules']

+         assert expected == tag_configs['f30-build']['modules']

file modified
+10 -18

@@ -24,76 +24,68 @@ 

  

  from tests import UrsaMajorTestCase

  from ursa_major.utils import mmd_has_requires

+ from tests import make_mmd

  

  

  class TestMmdHasRequires(UrsaMajorTestCase):

      def test_mmd_has_neg_requires(self):

          requires = {'platform': 'f26'}

-         mmd = self.make_mmd('testmodule', 'master', '123', '00000000',

-                             requires)

+         mmd = make_mmd('testmodule', 'master', '123', '00000000', requires)

          check_requires = {'platform': '-f26'}

          result = mmd_has_requires(mmd, check_requires)

          self.assertFalse(result)

  

      def test_mmd_missing_requires(self):

          requires = {'platform': 'f28'}

-         mmd = self.make_mmd('testmodule', 'master', '123', '00000000',

-                             requires)

+         mmd = make_mmd('testmodule', 'master', '123', '00000000', requires)

          check_requires = {'platform': 'f28', 'python3': 'master'}

          result = mmd_has_requires(mmd, check_requires)

          self.assertFalse(result)

  

      def test_mmd_not_has_neg_requires(self):

          requires = {'platform': 'f28', 'python3': 'master'}

-         mmd = self.make_mmd('testmodule', 'master', '123', '00000000',

-                             requires)

+         mmd = make_mmd('testmodule', 'master', '123', '00000000', requires)

          check_requires = {'platform': '-f26', 'python3': 'master'}

          result = mmd_has_requires(mmd, check_requires)

          self.assertTrue(result)

  

      def test_mmd_does_not_has_pos_requires(self):

          requires = {'platform': 'f28', 'python3': 'master'}

-         mmd = self.make_mmd('testmodule', 'master', '123', '00000000',

-                             requires)

+         mmd = make_mmd('testmodule', 'master', '123', '00000000', requires)

          check_requires = {'platform': 'f28', 'python3': 'f28'}

          result = mmd_has_requires(mmd, check_requires)

          self.assertFalse(result)

  

      def test_mmd_has_more_requires_than_checks(self):

          requires = {'platform': 'f28', 'python3': 'master'}

-         mmd = self.make_mmd('testmodule', 'master', '123', '00000000',

-                             requires)

+         mmd = make_mmd('testmodule', 'master', '123', '00000000', requires)

          check_requires = {'platform': 'f28'}

          result = mmd_has_requires(mmd, check_requires)

          self.assertTrue(result)

  

      def test_mmd_has_more_require_streams_than_checks(self):

          requires = {'platform': ['f28', 'f29'], 'python3': 'master'}

-         mmd = self.make_mmd('testmodule', 'master', '123', '00000000',

-                             requires)

+         mmd = make_mmd('testmodule', 'master', '123', '00000000', requires)

          check_requires = {'platform': 'f28'}

          result = mmd_has_requires(mmd, check_requires)

          self.assertTrue(result)

  

      def test_mmd_has_a_neg_stream_in_streams_list(self):

          requires = {'platform': ['f28', 'f29'], 'python3': 'master'}

-         mmd = self.make_mmd('testmodule', 'master', '123', '00000000',

-                             requires)

+         mmd = make_mmd('testmodule', 'master', '123', '00000000', requires)

          check_requires = {'platform': '-f28'}

          result = mmd_has_requires(mmd, check_requires)

          self.assertFalse(result)

  

      def test_mmd_has_a_neg_stream_in_multi_neg_streams(self):

          requires = {'platform': ['f27'], 'python3': 'master'}

-         mmd = self.make_mmd('testmodule', 'master', '123', '00000000',

-                             requires)

+         mmd = make_mmd('testmodule', 'master', '123', '00000000', requires)

          check_requires = {'platform': ['-f26', '-f27']}

          result = mmd_has_requires(mmd, check_requires)

          self.assertFalse(result)

  

      def test_check_with_empty_requires(self):

          requires = {'platform': ['f27'], 'python3': 'master'}

-         mmd = self.make_mmd('testmodule', 'master', '123', '00000000',

-                             requires)

+         mmd = make_mmd('testmodule', 'master', '123', '00000000', requires)

          result = mmd_has_requires(mmd, {})

          self.assertTrue(result)

file modified
+7

@@ -125,6 +125,13 @@ 

          action='append',

          dest='module_requires',

          help='module runtime dependency in NAME:STREAM')

+     module_config_parser.add_argument(

+         '--buildrequire',

+         metavar='NAME:STREAM',

+         type=clean_module_require,

+         action='append',

+         dest='module_buildrequires',

+         help='module build dependency in NAME:STREAM, for example platform:f30')

  

      show_config_parser = subparser.add_parser(

          "show-config",

@@ -47,13 +47,15 @@ 

          self.name = module_config['name']

          self.stream = module_config['stream']

          self.requires = module_config.get('requires', None)

+         self.buildrequires = module_config.get('buildrequires', None)

  

      @property

      def raw_module_config(self):

          return self._raw_module_config

  

      def __str__(self):

-         return '{}:{}:{!r}'.format(self.name, self.stream, self.requires)

+         return '{}:{}:{!r}:{!r}'.format(

+             self.name, self.stream, self.requires, self.buildrequires)

  

      def compare(self, other):

          """

@@ -66,6 +68,8 @@ 

              return ModuleConfigCmp.DIFFERENT

          if self.requires != other.requires:

              return ModuleConfigCmp.DIFFERENT

+         if self.buildrequires != other.buildrequires:

+             return ModuleConfigCmp.DIFFERENT

          return ModuleConfigCmp.SAME

  

  

@@ -116,12 +120,19 @@ 

          else:

              module_requires = None

  

+         if self.args.module_buildrequires:

+             module_buildrequires = dict(self.args.module_buildrequires)

+         else:

+             module_buildrequires = None

+ 

          input_module_config = ModuleConfig(

              module_config=dict(

                  name=self.args.module_name,

                  stream=self.args.module_stream,

                  priority=self.args.priority,

-                 requires=module_requires))

+                 requires=module_requires,

+                 buildrequires=module_buildrequires,

+             ))

  

          # existing tags of specified module from cmdline

          existing_input_module_tags = \

@@ -193,10 +204,13 @@ 

              # which has been added to tags_to_remove before

  

          elif config_diff == ModuleConfigCmp.DIFFERENT:

+             # Module in config changes, it can be a change in requires,

+             # buildrequires or priority.

+ 

              log.info("A module with same name and stream exists in tag config file "

                       "under tag '%s', but has different configuration", self.op_tag)

-             # module in config changes, it can be a change in requires or priority,

-             # check whether the specified priority conflicts with other modules

+ 

+             # Check whether the specified priority conflicts with other modules

              existing_priorities = self.get_module_priorities_in_config(self.op_tag)

              if (input_module_config.priority != stock_module_config.priority and

                      input_module_config.priority in existing_priorities):

@@ -205,7 +219,8 @@ 

                                         input_module_config.priority, self.op_tag))

  

              if to_update_inheritance:

-                 if input_module_config.requires != stock_module_config.requires:

+                 if (input_module_config.requires != stock_module_config.requires or

+                         input_module_config.buildrequires != stock_module_config.buildrequires):

                      # module requires changed, we need to remove old tags

                      existing_stock_tags = self.find_module_tags_from_koji_inheritance(

                          self.op_tag, stock_module_config)

@@ -315,8 +330,9 @@ 

          :param module_config: an object of `ModuleConfig`

          :return: tag name or None

          """

-         if module_config.requires:

+         if module_config.requires or module_config.buildrequires:

              modules = self.mbs.get_modules_with_requires(

+                 buildrequires=module_config.buildrequires,

                  requires=module_config.requires,

                  name=module_config.name,

                  stream=module_config.stream,

@@ -359,9 +375,10 @@ 

          :param module_config: instance of ModuleConfig

          """

          tags_from_koji = set(t['name'] for t in self.koji.get_inheritance_data(tag))

-         if module_config.requires:

+         if module_config.requires or module_config.buildrequires:

              modules = self.mbs.get_modules_with_requires(

                  requires=module_config.requires,

+                 buildrequires=module_config.buildrequires,

                  name=module_config.name,

                  stream=module_config.stream,

                  state=5)  # only ready modules

@@ -388,6 +405,8 @@ 

                                    priority=module_config.priority)

          if module_config.requires:

              module_config_dict['requires'] = module_config.requires

+         if module_config.buildrequires:

+             module_config_dict['buildrequires'] = module_config.buildrequires

  

          modules_with_same_name_stream = [m for m in modules if

                                           m['name'] == module_config.name and

@@ -112,6 +112,7 @@ 

  

                  modules_of_config = self.mbs.get_modules_with_requires(

                      requires=module_config.get('requires', {}),

+                     buildrequires=module_config.get('buildrequires'),

                      name=module_config['name'],

                      stream=module_config['stream'],

                      state=5,  # state_name = ready

@@ -36,6 +36,10 @@ 

          self.module_requires = {}

          if self.args.module_requires is not None:

              self.module_requires = dict(self.args.module_requires)

+         if self.args.module_buildrequires is not None:

+             self.module_buildrequires = dict(self.args.module_buildrequires)

+         else:

+             self.module_buildrequires = {}

          self.tag_config_file = self.args.tag_config_file

          self.not_update_config = self.args.not_update_config

  

@@ -55,11 +59,16 @@ 

  

          inheritance_tags = [t['name'] for t in tag_inheritance]

  

-         log.info("Querying modules of '%s:%s' from MBS with requires: %r",

-                  self.module_name, self.module_stream, self.module_requires)

-         modules = self.mbs.get_modules_with_requires(requires=self.module_requires,

-                                                      name=self.module_name,

-                                                      stream=self.module_stream)

+         log.info("Querying modules of '%s:%s' from MBS with requires: %r and "

+                  "buildrequires: %r",

+                  self.module_name, self.module_stream, self.module_requires,

+                  self.module_buildrequires)

+ 

+         modules = self.mbs.get_modules_with_requires(

+             name=self.module_name,

+             stream=self.module_stream,

+             requires=self.module_requires,

+             buildrequires=self.module_buildrequires)

  

          module_tags = [m['koji_tag'] for m in modules]

          match_tags = list(set(inheritance_tags) & set(module_tags))

@@ -82,14 +91,15 @@ 

  

          found_config = None

          for config in module_configs:

-             same_name = config['name'] == self.module_name

-             same_stream = config['stream'] == self.module_stream

- 

-             config_requires = config.get('requires', {})

-             # NOTE: we only find config that matches module_requires exactly

-             same_requires = self.module_requires == config_requires

- 

-             if same_name and same_stream and same_requires:

+             if all((

Not saying it won't work, but why not to use simple if (self.module_name == config['name'] and self.module_stream == config['stream'] and ...)?

cqi commented 4 months ago

Not saying it won't work, but why not to use simple if (self.module_name == config['name'] and self.module_stream == config['stream'] and ...)?

It would be easier to add more conditions and still keep the code clear and easy to read than if (... and ... and ...).

By calling all to organize these statements, it could be read much easier than the if (... and ... and ...) I think. But, yeah, it is just personal preference, from the result point of view, there is no big difference. :)

+                 self.module_name == config['name'],

+                 self.module_stream == config['stream'],

+ 

+                 # NOTE: we only find config that matches module_requires exactly

+                 self.module_requires == config.get('requires', {}),

+                 # Same as requires

+                 self.module_buildrequires == config.get('buildrequires', {})

+             )):

                  found_config = config

                  break

  

file modified
+6 -5

@@ -241,12 +241,13 @@ 

          return self.koji_proxy.getInheritanceData(tag)

  

      def set_inheritance_data(self, tag, data):

-         """Set inheritance data

+         """A simple wrapper of Koji API setInheritanceData to make it easy to dry run.

  

-         :param tag: tag name passed into the first argument of

-             `setInheritanceData`.

-         :param data: list of mappings passed into the second argument of

-             `setInheritanceData`.

+         :param tag: tag id or name.

+         :type tag: int or str

+         :param data: list of mappings containing tag update information that

+             will be done by Koji.

+         :type data: list[dict]

          """

          if self.dry_run:

              log.info('DRY RUN: set inheritance data for tag %s: %s',

Not saying it won't work, but why not to use simple if (self.module_name == config['name'] and self.module_stream == config['stream'] and ...)?

Not saying it won't work, but why not to use simple if (self.module_name == config['name'] and self.module_stream == config['stream'] and ...)?

It would be easier to add more conditions and still keep the code clear and easy to read than if (... and ... and ...).

By calling all to organize these statements, it could be read much easier than the if (... and ... and ...) I think. But, yeah, it is just personal preference, from the result point of view, there is no big difference. :)

2 new commits added

  • Command check-config supports filtering modules on buildrequires
  • Command add-module supports buildrequires now
4 months ago

buildrequires change is not handled in later code, we need to add that.

we need to check buildrequires too:

if module_config.requires or module_config.buildrequires

1 new commit added

  • Fixes according to review comments
4 months ago

typos of "config"

Fixed.

missing tag config file.

Fixed.

we need to check buildrequires too:
if module_config.requires or module_config.buildrequires

Fixed.

4 new commits added

  • Fixes according to review comments
  • Command check-config supports filtering modules on buildrequires
  • Command add-module supports buildrequires now
  • Command remove-module supports filtering modules on buildrequires
4 months ago

2 new commits added

  • Add tests for AddModuleHandler methods
  • Avoid long modulemd embedded into fake data for tests
4 months ago

Pull-Request has been merged by cqi

4 months ago