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

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.

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:
+                        if CONF.remove_test_attr_prefix:
-                            test_meta_dict = {'attr': attr}
+                            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,

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()