From 04893053c7e1d3539bdace3a30735bf69742bb0a Mon Sep 17 00:00:00 2001 From: "Andrea Frittoli (andreaf)" Date: Mon, 15 Aug 2016 10:41:50 +0100 Subject: [PATCH] Remove the test_attr_prefix before injecting The test_attr_prefix allows filtering test attributes for injection into the test_metadata table. Such attributes only get a prefix for the sake of filtering, so the prefix should be removed before they are injected into the DB. This is a change in behaviour, but I believe it's safe to do so, since the CLI option was broken until now, which suggests no-one was using it really. Change-Id: I0cf3701a143f6fb71d03a0a7bdfb0a84c107ecbe --- ...ove-test-attr-prefix-dff9d378a5e940ab.yaml | 15 +++ subunit2sql/shell.py | 13 +- subunit2sql/tests/test_shell.py | 112 ++++++++++++++++++ 3 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/remove-test-attr-prefix-dff9d378a5e940ab.yaml diff --git a/releasenotes/notes/remove-test-attr-prefix-dff9d378a5e940ab.yaml b/releasenotes/notes/remove-test-attr-prefix-dff9d378a5e940ab.yaml new file mode 100644 index 0000000..4e7af3c --- /dev/null +++ b/releasenotes/notes/remove-test-attr-prefix-dff9d378a5e940ab.yaml @@ -0,0 +1,15 @@ +--- +features: + - A new CLI / configuration option remove_test_attr_prefix + is available to remove the test attribute prefix from test + attributes before they are added to the test metadata. + The CLI option defaults to False, so that the current + behavior is preserved. It is only applicable when used in + combination with test_attr_prefix. +upgrade: + - The test attribute prefix is removed from the + test attributes before they are injected into the DB, + when using the remove_test_attr_prefix config/CLI option. + If metadata was injected before, enabling + remove_test_attr_prefix may lead to test metadata + duplication, with and without prefix. diff --git a/subunit2sql/shell.py b/subunit2sql/shell.py index 02d876d..165426b 100644 --- a/subunit2sql/shell.py +++ b/subunit2sql/shell.py @@ -48,6 +48,10 @@ SHELL_OPTS = [ help='An optional prefix to identify global test attrs ' 'and treat it as test metadata instead of test_run ' 'metadata'), + cfg.BoolOpt('remove_test_attr_prefix', short='x', default=False, + help='When True, the prefix configured in "test_attr_prefix", ' + 'if any, is removed from the metadata before it\'s ' + 'added to the test metadata'), cfg.StrOpt('run_at', default=None, help="The optional datetime string for the run was started, " "If one isn't provided the date and time of when " @@ -178,8 +182,13 @@ def process_results(results): test_metadata] if test_attr_list: for attr in test_attr_list: - if ('attr', attr) not in test_metadata: - test_meta_dict = {'attr': attr} + if CONF.remove_test_attr_prefix: + normalized_attr = attr[len( + CONF.test_attr_prefix):] + else: + normalized_attr = attr + if ('attr', normalized_attr) not in test_metadata: + test_meta_dict = {'attr': normalized_attr} api.add_test_metadata(test_meta_dict, db_test.id, session=session) api.add_test_run_metadata(results[test]['metadata'], test_run.id, diff --git a/subunit2sql/tests/test_shell.py b/subunit2sql/tests/test_shell.py index fa0a432..8ef753c 100644 --- a/subunit2sql/tests/test_shell.py +++ b/subunit2sql/tests/test_shell.py @@ -604,3 +604,115 @@ class TestProcessResults(base.TestCase): mock.call(fake_attachment, fake_db_test_run_id, self.fake_session)]) self.fake_session.close.assert_called_once() + + def test_process_results_test_metadata_new_remove_prefix(self): + # Setup the metadata prefix configuration + fake_meta_prefix = '_meta_' + cfg.CONF.set_override(name='test_attr_prefix', + override=fake_meta_prefix) + cfg.CONF.set_override(name='remove_test_attr_prefix', + override=True) + fake_run_time = 'run time' + # Setup a common fake DB test + fake_db_test = mock.Mock(name='db test') + fake_db_test.id = 'test id' + fake_db_test.run_count = 3 + fake_db_test.success = 2 + fake_db_test.failure = 1 + # Setup results + expected_metadata_list = ['a', 'b'] + extra_metadata_list = ['c', 'd'] + full_metadata_list = [fake_meta_prefix + x for x in + expected_metadata_list] + extra_metadata_list + fake_results = dict( + test1={'status': 'fail', 'start_time': 0, 'end_time': 1, + 'metadata': {'attrs': ','.join(full_metadata_list)}, + 'attachments': None}) + fake_results_all = copy.deepcopy(fake_results) + fake_results_all['run_time'] = fake_run_time + # Mock create run + fake_db_run_id = 'run id' + fake_db_run = mock.Mock(name='db run') + fake_db_run.id = fake_db_run_id + self.db_api_mock.create_run.return_value = fake_db_run + # Mock create test run + fake_db_test_run_id = 'test run id' + fake_db_test_run = mock.Mock(name='db test run') + fake_db_test_run.id = fake_db_test_run_id + self.db_api_mock.create_test_run.return_value = fake_db_test_run + # Tests are found in the DB + self.db_api_mock.get_test_by_test_id.return_value = fake_db_test + # Test metadata is not found in the DB + self.db_api_mock.get_test_metadata.return_value = [] + # Run process_results + shell.process_results(fake_results_all) + # Check only matching metadata is added to test_metadata + expected_add_test_metadata_calls = [ + mock.call({'attr': x}, fake_db_test.id, session=self.fake_session) + for x in expected_metadata_list] + self.db_api_mock.add_test_metadata.assert_has_calls( + expected_add_test_metadata_calls, any_order=True) + # Check all metadata is added to test_run_metadata + self.db_api_mock.add_test_run_metadata.assert_has_calls([ + mock.call(fake_results['test1']['metadata'], fake_db_test_run_id, + self.fake_session)]) + self.fake_session.close.assert_called_once() + + def test_process_results_test_metadata_existing_remove_prefix(self): + # Setup the metadata prefix configuration + fake_meta_prefix = '_meta_' + cfg.CONF.set_override(name='test_attr_prefix', + override=fake_meta_prefix) + cfg.CONF.set_override(name='remove_test_attr_prefix', + override=True) + fake_run_time = 'run time' + # Setup a common fake DB test + fake_db_test = mock.Mock(name='db test') + fake_db_test.id = 'test id' + fake_db_test.run_count = 3 + fake_db_test.success = 2 + fake_db_test.failure = 1 + # Setup results + expected_metadata_list = ['a', 'b'] + existing_metadata_list = ['c', 'd'] + full_metadata_list = [fake_meta_prefix + x for x in ( + expected_metadata_list + existing_metadata_list)] + fake_results = dict( + test1={'status': 'fail', 'start_time': 0, 'end_time': 1, + 'metadata': {'attrs': ','.join(full_metadata_list)}, + 'attachments': None}) + fake_results_all = copy.deepcopy(fake_results) + fake_results_all['run_time'] = fake_run_time + # Mock create run + fake_db_run_id = 'run id' + fake_db_run = mock.Mock(name='db run') + fake_db_run.id = fake_db_run_id + self.db_api_mock.create_run.return_value = fake_db_run + # Mock create test run + fake_db_test_run_id = 'test run id' + fake_db_test_run = mock.Mock(name='db test run') + fake_db_test_run.id = fake_db_test_run_id + self.db_api_mock.create_test_run.return_value = fake_db_test_run + # Tests are found in the DB + self.db_api_mock.get_test_by_test_id.return_value = fake_db_test + # Test metadata is found in the DB + test_metadata_list = [] + for value in existing_metadata_list: + test_metadata = mock.Mock() + test_metadata.key = 'attr' + test_metadata.value = value + test_metadata_list.append(test_metadata) + self.db_api_mock.get_test_metadata.return_value = test_metadata_list + # Run process_results + shell.process_results(fake_results_all) + # Check only matching metadata is added to test_metadata + expected_add_test_metadata_calls = [ + mock.call({'attr': x}, fake_db_test.id, session=self.fake_session) + for x in expected_metadata_list] + self.db_api_mock.add_test_metadata.assert_has_calls( + expected_add_test_metadata_calls, any_order=True) + # Check all metadata is added to test_run_metadata + self.db_api_mock.add_test_run_metadata.assert_has_calls([ + mock.call(fake_results['test1']['metadata'], fake_db_test_run_id, + self.fake_session)]) + self.fake_session.close.assert_called_once() \ No newline at end of file