From 932f0d89a98e1aa0bd0ad9669f60137303a05dda Mon Sep 17 00:00:00 2001 From: Dan Mick Date: Fri, 27 May 2016 15:11:30 -0700 Subject: [PATCH] test_describe_tests: remove 'manual test stubs' Use a combined strategy: for test classes, use setup to manage the list of patchers, also starting them in setup because all the class-based tests use the same fake fs data. For the file-scope test functions, use @patch to manage them, because the filesystem data changes with each test. Signed-off-by: Dan Mick --- teuthology/describe_tests.py | 25 +++-- teuthology/test/test_describe_tests.py | 122 ++++++++++++------------- 2 files changed, 72 insertions(+), 75 deletions(-) diff --git a/teuthology/describe_tests.py b/teuthology/describe_tests.py index 5b77fbd142..10206a67af 100644 --- a/teuthology/describe_tests.py +++ b/teuthology/describe_tests.py @@ -71,8 +71,7 @@ def output_results(headers, rows, output_format, hrule): def get_combinations(suite_dir, fields, subset, limit, filter_in, filter_out, - include_facet, _isdir=os.path.isdir, _open=open, - _isfile=os.path.isfile, _listdir=os.listdir): + include_facet): """ Describes the combinations of a suite, optionally limiting or filtering output based on the given parameters. Includes @@ -82,7 +81,7 @@ def get_combinations(suite_dir, fields, subset, of strings. """ configs = [(combine_path(suite_dir, item[0]), item[1]) for item in - build_matrix(suite_dir, _isfile, _isdir, _listdir, subset)] + build_matrix(suite_dir, subset)] num_listed = 0 rows = [] @@ -101,7 +100,7 @@ def get_combinations(suite_dir, fields, subset, for path in fragment_paths]): continue - fragment_fields = [extract_info(path, fields, _isdir, _open) + fragment_fields = [extract_info(path, fields) for path in fragment_paths] # merge fields from multiple fragments by joining their values with \n @@ -173,7 +172,7 @@ def describe_suite(suite_dir, fields, include_facet, output_format): return headers + fields, rows -def extract_info(file_name, fields, _isdir=os.path.isdir, _open=open): +def extract_info(file_name, fields): """ Read a yaml file and return a dictionary mapping the fields to the values of those fields in the file. @@ -195,10 +194,10 @@ def extract_info(file_name, fields, _isdir=os.path.isdir, _open=open): message and raises ParseError. """ empty_result = {f: '' for f in fields} - if _isdir(file_name) or not file_name.endswith('.yaml'): + if os.path.isdir(file_name) or not file_name.endswith('.yaml'): return empty_result - with _open(file_name, 'r') as f: + with open(file_name, 'r') as f: parsed = yaml.load(f) if not isinstance(parsed, dict): @@ -229,8 +228,7 @@ def path_relative_to_suites(path): def tree_with_info(cur_dir, fields, include_facet, prefix, rows, - _listdir=os.listdir, _isdir=os.path.isdir, - _open=open, output_format='plain'): + output_format='plain'): """ Gather fields from all files and directories in cur_dir. Returns a list of strings for each path containing: @@ -241,7 +239,7 @@ def tree_with_info(cur_dir, fields, include_facet, prefix, rows, 3) the values of the provided fields in the path ('' is used for missing values) in the same order as the provided fields """ - files = sorted(_listdir(cur_dir)) + files = sorted(os.listdir(cur_dir)) has_yamls = any([x.endswith('.yaml') for x in files]) facet = os.path.basename(cur_dir) if has_yamls else '' for i, f in enumerate(files): @@ -252,7 +250,7 @@ def tree_with_info(cur_dir, fields, include_facet, prefix, rows, else: file_pad = '├── ' dir_pad = '│ ' - info = extract_info(path, fields, _isdir, _open) + info = extract_info(path, fields) tree_node = prefix + file_pad + f if output_format != 'plain': tree_node = path_relative_to_suites(path) @@ -261,8 +259,7 @@ def tree_with_info(cur_dir, fields, include_facet, prefix, rows, if include_facet: row.append(facet) rows.append(row + meta) - if _isdir(path): + if os.path.isdir(path): tree_with_info(path, fields, include_facet, - prefix + dir_pad, rows, - _listdir, _isdir, _open, output_format) + prefix + dir_pad, rows, output_format) return rows diff --git a/teuthology/test/test_describe_tests.py b/teuthology/test/test_describe_tests.py index 19cb11e92c..84e3dbfe6a 100644 --- a/teuthology/test/test_describe_tests.py +++ b/teuthology/test/test_describe_tests.py @@ -5,6 +5,7 @@ from fake_fs import make_fake_fstools from teuthology.describe_tests import (tree_with_info, extract_info, get_combinations) from teuthology.exceptions import ParseError +from mock import MagicMock, patch realistic_fs = { 'basic': { @@ -105,9 +106,33 @@ expected_rbd_features = [ class TestDescribeTests(object): + patchpoints = [ + 'os.path.exists', + 'os.listdir', + 'os.path.isfile', + 'os.path.isdir', + '__builtin__.open', + ] + fake_fns = make_fake_fstools(realistic_fs) + def setup(self): - self.fake_exists, self.fake_listdir, self.fake_isfile, - self.fake_isdir, self.fake_open = make_fake_fstools(realistic_fs) + self.mocks = dict() + self.patchers = dict() + klass = self.__class__ + for ppoint, fn in zip(klass.patchpoints, klass.fake_fns): + mockobj = MagicMock() + patcher = patch(ppoint, mockobj) + mockobj.side_effect = fn + patcher.start() + self.mocks[ppoint] = mockobj + self.patchers[ppoint] = patcher + + def stop_patchers(self): + for patcher in self.patchers.values(): + patcher.stop() + + def teardown(self): + self.stop_patchers() @staticmethod def assert_expected_combo_headers(headers): @@ -115,94 +140,65 @@ class TestDescribeTests(object): sorted(set(filter(bool, expected_facets)))) def test_no_filters(self): - rows = tree_with_info('basic', [], False, '', [], - self.fake_listdir, self.fake_isdir, - self.fake_open) + rows = tree_with_info('basic', [], False, '', []) assert rows == [[x] for x in expected_tree] def test_single_filter(self): - rows = tree_with_info('basic', ['desc'], False, '', [], - self.fake_listdir, self.fake_isdir, - self.fake_open) + rows = tree_with_info('basic', ['desc'], False, '', []) assert rows == map(list, zip(expected_tree, expected_desc)) - rows = tree_with_info('basic', ['rbd_features'], False, '', [], - self.fake_listdir, self.fake_isdir, - self.fake_open) + rows = tree_with_info('basic', ['rbd_features'], False, '', []) assert rows == map(list, zip(expected_tree, expected_rbd_features)) def test_single_filter_with_facets(self): - rows = tree_with_info('basic', ['desc'], True, '', [], - self.fake_listdir, self.fake_isdir, - self.fake_open) + rows = tree_with_info('basic', ['desc'], True, '', []) assert rows == map(list, zip(expected_tree, expected_facets, expected_desc)) - rows = tree_with_info('basic', ['rbd_features'], True, '', [], - self.fake_listdir, self.fake_isdir, - self.fake_open) + rows = tree_with_info('basic', ['rbd_features'], True, '', []) assert rows == map(list, zip(expected_tree, expected_facets, expected_rbd_features)) def test_no_matching(self): - rows = tree_with_info('basic', ['extra'], False, '', [], - self.fake_listdir, self.fake_isdir, - self.fake_open) + rows = tree_with_info('basic', ['extra'], False, '', []) assert rows == map(list, zip(expected_tree, [''] * len(expected_tree))) - rows = tree_with_info('basic', ['extra'], True, '', [], - self.fake_listdir, self.fake_isdir, - self.fake_open) + rows = tree_with_info('basic', ['extra'], True, '', []) assert rows == map(list, zip(expected_tree, expected_facets, [''] * len(expected_tree))) def test_multiple_filters(self): - rows = tree_with_info('basic', ['desc', 'rbd_features'], False, - '', [], self.fake_listdir, - self.fake_isdir, self.fake_open) + rows = tree_with_info('basic', ['desc', 'rbd_features'], False, '', []) assert rows == map(list, zip(expected_tree, expected_desc, expected_rbd_features)) - rows = tree_with_info('basic', ['rbd_features', 'desc'], False, - '', [], self.fake_listdir, - self.fake_isdir, self.fake_open) + rows = tree_with_info('basic', ['rbd_features', 'desc'], False, '', []) assert rows == map(list, zip(expected_tree, expected_rbd_features, expected_desc)) def test_multiple_filters_with_facets(self): - rows = tree_with_info('basic', ['desc', 'rbd_features'], True, - '', [], self.fake_listdir, - self.fake_isdir, self.fake_open) + rows = tree_with_info('basic', ['desc', 'rbd_features'], True, '', []) assert rows == map(list, zip(expected_tree, expected_facets, expected_desc, expected_rbd_features)) - rows = tree_with_info('basic', ['rbd_features', 'desc'], True, - '', [], self.fake_listdir, - self.fake_isdir, self.fake_open) + rows = tree_with_info('basic', ['rbd_features', 'desc'], True, '', []) assert rows == map(list, zip(expected_tree, expected_facets, expected_rbd_features, expected_desc)) def test_combinations_only_facets(self): - headers, rows = get_combinations('basic', [], None, 1, None, - None, True, - self.fake_isdir, self.fake_open, - self.fake_isfile, - self.fake_listdir) + headers, rows = get_combinations('basic', [], None, 1, None, None, True) self.assert_expected_combo_headers(headers) assert rows == [['basic', 'install', 'fixed-1', 'rbd_api_tests']] def test_combinations_desc_features(self): headers, rows = get_combinations('basic', ['desc', 'rbd_features'], - None, 1, None, None, False, - self.fake_isdir, self.fake_open, - self.fake_isfile, - self.fake_listdir) + None, 1, None, None, False) assert headers == ['desc', 'rbd_features'] descriptions = '\n'.join([ 'install ceph', @@ -213,39 +209,40 @@ class TestDescribeTests(object): def test_combinations_filter_in(self): headers, rows = get_combinations('basic', [], None, 0, ['old_format'], - None, True, - self.fake_isdir, self.fake_open, - self.fake_isfile, self.fake_listdir) + None, True) self.assert_expected_combo_headers(headers) assert rows == [['basic', 'install', 'fixed-1', 'rbd_api_tests_old_format']] def test_combinations_filter_out(self): headers, rows = get_combinations('basic', [], None, 0, None, - ['old_format'], True, - self.fake_isdir, self.fake_open, - self.fake_isfile, self.fake_listdir) + ['old_format'], True) self.assert_expected_combo_headers(headers) assert rows == [['basic', 'install', 'fixed-1', 'rbd_api_tests']] -def test_extract_info_dir(): +@patch('__builtin__.open') +@patch('os.path.isdir') +def test_extract_info_dir(m_isdir, m_open): simple_fs = {'a': {'b.yaml': 'meta: [{foo: c}]'}} - _, _, _, fake_isdir, fake_open = make_fake_fstools(simple_fs) - info = extract_info('a', [], fake_isdir, fake_open) + _, _, _, m_isdir.side_effect, m_open.side_effect = \ + make_fake_fstools(simple_fs) + info = extract_info('a', []) assert info == {} - info = extract_info('a', ['foo', 'bar'], fake_isdir, fake_open) + info = extract_info('a', ['foo', 'bar']) assert info == {'foo': '', 'bar': ''} - info = extract_info('a/b.yaml', ['foo', 'bar'], fake_isdir, fake_open) + info = extract_info('a/b.yaml', ['foo', 'bar']) assert info == {'foo': 'c', 'bar': ''} -def check_parse_error(fs): - _, _, _, fake_isdir, fake_open = make_fake_fstools(fs) +@patch('__builtin__.open') +@patch('os.path.isdir') +def check_parse_error(fs, m_isdir, m_open): + _, _, _, m_isdir.side_effect, m_open.side_effect = make_fake_fstools(fs) with pytest.raises(ParseError): - a = extract_info('a.yaml', ['a'], fake_isdir, fake_open) + a = extract_info('a.yaml', ['a']) raise Exception(str(a)) @@ -261,8 +258,11 @@ def test_extract_info_not_a_dict(): check_parse_error({'a.yaml': 'meta: [[a, b]]'}) -def test_extract_info_empty_file(): +@patch('__builtin__.open') +@patch('os.path.isdir') +def test_extract_info_empty_file(m_isdir, m_open): simple_fs = {'a.yaml': ''} - _, _, _, fake_isdir, fake_open = make_fake_fstools(simple_fs) - info = extract_info('a.yaml', [], fake_isdir, fake_open) + _, _, _, m_isdir.side_effect, m_open.side_effect = \ + make_fake_fstools(simple_fs) + info = extract_info('a.yaml', []) assert info == {} -- 2.39.5