From b08a9796e2e2904f7e3fe61ef42c37a212640f39 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Aug 10 2018 06:29:49 +0000 Subject: Reduce the number of repo creation for tests For writting further tests, developer has to consider if the fake repo for test case should be created only once, or each test requires fake repos. The creation could be controlled by two class variables. * require_test_repos: indicate if test case requires fake repos. * create_repo_per_test: indicate if fake repos should be created for each test or only the test case. Signed-off-by: Chenxiong Qi --- diff --git a/tests/test_cli.py b/tests/test_cli.py index 5b7583f..2b43d14 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -2,6 +2,7 @@ import hashlib import logging +import glob import os import shutil import six @@ -93,6 +94,8 @@ class CliTestCase(CommandTestCase): class TestModuleNameOption(CliTestCase): + create_repo_per_test = False + def setUp(self): super(TestModuleNameOption, self).setUp() self.conf_file = self.get_absolute_conf_filename('rpkg-ns.conf') @@ -135,6 +138,8 @@ class TestKojiConfigBackwardCompatibility(CliTestCase): Remove this test case after deprecated kojiconfig is removed eventually. """ + create_repo_per_test = False + @patch('pyrpkg.Commands._deprecated_read_koji_config') @patch('pyrpkg.koji.read_config') def test_use_deprecated_kojiconfig(self, @@ -179,6 +184,8 @@ class TestKojiConfigBackwardCompatibility(CliTestCase): class TestContainerBuildWithKoji(CliTestCase): """Test container_build with koji""" + create_repo_per_test = False + def setUp(self): super(TestContainerBuildWithKoji, self).setUp() self.checkout_branch(git.Repo(self.cloned_repo_path), 'eng-rhel-7') @@ -298,6 +305,8 @@ class TestContainerBuildWithKoji(CliTestCase): class TestClog(CliTestCase): + create_repo_per_test = False + def setUp(self): super(TestClog, self).setUp() self.checkout_branch(git.Repo(self.cloned_repo_path), 'eng-rhel-6') @@ -483,6 +492,8 @@ class TestSrpm(CliTestCase): class TestCompile(CliTestCase): + create_repo_per_test = False + @patch('pyrpkg.Commands._run_command') def test_compile(self, _run_command): cli_cmd = ['rpkg', '--path', self.cloned_repo_path, '--release', 'rhel-6', 'compile'] @@ -517,6 +528,8 @@ class TestCompile(CliTestCase): class TestPrep(CliTestCase): + create_repo_per_test = False + @patch('pyrpkg.Commands._run_command') def test_prep(self, _run_command): cli_cmd = ['rpkg', '--path', self.cloned_repo_path, '--release', 'rhel-6', 'prep'] @@ -549,6 +562,8 @@ class TestPrep(CliTestCase): class TestInstall(CliTestCase): + create_repo_per_test = False + @patch('pyrpkg.Commands._run_command') def test_install(self, _run_command): cli_cmd = ['rpkg', '--path', self.cloned_repo_path, '--release', 'rhel-6', 'install'] @@ -583,6 +598,8 @@ class TestInstall(CliTestCase): class TestLocal(CliTestCase): + create_repo_per_test = False + @patch('pyrpkg.subprocess.check_call') def test_local(self, check_call): cli_cmd = ['rpkg', '--path', self.cloned_repo_path, '--release', 'rhel-6', 'local'] @@ -625,6 +642,8 @@ class TestLocal(CliTestCase): class TestVerifyFiles(CliTestCase): + create_repo_per_test = False + @patch('pyrpkg.Commands._run_command') def test_verify_files(self, _run_command): cli_cmd = ['rpkg', '--path', self.cloned_repo_path, '--release', 'rhel-6', 'verify-files'] @@ -656,6 +675,8 @@ class TestVerifyFiles(CliTestCase): class TestVerrel(CliTestCase): + create_repo_per_test = False + @patch('sys.stdout', new=StringIO()) def test_verrel_get_repo_name_from_spec(self): cli_cmd = ['rpkg', '--path', self.repo_path, '--release', 'rhel-6', 'verrel'] @@ -1316,6 +1337,8 @@ class TestImportSrpm(LookasideCacheMock, CliTestCase): class TestMockbuild(CliTestCase): """Test mockbuild command""" + create_repo_per_test = False + def setUp(self): super(TestMockbuild, self).setUp() self.run_command_patcher = patch('pyrpkg.Commands._run_command') @@ -1400,6 +1423,8 @@ class TestMockbuild(CliTestCase): class TestCoprBuild(CliTestCase): """Test copr command""" + create_repo_per_test = False + def setUp(self): super(TestCoprBuild, self).setUp() self.nvr_patcher = patch('pyrpkg.Commands.nvr', @@ -1460,6 +1485,8 @@ class TestCoprBuild(CliTestCase): class TestMockConfig(CliTestCase): """Test mockconfig command""" + create_repo_per_test = False + def setUp(self): super(TestMockConfig, self).setUp() @@ -1664,6 +1691,8 @@ enabled = false''', repo_config) class TestPatch(CliTestCase): """Test patch command""" + create_repo_per_test = False + def setUp(self): super(TestPatch, self).setUp() @@ -1805,6 +1834,8 @@ class TestPatch(CliTestCase): class TestModulesCli(CliTestCase): """Test module commands""" + create_repo_per_test = False + scopes = [ 'openid', 'https://id.fedoraproject.org/scope/groups', @@ -1855,6 +1886,15 @@ class TestModulesCli(CliTestCase): 'version': '20171010145511' } + def tearDown(self): + super(TestModulesCli, self).tearDown() + + # Test may generate yaml file for itself to run the test. + # Clean it for next test to run. + files_pattern = os.path.join(self.cloned_repo_path, '*.yaml') + for filename in glob.glob(files_pattern): + os.unlink(filename) + @patch('sys.stdout', new=StringIO()) @patch('requests.get') @patch('openidc_client.OpenIDCClient.send_request') @@ -2546,6 +2586,8 @@ State: failed class TestOptionNameAndNamespace(CliTestCase): """Test CLI option --name and --namespace""" + create_repo_per_test = False + @patch('sys.stderr', new_callable=six.StringIO) def test_name_exclusive_with_module_name(self, stderr): cli = [ diff --git a/tests/test_commands.py b/tests/test_commands.py index 579660d..d7b20cc 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -71,6 +71,8 @@ def mock_load_branch_merge(fake_branch_merge): class LoadNameVerRelTest(CommandTestCase): """Test case for Commands.load_nameverrel""" + create_repo_per_test = False + def setUp(self): super(LoadNameVerRelTest, self).setUp() self.cmd = self.make_commands() @@ -151,6 +153,8 @@ class LoadNameVerRelTest(CommandTestCase): class LoadBranchMergeTest(CommandTestCase): """Test case for testing Commands.load_branch_merge""" + create_repo_per_test = False + def setUp(self): super(LoadBranchMergeTest, self).setUp() @@ -203,6 +207,8 @@ class LoadBranchMergeTest(CommandTestCase): class LoadRPMDefinesTest(CommandTestCase): """Test case for Commands.load_rpmdefines""" + create_repo_per_test = False + def setUp(self): super(LoadRPMDefinesTest, self).setUp() self.cmd = self.make_commands() @@ -326,6 +332,8 @@ class CheckRepoWithOrWithoutDistOptionCase(CommandTestCase): class ClogTest(CommandTestCase): + create_repo_per_test = False + def setUp(self): super(ClogTest, self).setUp() @@ -375,6 +383,8 @@ $what_is_this class TestProperties(CommandTestCase): + create_repo_per_test = False + def setUp(self): super(TestProperties, self).setUp() self.invalid_repo = tempfile.mkdtemp() @@ -481,8 +491,10 @@ class TestProperties(CommandTestCase): class TestNamespaced(CommandTestCase): + require_test_repos = False + def test_get_namespace_giturl(self): - cmd = self.make_commands() + cmd = self.make_commands(path='/path/to/repo') cmd.gitbaseurl = 'ssh://%(user)s@localhost/%(module)s' cmd.distgit_namespaced = False @@ -494,7 +506,7 @@ class TestNamespaced(CommandTestCase): cmd._get_namespace_giturl('rpms/docpkg')) def test_get_namespace_giturl_namespaced_is_enabled(self): - cmd = self.make_commands() + cmd = self.make_commands(path='/path/to/repo') cmd.gitbaseurl = 'ssh://%(user)s@localhost/%(module)s' cmd.distgit_namespaced = True @@ -506,7 +518,7 @@ class TestNamespaced(CommandTestCase): cmd._get_namespace_giturl('rpms/docpkg')) def test_get_namespace_anongiturl(self): - cmd = self.make_commands() + cmd = self.make_commands(path='/path/to/repo') cmd.anongiturl = 'git://localhost/%(module)s' cmd.distgit_namespaced = False @@ -518,7 +530,7 @@ class TestNamespaced(CommandTestCase): cmd._get_namespace_anongiturl('rpms/docpkg')) def test_get_namespace_anongiturl_namespaced_is_enabled(self): - cmd = self.make_commands() + cmd = self.make_commands(path='/path/to/repo') cmd.anongiturl = 'git://localhost/%(module)s' cmd.distgit_namespaced = True @@ -605,10 +617,12 @@ class TestLoadRepoNameFromSpecialPushURL(CommandTestCase): class TestLoginKojiSession(CommandTestCase): """Test login_koji_session""" + require_test_repos = False + def setUp(self): super(TestLoginKojiSession, self).setUp() - self.cmd = self.make_commands() + self.cmd = self.make_commands(path='/path/to/repo') self.cmd.log = Mock() self.koji_config = { 'authtype': 'ssl', @@ -695,17 +709,21 @@ class TestLoginKojiSession(CommandTestCase): class TestConstructBuildURL(CommandTestCase): """Test Commands.construct_build_url""" + require_test_repos = False + + def setUp(self): + super(TestConstructBuildURL, self).setUp() + self.cmd = self.make_commands(path='/path/to/repo') + @patch('pyrpkg.Commands.ns_repo_name', new_callable=PropertyMock) @patch('pyrpkg.Commands.commithash', new_callable=PropertyMock) def test_construct_url(self, commithash, ns_repo_name): commithash.return_value = '12345' ns_repo_name.return_value = 'container/fedpkg' - cmd = self.make_commands() - anongiturl = 'https://src.example.com/%(repo)s' - with patch.object(cmd, 'anongiturl', new=anongiturl): - url = cmd.construct_build_url() + with patch.object(self.cmd, 'anongiturl', new=anongiturl): + url = self.cmd.construct_build_url() expected_url = '{0}#{1}'.format( anongiturl % {'repo': ns_repo_name.return_value}, @@ -713,12 +731,10 @@ class TestConstructBuildURL(CommandTestCase): self.assertEqual(expected_url, url) def test_construct_with_given_repo_name_and_hash(self): - cmd = self.make_commands() - anongiturl = 'https://src.example.com/%(repo)s' - with patch.object(cmd, 'anongiturl', new=anongiturl): + with patch.object(self.cmd, 'anongiturl', new=anongiturl): for repo_name in ('extra-cmake-modules', 'rpms/kf5-kfilemetadata'): - url = cmd.construct_build_url(repo_name, '123456') + url = self.cmd.construct_build_url(repo_name, '123456') expected_url = '{0}#{1}'.format( anongiturl % {'repo': repo_name}, '123456') @@ -728,9 +744,12 @@ class TestConstructBuildURL(CommandTestCase): class TestCleanupTmpDir(CommandTestCase): """Test Commands._cleanup_tmp_dir for mockbuild command""" + require_test_repos = False + def setUp(self): super(TestCleanupTmpDir, self).setUp() self.tmp_dir_name = tempfile.mkdtemp(prefix='test-cleanup-tmp-dir-') + self.cmd = self.make_commands(path='/path/to/repo') def tearDown(self): if os.path.exists(self.tmp_dir_name): @@ -739,38 +758,36 @@ class TestCleanupTmpDir(CommandTestCase): @patch('shutil.rmtree') def test_do_nothing_is_tmp_dir_is_invalid(self, rmtree): - cmd = self.make_commands() for invalid_dir in ('', None): - cmd._cleanup_tmp_dir(invalid_dir) + self.cmd._cleanup_tmp_dir(invalid_dir) rmtree.assert_not_called() def test_remove_tmp_dir(self): - cmd = self.make_commands() - cmd._cleanup_tmp_dir(self.tmp_dir_name) + self.cmd._cleanup_tmp_dir(self.tmp_dir_name) self.assertFalse(os.path.exists(self.tmp_dir_name)) def test_keep_silient_if_tmp_dir_does_not_exist(self): - cmd = self.make_commands() tmp_dir = tempfile.mkdtemp() os.rmdir(tmp_dir) - cmd._cleanup_tmp_dir(tmp_dir) + self.cmd._cleanup_tmp_dir(tmp_dir) def test_raise_error_if_other_non_no_such_file_dir_error(self): - cmd = self.make_commands() with patch('shutil.rmtree', side_effect=OSError((errno.EEXIST), 'error message')): - self.assertRaises(rpkgError, cmd._cleanup_tmp_dir, '/tmp/dir') + self.assertRaises(rpkgError, self.cmd._cleanup_tmp_dir, '/tmp/dir') class TestConfigMockConfigDir(CommandTestCase): """Test Commands._config_dir_basic for mockbuild""" + require_test_repos = False + def setUp(self): super(TestConfigMockConfigDir, self).setUp() - self.cmd = self.make_commands() + self.cmd = self.make_commands(path='/path/to/repo') self.temp_config_dir = tempfile.mkdtemp() self.fake_root = 'fedora-26-x86_64' @@ -862,11 +879,16 @@ class TestConfigMockConfigDir(CommandTestCase): class TestConfigMockConfigDirWithNecessaryFiles(CommandTestCase): """Test Commands._config_dir_other""" + require_test_repos = False + + def setUp(self): + super(TestConfigMockConfigDirWithNecessaryFiles, self).setUp() + self.cmd = self.make_commands(path='/path/to/repo') + @patch('shutil.copy2') @patch('os.path.exists', return_value=True) def test_copy_cfg_files_from_etc_mock_dir(self, exists, copy2): - cmd = self.make_commands() - cmd._config_dir_other('/path/to/config-dir') + self.cmd._config_dir_other('/path/to/config-dir') exists.assert_has_calls([ call('/etc/mock/site-defaults.cfg'), @@ -881,10 +903,8 @@ class TestConfigMockConfigDirWithNecessaryFiles(CommandTestCase): @patch('os.path.exists', return_value=False) def test_create_empty_cfg_files_if_not_exist_in_system_mock(self, exists): - cmd = self.make_commands() - with patch.object(six.moves.builtins, 'open', mock_open()) as m: - cmd._config_dir_other('/path/to/config-dir') + self.cmd._config_dir_other('/path/to/config-dir') m.assert_has_calls([ call('/path/to/config-dir/site-defaults.cfg', 'w'), @@ -903,18 +923,15 @@ class TestConfigMockConfigDirWithNecessaryFiles(CommandTestCase): def test_fail_when_copy_cfg_file(self, exists, copy2): copy2.side_effect = OSError - cmd = self.make_commands() - self.assertRaises(rpkgError, - cmd._config_dir_other, '/path/to/config-dir') + self.assertRaises( + rpkgError, self.cmd._config_dir_other, '/path/to/config-dir') @patch('os.path.exists', return_value=False) def test_fail_if_error_when_write_empty_cfg_files(self, exists): - cmd = self.make_commands() - with patch.object(six.moves.builtins, 'open', mock_open()) as m: m.side_effect = IOError - self.assertRaises(rpkgError, - cmd._config_dir_other, '/path/to/config-dir') + self.assertRaises( + rpkgError, self.cmd._config_dir_other, '/path/to/config-dir') class TestLint(CommandTestCase): diff --git a/tests/utils.py b/tests/utils.py index a428731..7913684 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -112,7 +112,8 @@ class Assertions(object): class Utils(object): - def run_cmd(self, cmd, **kwargs): + @staticmethod + def run_cmd(cmd, **kwargs): returncode = subprocess.call(cmd, **kwargs) if returncode != 0: raise RuntimeError('Command fails. Command: %s. Return code %d' % ( @@ -142,13 +143,14 @@ class Utils(object): class RepoCreationMixin(object): """Provide methods to create and destroy git repositories for tests""" - def _create_test_repos(self): - self.repo_path = tempfile.mkdtemp(prefix='rpkg-commands-tests-') + @classmethod + def create_fake_repos(cls): + cls.repo_path = tempfile.mkdtemp(prefix='rpkg-commands-tests-') - self.spec_file = 'docpkg.spec' + cls.spec_file = 'docpkg.spec' # Add spec file to this repo and commit - spec_file_path = os.path.join(self.repo_path, self.spec_file) + spec_file_path = os.path.join(cls.repo_path, cls.spec_file) with open(spec_file_path, 'w') as f: f.write(spec_file) @@ -166,13 +168,13 @@ class RepoCreationMixin(object): ['git', 'branch', 'rhel-7'], ] for cmd in git_cmds: - self.run_cmd(cmd, cwd=self.repo_path, - stdout=subprocess.PIPE, stderr=subprocess.PIPE) + cls.run_cmd(cmd, cwd=cls.repo_path, + stdout=subprocess.PIPE, stderr=subprocess.PIPE) # Clone the repo - self.cloned_repo_path = tempfile.mkdtemp(prefix='rpkg-commands-tests-cloned-') - self.run_cmd(['git', 'clone', self.repo_path, self.cloned_repo_path], - stdout=subprocess.PIPE, stderr=subprocess.PIPE) + cls.cloned_repo_path = tempfile.mkdtemp(prefix='rpkg-commands-tests-cloned-') + cls.run_cmd(['git', 'clone', cls.repo_path, cls.cloned_repo_path], + stdout=subprocess.PIPE, stderr=subprocess.PIPE) git_cmds = [ ['git', 'config', 'user.email', 'tester@example.com'], ['git', 'config', 'user.name', 'tester'], @@ -182,25 +184,37 @@ class RepoCreationMixin(object): ['git', 'branch', '--track', 'rhel-7', 'origin/rhel-7'], ] for cmd in git_cmds: - self.run_cmd(cmd, cwd=self.cloned_repo_path, - stdout=subprocess.PIPE, stderr=subprocess.PIPE) + cls.run_cmd(cmd, cwd=cls.cloned_repo_path, + stdout=subprocess.PIPE, stderr=subprocess.PIPE) - def _destroy_test_repos(self): - shutil.rmtree(self.repo_path) - shutil.rmtree(self.cloned_repo_path) + @classmethod + def destroy_fake_repos(cls): + shutil.rmtree(cls.repo_path) + shutil.rmtree(cls.cloned_repo_path) class CommandTestCase(RepoCreationMixin, Assertions, Utils, unittest.TestCase): + create_repo_per_test = True require_test_repos = True + @classmethod + def setUpClass(cls): + if cls.require_test_repos and not cls.create_repo_per_test: + cls.create_fake_repos() + + @classmethod + def tearDownClass(cls): + if cls.require_test_repos and not cls.create_repo_per_test: + cls.destroy_fake_repos() + def setUp(self): - if self.require_test_repos: - self._create_test_repos() + if self.require_test_repos and self.create_repo_per_test: + self.create_fake_repos() def tearDown(self): - if self.require_test_repos: - self._destroy_test_repos() + if self.require_test_repos and self.create_repo_per_test: + self.destroy_fake_repos() def make_commands(self, path=None, user=None, dist=None, target=None, quiet=None): """Helper method for creating Commands object for test cases