From 3c10906d5a5ea5f0b2cd49691edf1c74404f59e1 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Aug 19 2018 05:52:40 +0000 Subject: Fix tests for greenwave policy validation The problem was found in test_not_allow_to_build_if_gating_yaml_is_invalid originally, which does not have objects mocked properly, that causes expected error is not raised. This patch fixes tests relative to gating.yaml validation by having object mocked correctly and mocking request.post to cover more code path to ensure the code works as expected. Additionally, a new config file is added to fixtures directory which has rpkg.greenwave section added for testing gating.yaml validation specifically. And, test case TestBuildPackage is updated to create test repositories only once to make tests faster. Signed-off-by: Chenxiong Qi --- diff --git a/tests/fixtures/rpkg-greenwave.conf b/tests/fixtures/rpkg-greenwave.conf new file mode 100644 index 0000000..55069d5 --- /dev/null +++ b/tests/fixtures/rpkg-greenwave.conf @@ -0,0 +1,22 @@ +[rpkg] +lookaside = http://localhost/repo/pkgs +lookasidehash = md5 +lookaside_cgi = https://localhost/repo/pkgs/upload.cgi +gitbaseurl = ssh://%(user)s@localhost/%(repo)s +anongiturl = git://localhost/%(repo)s +branchre = f\d$|f\d\d$|el\d$|olpc\d$|master$ +kojiprofile = koji +build_client = koji +clone_config = + bz.default-component %(repo)s + +[rpkg.mbs] +auth_method = oidc +api_url = https://mbs.fedoraproject.org/module-build-service/ +oidc_id_provider = https://id.fedoraproject.org/openidc/ +oidc_client_id = mbs-authorizer +oidc_client_secret = notsecret +oidc_scopes = openid,https://id.fedoraproject.org/scope/groups,https://mbs.fedoraproject.org/oidc/submit-build + +[rpkg.greenwave] +url = http://greenwave.localhost/ \ No newline at end of file diff --git a/tests/test_cli.py b/tests/test_cli.py index a95736b..0b35694 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -18,6 +18,7 @@ except ImportError: from six.moves import configparser from six.moves import StringIO +from six.moves import http_client import git import pyrpkg.cli @@ -2707,10 +2708,14 @@ class TestOptionNameAndNamespace(CliTestCase): class TestBuildPackage(CliTestCase): """Test build package, common build, scratch build and chain build""" + create_repo_per_test = False UNIQUE_PATH_REGEX = r'^cli-build/\d+\.\d+\.[a-zA-Z]+$' @classmethod def setUpClass(cls): + super(TestBuildPackage, cls).setUpClass() + cls.checkout_branch(git.Repo(cls.cloned_repo_path), 'rhel-7') + fake_koji_config = dict( authtype='kerberos', server='http://localhost/kojihub', @@ -2733,10 +2738,10 @@ class TestBuildPackage(CliTestCase): cls.has_krb_creds_p.stop() cls.load_krb_user_p.stop() cls.read_config_p.stop() + super(TestBuildPackage, cls).tearDownClass() def setUp(self): super(TestBuildPackage, self).setUp() - self.checkout_branch(git.Repo(self.cloned_repo_path), 'rhel-7') self.ClientSession_p = patch('koji.ClientSession') self.mock_ClientSession = self.ClientSession_p.start() @@ -2768,12 +2773,27 @@ class TestBuildPackage(CliTestCase): session.build.return_value = 1000 session.chainBuild.return_value = 2000 + # Write gating.yaml for running tests that test valdation on this file + # for greenwave. + self.gating_yaml_content = 'abc' + with open(os.path.join(self.cloned_repo_path, 'gating.yaml'), 'w') as f: + f.write(self.gating_yaml_content) + def tearDown(self): self.ClientSession_p.stop() super(TestBuildPackage, self).tearDown() + # Some tests might make changes in the repository for their test + # purpose. These changes must be cleaned up in order to not impact + # others to run. + repo = git.Repo(self.cloned_repo_path) + if repo.is_dirty(): + self.run_cmd(['git', 'reset', 'HEAD', 'hello.py'], + cwd=self.cloned_repo_path) + def assert_build(self, sub_command, cli_opts=[], - expected_chain_urls=None, expected_opts={}): + expected_chain_urls=None, expected_opts={}, + config_file=None): session = self.mock_ClientSession.return_value cli_cmd = [ @@ -2785,7 +2805,7 @@ class TestBuildPackage(CliTestCase): with patch('koji_cli.lib.watch_tasks') as watch_tasks: with patch('sys.argv', new=cli_cmd): - cli = self.new_cli() + cli = self.new_cli(cfg=config_file) if sub_command == 'build': mock_build_api = session.build cli.build() @@ -3040,23 +3060,55 @@ class TestBuildPackage(CliTestCase): 'git://localhost/docpkg#45678'], ]) - @patch('pyrpkg.cli.cliClient.greenwave_validation_gating') - def test_not_allow_to_build_if_gating_yaml_is_invalid(self, greenwave_response): - greenwave_response.return_value = {'status_code': 500} - self.assert_build('build') - session = self.mock_ClientSession.return_value - session.build.assert_not_called() + @patch('requests.post') + def test_not_allow_to_build_if_gating_yaml_is_invalid(self, post): + response = Mock(status_code=http_client.BAD_REQUEST) + response.json.return_value = {'message': 'gating policy is invalid'} + post.return_value = response - @patch('pyrpkg.cli.cliClient.greenwave_validation_gating') - def test_allowed_to_build_if_skipped_gating_yaml_check(self, greenwave_response): - greenwave_response.return_value = {'status_code': 500} - self.assert_build('build', cli_opts=['--skip-remote-rules-validation']) - session = self.mock_ClientSession.return_value - session.build.assert_called_once() + six.assertRaisesRegex( + self, rpkgError, 'but it is not valid', + self.assert_build, 'build', + config_file=os.path.join(fixtures_dir, 'rpkg-greenwave.conf')) + + @patch('requests.post') + def test_not_build_if_greenwave_has_internal_error(self, post): + response = Mock(status_code=http_client.INTERNAL_SERVER_ERROR) + response.json.return_value = {'message': 'internal error'} + post.return_value = response + + six.assertRaisesRegex( + self, rpkgError, 'for an unknown problem', + self.assert_build, 'build', + config_file=os.path.join(fixtures_dir, 'rpkg-greenwave.conf')) @patch('pyrpkg.cli.cliClient.greenwave_validation_gating') - def test_allowed_to_build_if_gating_yaml_is_correct(self, greenwave_response): - greenwave_response.return_value = {'status_code': 200} + def test_skip_gating_policy_validation_if_greenwave_is_not_set( + self, greenwave_validation_gating): self.assert_build('build') + greenwave_validation_gating.assert_not_called() + + def test_allowed_to_build_if_skipped_gating_yaml_check(self): + self.assert_build( + 'build', + cli_opts=['--skip-remote-rules-validation'], + config_file=os.path.join(fixtures_dir, 'rpkg-greenwave.conf')) + session = self.mock_ClientSession.return_value session.build.assert_called_once() + + @patch('requests.post') + def test_allowed_to_build_if_gating_yaml_is_correct(self, post): + response = Mock(status_code=http_client.OK) + response.json.return_value = {'message': 'gating policy is ok'} + post.return_value = response + + cli = self.assert_build( + 'build', + config_file=os.path.join(fixtures_dir, 'rpkg-greenwave.conf')) + + post.assert_called_once_with( + '{0}/{1}'.format(cli.config.get('rpkg.greenwave', 'url'), + 'api/v1.0/validate-gating-yaml'), + data=six.b(self.gating_yaml_content), + timeout=30) diff --git a/tests/utils.py b/tests/utils.py index 7913684..0af45c0 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -241,7 +241,8 @@ class CommandTestCase(RepoCreationMixin, Assertions, Utils, unittest.TestCase): kojiconfig, build_client, user=user, dist=dist, target=target, quiet=quiet) - def checkout_branch(self, repo, branch_name): + @staticmethod + def checkout_branch(repo, branch_name): """Checkout to a local branch :param git.Repo repo: `git.Repo` instance represents a git repository