From 065abe3c453f01f442a54bc809987ad62984eb5e Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Tue, 17 Sep 2019 20:30:07 +0800 Subject: [PATCH] Fix DetachedInstanceError in command module_build_locally Update test test_build_module_locally_set_stream accordingly by not mocking database session object in order to test against the real SQLAlchemy Session scope. Resolves: RHBZ#1752075 Signed-off-by: Chenxiong Qi --- module_build_service/manage.py | 12 +++- tests/staged_data/testmodule-local-build.yaml | 38 ++++++++++++ tests/test_manage.py | 61 ++++++++++++++----- 3 files changed, 93 insertions(+), 18 deletions(-) create mode 100644 tests/staged_data/testmodule-local-build.yaml diff --git a/module_build_service/manage.py b/module_build_service/manage.py index 523b8005..34bdb502 100755 --- a/module_build_service/manage.py +++ b/module_build_service/manage.py @@ -188,7 +188,7 @@ def build_module_locally( handle = FileStorage(fd) handle.filename = filename try: - modules_list = submit_module_build_from_yaml( + module_builds = submit_module_build_from_yaml( db_session, username, handle, params, stream=str(stream), skiptests=skiptests ) @@ -197,12 +197,20 @@ def build_module_locally( logging.error("Use '-s module_name:module_stream' to choose the stream") return + module_build_ids = [build.id for build in module_builds] + stop = module_build_service.scheduler.make_simple_stop_condition(db_session) # Run the consumer until stop_condition returns True module_build_service.scheduler.main([], stop) - if any(module.state == models.BUILD_STATES["failed"] for module in modules_list): + with models.make_db_session(conf) as db_session: + has_failed_module = db_session.query(models.ModuleBuild).filter( + models.ModuleBuild.id.in_(module_build_ids), + models.ModuleBuild.state == models.BUILD_STATES["failed"], + ).count() > 0 + + if has_failed_module: raise RuntimeError("Module build failed") diff --git a/tests/staged_data/testmodule-local-build.yaml b/tests/staged_data/testmodule-local-build.yaml new file mode 100644 index 00000000..9e920709 --- /dev/null +++ b/tests/staged_data/testmodule-local-build.yaml @@ -0,0 +1,38 @@ +document: modulemd +version: 2 +data: + 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. ’ + license: + module: + - MIT + dependencies: + - buildrequires: + platform: [] + requires: + platform: [] + references: + community: https://docs.pagure.org/modularity/ + documentation: https://fedoraproject.org/wiki/Fedora_Packaging_Guidelines_for_Modules + profiles: + default: + rpms: + - tangerine + api: + rpms: + - perl-Tangerine + - tangerine + components: + rpms: + perl-List-Compare: + rationale: A dependency of tangerine. + ref: master + perl-Tangerine: + rationale: Provides API for this module and is a dependency of tangerine. + ref: master + tangerine: + rationale: Provides API for this module. + ref: master + buildorder: 10 diff --git a/tests/test_manage.py b/tests/test_manage.py index 9a43c70f..a4fdf4a5 100644 --- a/tests/test_manage.py +++ b/tests/test_manage.py @@ -18,11 +18,13 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. import pytest -from mock import patch, mock_open, ANY, Mock +from mock import patch -from module_build_service import app -from module_build_service.manage import retire, build_module_locally +from module_build_service import app, models +from module_build_service.manage import manager_wrapper, retire from module_build_service.models import BUILD_STATES, ModuleBuild +from module_build_service.utils.general import deps_to_dict +from tests import clean_database, staged_data_filename @pytest.mark.usefixtures("model_tests_init_data") @@ -120,29 +122,56 @@ class TestMBSManage: expected_changed_count = 3 if confirm_expected else 0 assert len(retired_module_builds) == expected_changed_count - @patch("module_build_service.manage.open", create=True, new_callable=mock_open) - @patch("module_build_service.manage.submit_module_build_from_yaml") @patch("module_build_service.scheduler.main") @patch("module_build_service.manage.conf.set_item") - @patch("module_build_service.models.make_db_session") + # Do not allow flask_script exits by itself because we have to assert + # something after the command finishes. + @patch("sys.exit") + # The consumer is not required to run actually, so it does not make sense + # to publish message after creating a module build. + @patch("module_build_service.messaging.publish") def test_build_module_locally_set_stream( - self, make_db_session, conf_set_item, main, submit_module_build_from_yaml, patched_open + self, publish, sys_exit, conf_set_item, main, db_session ): - mock_db_session = Mock() - make_db_session.return_value.__enter__.return_value = mock_db_session + clean_database() + + cli_cmd = [ + 'mbs-manager', 'build_module_locally', + '--set-stream', 'platform:f28', + "--file", staged_data_filename('testmodule-local-build.yaml') + ] # build_module_locally changes database uri to a local SQLite database file. # Restore the uri to original one in order to not impact the database # session in subsequent tests. original_db_uri = app.config['SQLALCHEMY_DATABASE_URI'] try: - build_module_locally( - yaml_file="./fake.yaml", default_streams=["platform:el8"], stream="foo") + # The local sqlite3 database created by function + # build_module_locally is useless for this test. Mock the + # db.create_call to stop to create that database file. + with patch('module_build_service.manage.db.create_all'): + with patch('sys.argv', new=cli_cmd): + manager_wrapper() finally: app.config['SQLALCHEMY_DATABASE_URI'] = original_db_uri - submit_module_build_from_yaml.assert_called_once_with( - mock_db_session, ANY, ANY, { - "default_streams": {"platform": "el8"}, "local_build": True - }, - skiptests=False, stream="foo") + # Since module_build_service.scheduler.main is mocked, MBS does not + # really build the testmodule for this test. Following lines assert the + # fact: + # Module testmodule-local-build is expanded and stored into database, + # and this build has buildrequires platform:f28 and requires + # platform:f28. + # Please note that, the f28 is specified from command line option + # --set-stream, which is the point this test tests. + + builds = db_session.query(models.ModuleBuild).filter_by( + name='testmodule-local-build').all() + assert 1 == len(builds) + + testmodule_build = builds[0] + mmd_deps = testmodule_build.mmd().get_dependencies() + + deps_dict = deps_to_dict(mmd_deps[0], "buildtime") + assert ["f28"] == deps_dict["platform"] + deps_dict = deps_to_dict(mmd_deps[0], "runtime") + assert ["f28"] == deps_dict["platform"]