From bac44feda152668e3a2cb99933b390bb07a3efd7 Mon Sep 17 00:00:00 2001 From: Megan Guiney Date: Thu, 1 Mar 2018 02:43:56 -0800 Subject: [PATCH] Reconcile inconsistencies in the db api namescheme Currently, test results are alternately referred to in the db api as "test", "results", and "test_result" These varying terminology choices will become even more obfuscating as we begin using the new subunit data api, as both "tests" and "results" have very different semantic meanings within the context of subunit2sql. This patch will convert all previously used variations to the more semantically clear phrasing "test_result". Updating the database to use clearer phrasing may prove to be worth doing as well, but because I anticipate that that may end up requiring a bit more discussion. Change-Id: Ica54b5b8095bd00217aa2ffefab898306c888c3f --- refstack/api/controllers/results.py | 40 +++---- refstack/api/utils.py | 8 +- refstack/db/api.py | 36 +++---- refstack/db/sqlalchemy/api.py | 18 ++-- refstack/tests/api/test_results.py | 18 ++-- refstack/tests/unit/test_api.py | 148 +++++++++++++------------- refstack/tests/unit/test_api_utils.py | 34 +++--- refstack/tests/unit/test_db.py | 87 +++++++-------- 8 files changed, 199 insertions(+), 190 deletions(-) diff --git a/refstack/api/controllers/results.py b/refstack/api/controllers/results.py index 5d9ed556..010b33db 100644 --- a/refstack/api/controllers/results.py +++ b/refstack/api/controllers/results.py @@ -52,7 +52,7 @@ class MetadataController(rest.RestController): @pecan.expose('json') def get(self, test_id): """Get test run metadata.""" - test_info = db.get_test(test_id) + test_info = db.get_test_result(test_id) role = api_utils.get_user_role(test_id) if role in (const.ROLE_FOUNDATION, const.ROLE_OWNER): return test_info['meta'] @@ -66,9 +66,9 @@ class MetadataController(rest.RestController): """Get value for key from test run metadata.""" role = api_utils.get_user_role(test_id) if role in (const.ROLE_FOUNDATION, const.ROLE_OWNER): - return db.get_test_meta_key(test_id, key) + return db.get_test_result_meta_key(test_id, key) elif role in (const.ROLE_USER) and key in self.rw_access_keys: - return db.get_test_meta_key(test_id, key) + return db.get_test_result_meta_key(test_id, key) pecan.abort(403) @_check_key @@ -76,11 +76,11 @@ class MetadataController(rest.RestController): @pecan.expose('json') def post(self, test_id, key): """Save value for key in test run metadata.""" - test = db.get_test(test_id) + test = db.get_test_result(test_id) if test['verification_status'] == const.TEST_VERIFIED: pecan.abort(403, 'Can not add/alter a new metadata key for a ' 'verified test run.') - db.save_test_meta_item(test_id, key, pecan.request.body) + db.save_test_result_meta_item(test_id, key, pecan.request.body) pecan.response.status = 201 @_check_key @@ -88,11 +88,11 @@ class MetadataController(rest.RestController): @pecan.expose('json') def delete(self, test_id, key): """Delete key from test run metadata.""" - test = db.get_test(test_id) + test = db.get_test_result(test_id) if test['verification_status'] == const.TEST_VERIFIED: pecan.abort(403, 'Can not delete a metadata key for a ' 'verified test run.') - db.delete_test_meta_item(test_id, key) + db.delete_test_result_meta_item(test_id, key) pecan.response.status = 204 @@ -143,14 +143,14 @@ class ResultsController(validation.BaseRestControllerWithValidation): """Handler for getting item.""" user_role = api_utils.get_user_role(test_id) if user_role in (const.ROLE_FOUNDATION, const.ROLE_OWNER): - test_info = db.get_test( + test_info = db.get_test_result( test_id, allowed_keys=['id', 'cpid', 'created_at', 'duration_seconds', 'meta', 'product_version', 'verification_status'] ) else: - test_info = db.get_test(test_id) + test_info = db.get_test_result(test_id) test_list = db.get_test_results(test_id) test_name_list = [test_dict['name'] for test_dict in test_list] test_info.update({'results': test_name_list, @@ -182,7 +182,7 @@ class ResultsController(validation.BaseRestControllerWithValidation): test_['meta'][const.USER] = pubkey.openid test_ = self._auto_version_associate(test, test_, pubkey) - test_id = db.store_results(test_) + test_id = db.store_test_results(test_) return {'test_id': test_id, 'url': parse.urljoin(CONF.ui_url, CONF.api.test_results_url) % test_id} @@ -191,11 +191,11 @@ class ResultsController(validation.BaseRestControllerWithValidation): @api_utils.check_permissions(level=const.ROLE_OWNER) def delete(self, test_id): """Delete test run.""" - test = db.get_test(test_id) + test = db.get_test_result(test_id) if test['verification_status'] == const.TEST_VERIFIED: pecan.abort(403, 'Can not delete a verified test run.') - db.delete_test(test_id) + db.delete_test_result(test_id) pecan.response.status = 204 @pecan.expose('json') @@ -231,13 +231,14 @@ class ResultsController(validation.BaseRestControllerWithValidation): elif not product['public']: pecan.abort(403, 'Forbidden.') - records_count = db.get_test_records_count(filters) + records_count = db.get_test_result_records_count(filters) page_number, total_pages_number = \ api_utils.get_page_number(records_count) try: per_page = CONF.api.results_per_page - results = db.get_test_records(page_number, per_page, filters) + results = db.get_test_result_records( + page_number, per_page, filters) is_foundation = api_utils.check_user_is_foundation_admin() for result in results: @@ -279,7 +280,7 @@ class ResultsController(validation.BaseRestControllerWithValidation): is_foundation_admin = api_utils.check_user_is_foundation_admin() if 'product_version_id' in kw: - test = db.get_test(test_id) + test = db.get_test_result(test_id) if test['verification_status'] == const.TEST_VERIFIED: pecan.abort(403, 'Can not update product_version_id for a ' 'verified test run.') @@ -314,9 +315,10 @@ class ResultsController(validation.BaseRestControllerWithValidation): # Check pre-conditions are met to mark a test verified. if (kw['verification_status'] == 1 and - not (db.get_test_meta_key(test_id, 'target') and - db.get_test_meta_key(test_id, 'guideline') and - db.get_test_meta_key(test_id, const.SHARED_TEST_RUN))): + not (db.get_test_result_meta_key(test_id, 'target') and + db.get_test_result_meta_key(test_id, 'guideline') and + db.get_test_result_meta_key(test_id, + const.SHARED_TEST_RUN))): pecan.abort(403, 'In order to mark a test verified, the ' 'test must be shared and have been ' @@ -325,6 +327,6 @@ class ResultsController(validation.BaseRestControllerWithValidation): test_info['verification_status'] = kw['verification_status'] - test = db.update_test(test_info) + test = db.update_test_result(test_info) pecan.response.status = 201 return test diff --git a/refstack/api/utils.py b/refstack/api/utils.py index 4d608c14..ddb385c0 100644 --- a/refstack/api/utils.py +++ b/refstack/api/utils.py @@ -253,10 +253,10 @@ def get_user_role(test_id): def check_user(test_id): """Check that user has access to shared test run.""" - test_owner = db.get_test_meta_key(test_id, const.USER) + test_owner = db.get_test_result_meta_key(test_id, const.USER) if not test_owner: return True - elif db.get_test_meta_key(test_id, const.SHARED_TEST_RUN): + elif db.get_test_result_meta_key(test_id, const.SHARED_TEST_RUN): return True else: return check_owner(test_id) @@ -267,14 +267,14 @@ def check_owner(test_id): if not is_authenticated(): return False - test = db.get_test(test_id) + test = db.get_test_result(test_id) # If the test is owned by a product. if test.get('product_version_id'): version = db.get_product_version(test['product_version_id']) return check_user_is_product_admin(version['product_id']) # Otherwise, check the user ownership. else: - user = db.get_test_meta_key(test_id, const.USER) + user = db.get_test_result_meta_key(test_id, const.USER) return user and user == get_user_id() diff --git a/refstack/db/api.py b/refstack/db/api.py index defd8458..f6ce4131 100644 --- a/refstack/db/api.py +++ b/refstack/db/api.py @@ -46,36 +46,36 @@ NotFound = IMPL.NotFound Duplication = IMPL.Duplication -def store_results(results): +def store_test_results(results): """Storing results into database. :param results: Dict describes test results. """ - return IMPL.store_results(results) + return IMPL.store_test_results(results) -def get_test(test_id, allowed_keys=None): +def get_test_result(test_id, allowed_keys=None): """Get test run information from the database. :param test_id: The ID of the test. """ - return IMPL.get_test(test_id, allowed_keys=allowed_keys) + return IMPL.get_test_result(test_id, allowed_keys=allowed_keys) -def delete_test(test_id): +def delete_test_result(test_id): """Delete test run information from the database. :param test_id: The ID of the test. """ - return IMPL.delete_test(test_id) + return IMPL.delete_test_result(test_id) -def update_test(test_info): +def update_test_result(test_info): """Update test from the given test_info dictionary. :param test_info: The test """ - return IMPL.update_test(test_info) + return IMPL.update_test_result(test_info) def get_test_results(test_id): @@ -86,7 +86,7 @@ def get_test_results(test_id): return IMPL.get_test_results(test_id) -def get_test_meta_key(test_id, key, default=None): +def get_test_result_meta_key(test_id, key, default=None): """Get metadata value related to specified test run. :param test_id: The ID of the test. @@ -94,20 +94,20 @@ def get_test_meta_key(test_id, key, default=None): :param default: Default value """ - return IMPL.get_test_meta_key(test_id, key, default) + return IMPL.get_test_result_meta_key(test_id, key, default) -def save_test_meta_item(test_id, key, value): +def save_test_result_meta_item(test_id, key, value): """Store or update item value related to specified test run. :param test_id: The ID of the test. :param key: Metadata key """ - return IMPL.save_test_meta_item(test_id, key, value) + return IMPL.save_test_result_meta_item(test_id, key, value) -def delete_test_meta_item(test_id, key): +def delete_test_result_meta_item(test_id, key): """Delete metadata item related to specified test run. :param test_id: The ID of the test. @@ -116,25 +116,25 @@ def delete_test_meta_item(test_id, key): :raise NotFound if default value is not set and no value found """ - return IMPL.delete_test_meta_item(test_id, key) + return IMPL.delete_test_result_meta_item(test_id, key) -def get_test_records(page_number, per_page, filters): +def get_test_result_records(page_number, per_page, filters): """Get page with applied filters for uploaded test records. :param page_number: The number of page. :param per_page: The number of results for one page. :param filters: (Dict) Filters that will be applied for records. """ - return IMPL.get_test_records(page_number, per_page, filters) + return IMPL.get_test_result_records(page_number, per_page, filters) -def get_test_records_count(filters): +def get_test_result_records_count(filters): """Get total pages number with applied filters for uploaded test records. :param filters: (Dict) Filters that will be applied for records. """ - return IMPL.get_test_records_count(filters) + return IMPL.get_test_result_records_count(filters) def user_get(user_openid): diff --git a/refstack/db/sqlalchemy/api.py b/refstack/db/sqlalchemy/api.py index 1ddf274b..cb29b4b3 100644 --- a/refstack/db/sqlalchemy/api.py +++ b/refstack/db/sqlalchemy/api.py @@ -109,7 +109,7 @@ def _to_dict(sqlalchemy_object, allowed_keys=None): return sqlalchemy_object -def store_results(results): +def store_test_results(results): """Store test results.""" test = models.Test() test_id = str(uuid.uuid4()) @@ -133,7 +133,7 @@ def store_results(results): return test_id -def get_test(test_id, allowed_keys=None): +def get_test_result(test_id, allowed_keys=None): """Get test info.""" session = get_session() test_info = session.query(models.Test). \ @@ -144,7 +144,7 @@ def get_test(test_id, allowed_keys=None): return _to_dict(test_info, allowed_keys) -def delete_test(test_id): +def delete_test_result(test_id): """Delete test information from the database.""" session = get_session() with session.begin(): @@ -159,7 +159,7 @@ def delete_test(test_id): raise NotFound('Test result %s not found' % test_id) -def update_test(test_info): +def update_test_result(test_info): """Update test from the given test_info dictionary.""" session = get_session() _id = test_info.get('id') @@ -177,7 +177,7 @@ def update_test(test_info): return _to_dict(test) -def get_test_meta_key(test_id, key, default=None): +def get_test_result_meta_key(test_id, key, default=None): """Get metadata value related to specified test run.""" session = get_session() meta_item = session.query(models.TestMeta). \ @@ -188,7 +188,7 @@ def get_test_meta_key(test_id, key, default=None): return value -def save_test_meta_item(test_id, key, value): +def save_test_result_meta_item(test_id, key, value): """Store or update item value related to specified test run.""" session = get_session() meta_item = (session.query(models.TestMeta) @@ -201,7 +201,7 @@ def save_test_meta_item(test_id, key, value): meta_item.save(session) -def delete_test_meta_item(test_id, key): +def delete_test_result_meta_item(test_id, key): """Delete metadata item related to specified test run.""" session = get_session() meta_item = session.query(models.TestMeta). \ @@ -274,7 +274,7 @@ def _apply_filters_for_query(query, filters): return query -def get_test_records(page, per_page, filters): +def get_test_result_records(page, per_page, filters): """Get page with list of test records.""" session = get_session() query = session.query(models.Test) @@ -285,7 +285,7 @@ def get_test_records(page, per_page, filters): return _to_dict(results) -def get_test_records_count(filters): +def get_test_result_records_count(filters): """Get total test records count.""" session = get_session() query = session.query(models.Test.id) diff --git a/refstack/tests/api/test_results.py b/refstack/tests/api/test_results.py index 05bce243..73b29d8d 100644 --- a/refstack/tests/api/test_results.py +++ b/refstack/tests/api/test_results.py @@ -142,21 +142,21 @@ class TestResultsEndpoint(api.FunctionalTest): self.assertEqual(403, put_response.status_code) # Share the test run. - db.save_test_meta_item(test_id, api_const.SHARED_TEST_RUN, True) + db.save_test_result_meta_item(test_id, api_const.SHARED_TEST_RUN, True) put_response = self.put_json(url, expect_errors=True, params=json.dumps(body)) self.assertEqual(403, put_response.status_code) # Now associate guideline and target program. Now we should be # able to mark a test verified. - db.save_test_meta_item(test_id, 'target', 'platform') - db.save_test_meta_item(test_id, 'guideline', '2016.01.json') + db.save_test_result_meta_item(test_id, 'target', 'platform') + db.save_test_result_meta_item(test_id, 'guideline', '2016.01.json') put_response = self.put_json(url, params=json.dumps(body)) self.assertEqual(api_const.TEST_VERIFIED, put_response['verification_status']) # Unshare the test, and check that we can mark it not verified. - db.delete_test_meta_item(test_id, api_const.SHARED_TEST_RUN) + db.delete_test_result_meta_item(test_id, api_const.SHARED_TEST_RUN) body = {'verification_status': api_const.TEST_NOT_VERIFIED} put_response = self.put_json(url, params=json.dumps(body)) self.assertEqual(api_const.TEST_NOT_VERIFIED, @@ -368,8 +368,8 @@ class TestResultsEndpoint(api.FunctionalTest): post_response = self.post_json('/v1/results', params=results) test_id = post_response['test_id'] test_info = {'id': test_id, 'product_version_id': version_id} - db.update_test(test_info) - db.save_test_meta_item(test_id, api_const.USER, 'test-open-id') + db.update_test_result(test_info) + db.save_test_result_meta_item(test_id, api_const.USER, 'test-open-id') url = self.URL + '?page=1&product_id=' + product_id @@ -392,7 +392,7 @@ class TestResultsEndpoint(api.FunctionalTest): self.assertFalse(response['results']) # Share the test run. - db.save_test_meta_item(test_id, api_const.SHARED_TEST_RUN, 1) + db.save_test_result_meta_item(test_id, api_const.SHARED_TEST_RUN, 1) response = self.get_json(url) self.assertEqual(1, len(response['results'])) self.assertEqual(test_id, response['results'][0]['id']) @@ -407,12 +407,12 @@ class TestResultsEndpoint(api.FunctionalTest): mock_check_owner.return_value = True # Test can't delete verified test run. - db.update_test({'id': test_id, 'verification_status': 1}) + db.update_test_result({'id': test_id, 'verification_status': 1}) resp = self.delete(url, expect_errors=True) self.assertEqual(403, resp.status_code) # Test can delete verified test run. - db.update_test({'id': test_id, 'verification_status': 0}) + db.update_test_result({'id': test_id, 'verification_status': 0}) resp = self.delete(url, expect_errors=True) self.assertEqual(204, resp.status_code) diff --git a/refstack/tests/unit/test_api.py b/refstack/tests/unit/test_api.py index 689147cf..8ce343fa 100644 --- a/refstack/tests/unit/test_api.py +++ b/refstack/tests/unit/test_api.py @@ -80,17 +80,17 @@ class ResultsControllerTestCase(BaseControllerTestCase): 'api') self.CONF.set_override('ui_url', self.ui_url) - @mock.patch('refstack.db.get_test') + @mock.patch('refstack.db.get_test_result') @mock.patch('refstack.db.get_test_results') - def test_get(self, mock_get_test_res, mock_get_test): + def test_get(self, mock_get_test_results, mock_get_test_result): self.mock_get_user_role.return_value = const.ROLE_FOUNDATION test_info = {'created_at': 'bar', 'duration_seconds': 999, 'meta': {'shared': 'true', 'user': 'fake-user'}} - mock_get_test.return_value = test_info + mock_get_test_result.return_value = test_info - mock_get_test_res.return_value = [{'name': 'test1'}, - {'name': 'test2'}] + mock_get_test_results.return_value = [{'name': 'test1'}, + {'name': 'test2'}] actual_result = self.controller.get_one('fake_arg') # All meta should be exposed when user is a Foundation admin. @@ -103,29 +103,29 @@ class ResultsControllerTestCase(BaseControllerTestCase): } self.assertEqual(expected_result, actual_result) - mock_get_test_res.assert_called_once_with('fake_arg') + mock_get_test_results.assert_called_once_with('fake_arg') # If not owner or Foundation admin, don't show all metadata. self.mock_get_user_role.return_value = const.ROLE_USER - mock_get_test.return_value = test_info - mock_get_test_res.return_value = [{'name': 'test1'}, - {'name': 'test2'}] + mock_get_test_result.return_value = test_info + mock_get_test_results.return_value = [{'name': 'test1'}, + {'name': 'test2'}] actual_result = self.controller.get_one('fake_arg') expected_result['meta'] = {'shared': 'true'} expected_result['user_role'] = const.ROLE_USER self.assertEqual(expected_result, actual_result) - @mock.patch('refstack.db.get_test') + @mock.patch('refstack.db.get_test_result') @mock.patch('refstack.db.get_test_results') - def test_get_for_owner(self, mock_get_test_res, mock_get_test): + def test_get_for_owner(self, mock_get_test_results, mock_get_test_result): self.mock_get_user_role.return_value = const.ROLE_OWNER test_info = {'cpid': 'foo', 'created_at': 'bar', 'duration_seconds': 999} - mock_get_test.return_value = test_info + mock_get_test_result.return_value = test_info - mock_get_test_res.return_value = [{'name': 'test1'}, - {'name': 'test2'}] + mock_get_test_results.return_value = [{'name': 'test1'}, + {'name': 'test2'}] actual_result = self.controller.get_one('fake_arg') expected_result = { @@ -137,19 +137,19 @@ class ResultsControllerTestCase(BaseControllerTestCase): } self.assertEqual(actual_result, expected_result) - mock_get_test_res.assert_called_once_with('fake_arg') - mock_get_test.assert_called_once_with( + mock_get_test_results.assert_called_once_with('fake_arg') + mock_get_test_result.assert_called_once_with( 'fake_arg', allowed_keys=['id', 'cpid', 'created_at', 'duration_seconds', 'meta', 'product_version', 'verification_status'] ) - @mock.patch('refstack.db.store_results') - def test_post(self, mock_store_results): + @mock.patch('refstack.db.store_test_results') + def test_post(self, mock_store_test_results): self.mock_request.body = b'{"answer": 42}' self.mock_request.headers = {} - mock_store_results.return_value = 'fake_test_id' + mock_store_test_results.return_value = 'fake_test_id' result = self.controller.post() self.assertEqual( result, @@ -158,14 +158,14 @@ class ResultsControllerTestCase(BaseControllerTestCase): self.test_results_url) % 'fake_test_id'} ) self.assertEqual(self.mock_response.status, 201) - mock_store_results.assert_called_once_with({'answer': 42}) + mock_store_test_results.assert_called_once_with({'answer': 42}) @mock.patch('refstack.api.utils.check_user_is_foundation_admin') @mock.patch('refstack.api.utils.check_user_is_product_admin') @mock.patch('refstack.db.get_product_version_by_cpid') - @mock.patch('refstack.db.store_results') + @mock.patch('refstack.db.store_test_results') @mock.patch('refstack.db.get_pubkey') - def test_post_with_sign(self, mock_get_pubkey, mock_store_results, + def test_post_with_sign(self, mock_get_pubkey, mock_store_test_results, mock_get_version, mock_check, mock_foundation): self.mock_request.body = b'{"answer": 42, "cpid": "123"}' self.mock_request.headers = { @@ -178,21 +178,21 @@ class ResultsControllerTestCase(BaseControllerTestCase): 'product_id': 'prod1'}] mock_check.return_value = True mock_foundation.return_value = False - mock_store_results.return_value = 'fake_test_id' + mock_store_test_results.return_value = 'fake_test_id' result = self.controller.post() self.assertEqual(result, {'test_id': 'fake_test_id', 'url': self.test_results_url % 'fake_test_id'}) self.assertEqual(self.mock_response.status, 201) mock_check.assert_called_once_with('prod1', 'fake_openid') - mock_store_results.assert_called_once_with( + mock_store_test_results.assert_called_once_with( {'answer': 42, 'cpid': '123', 'product_version_id': 'ver1', 'meta': {const.USER: 'fake_openid'}} ) - @mock.patch('refstack.db.get_test') - def test_get_item_failed(self, mock_get_test): - mock_get_test.return_value = None + @mock.patch('refstack.db.get_test_result') + def test_get_item_failed(self, mock_get_test_result): + mock_get_test_result.return_value = None self.assertRaises(webob.exc.HTTPError, self.controller.get_one, 'fake_id') @@ -204,16 +204,16 @@ class ResultsControllerTestCase(BaseControllerTestCase): self.assertRaises(api_exc.ParseInputsError, self.controller.get) - @mock.patch('refstack.db.get_test_records_count') + @mock.patch('refstack.db.get_test_result_records_count') @mock.patch('refstack.api.utils.parse_input_params') - def test_get_failed_in_get_test_records_number(self, - parse_inputs, - db_get_count): + def test_get_failed_in_get_test_result_records_number(self, + parse_inputs, + db_get_count): db_get_count.side_effect = api_exc.ParseInputsError() self.assertRaises(api_exc.ParseInputsError, self.controller.get) - @mock.patch('refstack.db.get_test_records_count') + @mock.patch('refstack.db.get_test_result_records_count') @mock.patch('refstack.api.utils.parse_input_params') @mock.patch('refstack.api.utils.get_page_number') def test_get_failed_in_get_page_number(self, @@ -225,32 +225,32 @@ class ResultsControllerTestCase(BaseControllerTestCase): self.assertRaises(api_exc.ParseInputsError, self.controller.get) - @mock.patch('refstack.db.get_test_records') - @mock.patch('refstack.db.get_test_records_count') + @mock.patch('refstack.db.get_test_result_records') + @mock.patch('refstack.db.get_test_result_records_count') @mock.patch('refstack.api.utils.parse_input_params') @mock.patch('refstack.api.utils.get_page_number') - def test_get_failed_in_get_test_records(self, - get_page, - parce_input, - db_get_count, - db_get_test): + def test_get_failed_in_get_test_result_records(self, + get_page, + parce_input, + db_get_count, + db_get_test_result): get_page.return_value = (mock.Mock(), mock.Mock()) - db_get_test.side_effect = Exception() + db_get_test_result.side_effect = Exception() self.assertRaises(webob.exc.HTTPError, self.controller.get) @mock.patch('refstack.api.utils.check_owner') @mock.patch('refstack.api.utils.check_user_is_foundation_admin') - @mock.patch('refstack.db.get_test_records') - @mock.patch('refstack.db.get_test_records_count') + @mock.patch('refstack.db.get_test_result_records') + @mock.patch('refstack.db.get_test_result_records_count') @mock.patch('refstack.api.utils.get_page_number') @mock.patch('refstack.api.utils.parse_input_params') def test_get_success(self, parse_input, get_page, - get_test_count, - db_get_test, + get_test_result_count, + db_get_test_result, check_foundation, check_owner): @@ -266,7 +266,7 @@ class ResultsControllerTestCase(BaseControllerTestCase): total_pages_number = 10 per_page = 5 records_count = 50 - get_test_count.return_value = records_count + get_test_result_count.return_value = records_count get_page.return_value = (page_number, total_pages_number) check_foundation.return_value = False check_owner.return_value = True @@ -278,7 +278,7 @@ class ResultsControllerTestCase(BaseControllerTestCase): expected_record = record.copy() expected_record['url'] = self.test_results_url % record['id'] - db_get_test.return_value = [record] + db_get_test_result.return_value = [record] expected_result = { 'results': [expected_record], 'pagination': { @@ -293,22 +293,23 @@ class ResultsControllerTestCase(BaseControllerTestCase): parse_input.assert_called_once_with(expected_input_params) filters = parse_input.return_value - get_test_count.assert_called_once_with(filters) + get_test_result_count.assert_called_once_with(filters) get_page.assert_called_once_with(records_count) - db_get_test.assert_called_once_with(page_number, per_page, filters) + db_get_test_result.assert_called_once_with( + page_number, per_page, filters) - @mock.patch('refstack.db.get_test') - @mock.patch('refstack.db.delete_test') - def test_delete(self, mock_db_delete, mock_get_test): + @mock.patch('refstack.db.get_test_result') + @mock.patch('refstack.db.delete_test_result') + def test_delete(self, mock_db_delete, mock_get_test_result): self.mock_get_user_role.return_value = const.ROLE_OWNER self.controller.delete('test_id') self.assertEqual(204, self.mock_response.status) # Verified test deletion attempt should raise error. - mock_get_test.return_value = {'verification_status': - const.TEST_VERIFIED} + mock_get_test_result.return_value = {'verification_status': + const.TEST_VERIFIED} self.assertRaises(webob.exc.HTTPError, self.controller.delete, 'test_id') @@ -614,14 +615,14 @@ class MetadataControllerTestCase(BaseControllerTestCase): super(MetadataControllerTestCase, self).setUp() self.controller = results.MetadataController() - @mock.patch('refstack.db.get_test') - def test_get(self, mock_db_get_test): + @mock.patch('refstack.db.get_test_result') + def test_get(self, mock_db_get_test_result): self.mock_get_user_role.return_value = const.ROLE_USER - mock_db_get_test.return_value = {'meta': {'shared': 'true', - 'user': 'fake-user'}} + mock_db_get_test_result.return_value = {'meta': {'shared': 'true', + 'user': 'fake-user'}} # Only the key 'shared' should be allowed through. self.assertEqual({'shared': 'true'}, self.controller.get('test_id')) - mock_db_get_test.assert_called_once_with('test_id') + mock_db_get_test_result.assert_called_once_with('test_id') # Test that the result owner can see all metadata keys. self.mock_get_user_role.return_value = const.ROLE_OWNER @@ -633,8 +634,8 @@ class MetadataControllerTestCase(BaseControllerTestCase): self.assertEqual({'shared': 'true', 'user': 'fake-user'}, self.controller.get('test_id')) - @mock.patch('refstack.db.get_test_meta_key') - def test_get_one(self, mock_db_get_test_meta_key): + @mock.patch('refstack.db.get_test_result_meta_key') + def test_get_one(self, mock_db_get_test_result_meta_key): self.mock_get_user_role.return_value = const.ROLE_USER # Test when key is not an allowed key. @@ -642,9 +643,10 @@ class MetadataControllerTestCase(BaseControllerTestCase): self.controller.get_one, 'test_id', 'answer') # Test when key is an allowed key. - mock_db_get_test_meta_key.return_value = 42 + mock_db_get_test_result_meta_key.return_value = 42 self.assertEqual(42, self.controller.get_one('test_id', 'shared')) - mock_db_get_test_meta_key.assert_called_once_with('test_id', 'shared') + mock_db_get_test_result_meta_key.assert_called_once_with( + 'test_id', 'shared') # Test when the user owns the test result. self.mock_get_user_role.return_value = const.ROLE_OWNER @@ -654,18 +656,18 @@ class MetadataControllerTestCase(BaseControllerTestCase): self.mock_get_user_role.return_value = const.ROLE_FOUNDATION self.assertEqual(42, self.controller.get_one('test_id', 'user')) - @mock.patch('refstack.db.get_test') - @mock.patch('refstack.db.save_test_meta_item') - def test_post(self, mock_save_test_meta_item, mock_get_test): + @mock.patch('refstack.db.get_test_result') + @mock.patch('refstack.db.save_test_result_meta_item') + def test_post(self, mock_save_test_result_meta_item, mock_get_test_result): self.mock_get_user_role.return_value = const.ROLE_OWNER - mock_get_test.return_value = { + mock_get_test_result.return_value = { 'verification_status': const.TEST_NOT_VERIFIED } # Test trying to post a valid key. self.controller.post('test_id', 'shared') self.assertEqual(201, self.mock_response.status) - mock_save_test_meta_item.assert_called_once_with( + mock_save_test_result_meta_item.assert_called_once_with( 'test_id', 'shared', self.mock_request.body) # Test trying to post an invalid key. @@ -678,16 +680,18 @@ class MetadataControllerTestCase(BaseControllerTestCase): self.assertRaises(webob.exc.HTTPError, self.controller.post, 'test_id', 'shared') - @mock.patch('refstack.db.get_test') - @mock.patch('refstack.db.delete_test_meta_item') - def test_delete(self, mock_delete_test_meta_item, mock_get_test): + @mock.patch('refstack.db.get_test_result') + @mock.patch('refstack.db.delete_test_result_meta_item') + def test_delete(self, mock_delete_test_result_meta_item, + mock_get_test_result): self.mock_get_user_role.return_value = const.ROLE_OWNER - mock_get_test.return_value = { + mock_get_test_result.return_value = { 'verification_status': const.TEST_NOT_VERIFIED } self.controller.delete('test_id', 'shared') self.assertEqual(204, self.mock_response.status) - mock_delete_test_meta_item.assert_called_once_with('test_id', 'shared') + mock_delete_test_result_meta_item.assert_called_once_with( + 'test_id', 'shared') # The key 'user' is not a valid key that can be deleted. self.assertRaises(webob.exc.HTTPError, diff --git a/refstack/tests/unit/test_api_utils.py b/refstack/tests/unit/test_api_utils.py index bd9cb162..6a7a0ef3 100644 --- a/refstack/tests/unit/test_api_utils.py +++ b/refstack/tests/unit/test_api_utils.py @@ -364,26 +364,26 @@ class APIUtilsTestCase(base.BaseTestCase): @mock.patch('refstack.api.utils.check_user_is_foundation_admin') @mock.patch('pecan.abort', side_effect=exc.HTTPError) - @mock.patch('refstack.db.get_test_meta_key') - @mock.patch('refstack.db.get_test') + @mock.patch('refstack.db.get_test_result_meta_key') + @mock.patch('refstack.db.get_test_result') @mock.patch.object(api_utils, 'is_authenticated') @mock.patch.object(api_utils, 'get_user_id') def test_check_get_user_role(self, mock_get_user_id, mock_is_authenticated, - mock_get_test, - mock_get_test_meta_key, + mock_get_test_result, + mock_get_test_result_meta_key, mock_pecan_abort, mock_check_foundation): # Check user level mock_check_foundation.return_value = False - mock_get_test_meta_key.return_value = None - mock_get_test.return_value = {} + mock_get_test_result_meta_key.return_value = None + mock_get_test_result.return_value = {} self.assertEqual(const.ROLE_USER, api_utils.get_user_role('fake_test')) api_utils.enforce_permissions('fake_test', const.ROLE_USER) self.assertRaises(exc.HTTPError, api_utils.enforce_permissions, 'fake_test', const.ROLE_OWNER) - mock_get_test_meta_key.side_effect = { + mock_get_test_result_meta_key.side_effect = { ('fake_test', const.USER): 'fake_openid', ('fake_test', const.SHARED_TEST_RUN): 'true', }.get @@ -394,7 +394,7 @@ class APIUtilsTestCase(base.BaseTestCase): mock_is_authenticated.return_value = True mock_get_user_id.return_value = 'fake_openid' - mock_get_test_meta_key.side_effect = { + mock_get_test_result_meta_key.side_effect = { ('fake_test', const.USER): 'fake_openid', ('fake_test', const.SHARED_TEST_RUN): 'true', }.get @@ -406,7 +406,7 @@ class APIUtilsTestCase(base.BaseTestCase): # Check owner level mock_is_authenticated.return_value = True mock_get_user_id.return_value = 'fake_openid' - mock_get_test_meta_key.side_effect = lambda *args: { + mock_get_test_result_meta_key.side_effect = lambda *args: { ('fake_test', const.USER): 'fake_openid', ('fake_test', const.SHARED_TEST_RUN): None, }.get(args) @@ -417,7 +417,7 @@ class APIUtilsTestCase(base.BaseTestCase): # Check negative cases mock_is_authenticated.return_value = False - mock_get_test_meta_key.side_effect = lambda *args: { + mock_get_test_result_meta_key.side_effect = lambda *args: { ('fake_test', const.USER): 'fake_openid', ('fake_test', const.SHARED_TEST_RUN): None, }.get(args) @@ -428,7 +428,7 @@ class APIUtilsTestCase(base.BaseTestCase): mock_is_authenticated.return_value = True mock_get_user_id.return_value = 'fake_openid' - mock_get_test_meta_key.side_effect = lambda *args: { + mock_get_test_result_meta_key.side_effect = lambda *args: { ('fake_test', const.USER): 'some_other_user', ('fake_test', const.SHARED_TEST_RUN): None, }.get(args) @@ -440,14 +440,14 @@ class APIUtilsTestCase(base.BaseTestCase): @mock.patch('refstack.api.utils.check_user_is_foundation_admin') @mock.patch('pecan.abort', side_effect=exc.HTTPError) - @mock.patch('refstack.db.get_test_meta_key') - @mock.patch('refstack.db.get_test') + @mock.patch('refstack.db.get_test_result_meta_key') + @mock.patch('refstack.db.get_test_result') @mock.patch.object(api_utils, 'is_authenticated') @mock.patch.object(api_utils, 'get_user_id') def test_check_permissions(self, mock_get_user_id, mock_is_authenticated, - mock_get_test, - mock_get_test_meta_key, + mock_get_test_result, + mock_get_test_result_meta_key, mock_pecan_abort, mock_foundation_check): @@ -471,8 +471,8 @@ class APIUtilsTestCase(base.BaseTestCase): private_test = 'fake_test' mock_get_user_id.return_value = 'fake_openid' - mock_get_test.return_value = {} - mock_get_test_meta_key.side_effect = lambda *args: { + mock_get_test_result.return_value = {} + mock_get_test_result_meta_key.side_effect = lambda *args: { (public_test, const.USER): None, (private_test, const.USER): 'fake_openid', (private_test, const.SHARED_TEST_RUN): None, diff --git a/refstack/tests/unit/test_db.py b/refstack/tests/unit/test_db.py index 2d51203a..1dbab28c 100644 --- a/refstack/tests/unit/test_db.py +++ b/refstack/tests/unit/test_db.py @@ -32,31 +32,31 @@ from refstack.db.sqlalchemy import models class DBAPITestCase(base.BaseTestCase): """Test case for database API.""" - @mock.patch.object(api, 'store_results') - def test_store_results(self, mock_store_results): - db.store_results('fake_results') - mock_store_results.assert_called_once_with('fake_results') + @mock.patch.object(api, 'store_test_results') + def test_store_test_results(self, mock_store_test_results): + db.store_test_results('fake_results') + mock_store_test_results.assert_called_once_with('fake_results') - @mock.patch.object(api, 'get_test') - def test_get_test(self, mock_get_test): - db.get_test(12345) - mock_get_test.assert_called_once_with(12345, allowed_keys=None) + @mock.patch.object(api, 'get_test_result') + def test_get_test_result(self, mock_get_test_result): + db.get_test_result(12345) + mock_get_test_result.assert_called_once_with(12345, allowed_keys=None) @mock.patch.object(api, 'get_test_results') def test_get_test_results(self, mock_get_test_results): db.get_test_results(12345) mock_get_test_results.assert_called_once_with(12345) - @mock.patch.object(api, 'get_test_records') - def test_get_test_records(self, mock_db): + @mock.patch.object(api, 'get_test_result_records') + def test_get_test_result_records(self, mock_db): filters = mock.Mock() - db.get_test_records(1, 2, filters) + db.get_test_result_records(1, 2, filters) mock_db.assert_called_once_with(1, 2, filters) - @mock.patch.object(api, 'get_test_records_count') - def test_get_test_records_count(self, mock_db): + @mock.patch.object(api, 'get_test_result_records_count') + def test_get_test_result_records_count(self, mock_db): filters = mock.Mock() - db.get_test_records_count(filters) + db.get_test_result_records_count(filters) mock_db.assert_called_once_with(filters) @mock.patch.object(api, 'user_get') @@ -161,8 +161,8 @@ class DBBackendTestCase(base.BaseTestCase): @mock.patch('refstack.db.sqlalchemy.models.Test') @mock.patch('refstack.db.sqlalchemy.models.TestMeta') @mock.patch('uuid.uuid4') - def test_store_results(self, mock_uuid, mock_test_meta, mock_test, - mock_test_result, mock_get_session): + def test_store_test_results(self, mock_uuid, mock_test_meta, mock_test, + mock_test_result, mock_get_session): fake_tests_result = { 'cpid': 'foo', 'duration_seconds': 10, @@ -184,7 +184,7 @@ class DBBackendTestCase(base.BaseTestCase): test_result = mock_test_result.return_value test_result.save = mock.Mock() - test_id = api.store_results(fake_tests_result) + test_id = api.store_test_results(fake_tests_result) mock_test.assert_called_once_with() mock_get_session.assert_called_once_with() @@ -201,7 +201,7 @@ class DBBackendTestCase(base.BaseTestCase): @mock.patch.object(api, 'get_session') @mock.patch('refstack.db.sqlalchemy.models.Test') @mock.patch.object(api, '_to_dict', side_effect=lambda x, *args: x) - def test_get_test(self, mock_to_dict, mock_test, mock_get_session): + def test_get_test_result(self, mock_to_dict, mock_test, mock_get_session): session = mock_get_session.return_value session.query = mock.Mock() query = session.query.return_value @@ -210,7 +210,7 @@ class DBBackendTestCase(base.BaseTestCase): mock_result = 'fake_test_info' filter_by.first = mock.Mock(return_value=mock_result) test_id = 'fake_id' - actual_result = api.get_test(test_id) + actual_result = api.get_test_result(test_id) mock_get_session.assert_called_once_with() session.query.assert_called_once_with(mock_test) @@ -222,11 +222,11 @@ class DBBackendTestCase(base.BaseTestCase): session.query = mock.Mock() query = session.query.return_value query.filter_by.return_value.first.return_value = None - self.assertRaises(api.NotFound, api.get_test, 'fake_id') + self.assertRaises(api.NotFound, api.get_test_result, 'fake_id') @mock.patch('refstack.db.sqlalchemy.api.models') @mock.patch.object(api, 'get_session') - def test_delete_test(self, mock_get_session, mock_models): + def test_delete_test_result(self, mock_get_session, mock_models): session = mock_get_session.return_value test_query = mock.Mock() test_meta_query = mock.Mock() @@ -236,7 +236,7 @@ class DBBackendTestCase(base.BaseTestCase): mock_models.TestMeta: test_meta_query, mock_models.TestResults: test_results_query }.get) - db.delete_test('fake_id') + db.delete_test_result('fake_id') session.begin.assert_called_once_with() test_query.filter_by.return_value.first\ .assert_called_once_with() @@ -252,18 +252,18 @@ class DBBackendTestCase(base.BaseTestCase): session.query.return_value\ .filter_by.return_value\ .first.return_value = None - self.assertRaises(api.NotFound, db.delete_test, 'fake_id') + self.assertRaises(api.NotFound, db.delete_test_result, 'fake_id') @mock.patch.object(api, 'get_session') @mock.patch.object(api, '_to_dict', side_effect=lambda x: x) - def test_update_test(self, mock_to_dict, mock_get_session): + def test_update_test_result(self, mock_to_dict, mock_get_session): session = mock_get_session.return_value mock_test = mock.Mock() session.query.return_value.filter_by.return_value\ .first.return_value = mock_test test_info = {'product_version_id': '123'} - api.update_test(test_info) + api.update_test_result(test_info) mock_get_session.assert_called_once_with() mock_test.save.assert_called_once_with(session=session) @@ -271,29 +271,31 @@ class DBBackendTestCase(base.BaseTestCase): @mock.patch('refstack.db.sqlalchemy.api.models') @mock.patch.object(api, 'get_session') - def test_get_test_meta_key(self, mock_get_session, mock_models): + def test_get_test_result_meta_key(self, mock_get_session, mock_models): session = mock_get_session.return_value session.query.return_value\ .filter_by.return_value\ .filter_by.return_value\ .first.return_value = mock.Mock(value=42) - self.assertEqual(42, db.get_test_meta_key('fake_id', 'fake_key')) + self.assertEqual( + 42, db.get_test_result_meta_key('fake_id', 'fake_key')) session.query.return_value\ .filter_by.return_value\ .filter_by.return_value\ .first.return_value = None - self.assertEqual(24, db.get_test_meta_key('fake_id', 'fake_key', 24)) + self.assertEqual(24, db.get_test_result_meta_key( + 'fake_id', 'fake_key', 24)) @mock.patch('refstack.db.sqlalchemy.api.models') @mock.patch.object(api, 'get_session') - def test_save_test_meta_item(self, mock_get_session, mock_models): + def test_save_test_result_meta_item(self, mock_get_session, mock_models): session = mock_get_session.return_value mock_meta_item = mock.Mock() session.query.return_value\ .filter_by.return_value\ .filter_by.return_value\ .first.return_value = mock_meta_item - db.save_test_meta_item('fake_id', 'fake_key', 42) + db.save_test_result_meta_item('fake_id', 'fake_key', 42) self.assertEqual('fake_id', mock_meta_item.test_id) self.assertEqual('fake_key', mock_meta_item.meta_key) self.assertEqual(42, mock_meta_item.value) @@ -306,21 +308,21 @@ class DBBackendTestCase(base.BaseTestCase): .first.return_value = None mock_meta_item = mock.Mock() mock_models.TestMeta.return_value = mock_meta_item - db.save_test_meta_item('fake_id', 'fake_key', 42) + db.save_test_result_meta_item('fake_id', 'fake_key', 42) self.assertEqual('fake_id', mock_meta_item.test_id) self.assertEqual('fake_key', mock_meta_item.meta_key) self.assertEqual(42, mock_meta_item.value) @mock.patch('refstack.db.sqlalchemy.api.models') @mock.patch.object(api, 'get_session') - def test_delete_test_meta_item(self, mock_get_session, mock_models): + def test_delete_test_result_meta_item(self, mock_get_session, mock_models): session = mock_get_session.return_value mock_meta_item = mock.Mock() session.query.return_value\ .filter_by.return_value\ .filter_by.return_value\ .first.return_value = mock_meta_item - db.delete_test_meta_item('fake_id', 'fake_key') + db.delete_test_result_meta_item('fake_id', 'fake_key') session.begin.assert_called_once_with() session.delete.assert_called_once_with(mock_meta_item) @@ -329,7 +331,8 @@ class DBBackendTestCase(base.BaseTestCase): .filter_by.return_value\ .first.return_value = None self.assertRaises(db.NotFound, - db.delete_test_meta_item, 'fake_id', 'fake_key') + db.delete_test_result_meta_item, + 'fake_id', 'fake_key') @mock.patch.object(api, 'get_session') @mock.patch('refstack.db.sqlalchemy.models.TestResults') @@ -446,9 +449,9 @@ class DBBackendTestCase(base.BaseTestCase): @mock.patch.object(api, '_apply_filters_for_query') @mock.patch.object(api, 'get_session') @mock.patch('refstack.db.sqlalchemy.models.Test') - def test_get_test_records(self, mock_model, - mock_get_session, - mock_apply): + def test_get_test_result_records(self, mock_model, + mock_get_session, + mock_apply): per_page = 9000 filters = { @@ -464,7 +467,7 @@ class DBBackendTestCase(base.BaseTestCase): query_with_offset = ordered_query.offset.return_value query_with_offset.limit.return_value.all.return_value = 'fake_uploads' - result = api.get_test_records(2, per_page, filters) + result = api.get_test_result_records(2, per_page, filters) mock_get_session.assert_called_once_with() session.query.assert_called_once_with(mock_model) @@ -479,9 +482,9 @@ class DBBackendTestCase(base.BaseTestCase): @mock.patch.object(api, '_apply_filters_for_query') @mock.patch.object(api, 'get_session') @mock.patch('refstack.db.sqlalchemy.models.Test') - def test_get_test_records_count(self, mock_model, - mock_get_session, - mock_apply): + def test_get_test_result_records_count(self, mock_model, + mock_get_session, + mock_apply): filters = mock.Mock() session = mock_get_session.return_value @@ -489,7 +492,7 @@ class DBBackendTestCase(base.BaseTestCase): apply_result = mock_apply.return_value apply_result.count.return_value = 999 - result = api.get_test_records_count(filters) + result = api.get_test_result_records_count(filters) self.assertEqual(result, 999) session.query.assert_called_once_with(mock_model.id)