Page MenuHomeSoftware Heritage
Differential D66 Diff 219 swh/web/ui/tests/test_utils.py

Changeset View

Standalone View

View Options

swh/web/ui/tests/test_utils.py

Show First 20 Lines • Show All 470 Lines • ▼ Show 20 Lines def enrich_entity_with_sha1(self, mock_flask):
'name': 'something', 'name': 'something',
}) })
mock_flask.url_for.assert_has_calls([call('api_entity_by_uuid', mock_flask.url_for.assert_has_calls([call('api_entity_by_uuid',
uuid='uuid-1'), uuid='uuid-1'),
call('api_entity_by_uuid', call('api_entity_by_uuid',
uuid='uuid-parent')]) uuid='uuid-parent')])
@nottest
def url_for_context_test(self, fn, **data):
ardumontUnsubmitted
Done
Inline Actions

Please, prefix it with _ as you did for the other @nottest util test function.
It is a good idea.

ardumont: Please, prefix it with _ as you did for the other @nottest util test function. It is a good…
if fn == 'api_revision':
if 'context' in data and data['context'] is not None:
return '/api/revision/%s/prev/%s/' % (data['sha1_git'], data['context']) # noqa
else:
return '/api/revision/%s/' % data['sha1_git']
elif fn == 'api_revision_log':
if 'prev_sha1s' in data:
return '/api/revision/%s/prev/%s/log/' % (data['sha1_git'], data['prev_sha1s']) # noqa
else:
return '/api/revision/%s/log/' % data['sha1_git']
@patch('swh.web.ui.utils.flask') @patch('swh.web.ui.utils.flask')
@istest @istest
def enrich_revision_without_children_or_parent(self, mock_flask): def enrich_revision_without_children_or_parent(self, mock_flask):
# given # given
def url_for_test(fn, **data): def url_for_test(fn, **data):
if fn == 'api_revision': if fn == 'api_revision' or fn == 'api_revision_log':
return '/api/revision/' + data['sha1_git'] + '/' return self.url_for_context_test(fn, **data)
ardumontUnsubmitted
Done
Inline Actions

The intent of the side-effect url_for_test function (defined in this test) is not to make the test behaves exactly the same as the production code. This would not make sense since this side-effect function is not the production code.

I just want to assert that the enrich mechanism works for a revision without children or parent entry with a relative coherence to the production code (that's why i wanted some sensible similarly like urls than production code here but again it's not necessary).

Furthermore, i see that you changed the test code to factorize (which is often good) but neither the input nor the output changed. So those modifications do not add anything to the test.

If anything, they add complexity to the maintenance (for the next person).
Can you please remove those modifications on that test only.

ardumont: The intent of the side-effect `url_for_test` function (defined in this test) is not to make the…
elif fn == 'api_revision_log':
return '/api/revision/' + data['sha1_git'] + '/log/'
elif fn == 'api_directory': elif fn == 'api_directory':
return '/api/directory/' + data['sha1_git'] + '/' return '/api/directory/' + data['sha1_git'] + '/'
elif fn == 'api_person': elif fn == 'api_person':
return '/api/person/' + data['person_id'] + '/' return '/api/person/' + data['person_id'] + '/'
mock_flask.url_for.side_effect = url_for_test mock_flask.url_for.side_effect = url_for_test
# when # when
actual_revision = utils.enrich_revision({ actual_revision = utils.enrich_revision({
'id': 'rev-id', 'id': 'rev-id',
'directory': '123', 'directory': '123',
'author': {'id': '1'}, 'author': {'id': '1'},
'committer': {'id': '2'}, 'committer': {'id': '2'},
}) })
# then expected_revision = {
self.assertEqual(actual_revision, {
'id': 'rev-id', 'id': 'rev-id',
'directory': '123', 'directory': '123',
'url': '/api/revision/rev-id/', 'url': '/api/revision/rev-id/',
'history_url': '/api/revision/rev-id/log/', 'history_url': '/api/revision/rev-id/log/',
'directory_url': '/api/directory/123/', 'directory_url': '/api/directory/123/',
'author': {'id': '1'}, 'author': {'id': '1'},
'author_url': '/api/person/1/', 'author_url': '/api/person/1/',
'committer': {'id': '2'}, 'committer': {'id': '2'},
'committer_url': '/api/person/2/' 'committer_url': '/api/person/2/'
}) }
# then
self.assertEqual(actual_revision, expected_revision)
mock_flask.url_for.assert_has_calls([call('api_revision', mock_flask.url_for.assert_has_calls(
[call('api_revision',
sha1_git='rev-id'), sha1_git='rev-id'),
call('api_revision_log', call('api_revision_log',
sha1_git='rev-id'), sha1_git='rev-id'),
call('api_person', call('api_person',
person_id='1'), person_id='1'),
call('api_person', call('api_person',
person_id='2'), person_id='2'),
call('api_directory', call('api_directory',
sha1_git='123')]) sha1_git='123')])
@patch('swh.web.ui.utils.flask') @patch('swh.web.ui.utils.flask')
@istest @istest
def enrich_revision_with_children_and_parent_no_dir(self, def enrich_revision_with_children_and_parent_no_dir(self,
mock_flask): mock_flask):
# given # given
def url_for_test(fn, **data): def url_for_test(fn, **data):
if fn == 'api_revision': if fn == 'api_revision' or fn == 'api_revision_log':
return '/api/revision/' + data['sha1_git'] + '/' return self.url_for_context_test(fn, **data)
ardumontUnsubmitted
Done
Inline Actions

This time, in this context, i agree this is necessary ^^

ardumont: This time, in this context, i agree this is necessary ^^
elif fn == 'api_revision_log':
return '/api/revision/' + data['sha1_git'] + '/log/'
else: else:
ardumontUnsubmitted
Done
Inline Actions

if the else statement is no longer used, please remove it.
Also, if it is removed, the side effect function become simply url_for_context_test.

It'll simplify later maintenance.
^^

ardumont: if the else statement is no longer used, please remove it. Also, if it is removed, the side…
return '/api/revision/' + data['sha1_git_root'] + '/history/' + data['sha1_git'] + '/' # noqa return '/api/revision/' + data['sha1_git_root'] + '/history/' + data['sha1_git'] + '/' # noqa
mock_flask.url_for.side_effect = url_for_test mock_flask.url_for.side_effect = url_for_test
# when # when
actual_revision = utils.enrich_revision({ actual_revision = utils.enrich_revision({
'id': 'rev-id', 'id': 'rev-id',
'parents': ['123'], 'parents': ['123'],
'children': ['456'], 'children': ['456'],
}, context='sha1_git_root') }, context='prev-rev')
# then expected_revision = {
self.assertEqual(actual_revision, {
'id': 'rev-id', 'id': 'rev-id',
'url': '/api/revision/rev-id/', 'url': '/api/revision/rev-id/',
'history_url': '/api/revision/rev-id/log/', 'history_url': '/api/revision/rev-id/log/',
'history_context_url': '/api/revision/rev-id/prev/prev-rev/log/',
'parents': ['123'],
'parent_urls': ['/api/revision/123/prev/prev-rev/rev-id/'],
'children': ['456'],
'children_urls': ['/api/revision/456/',
'/api/revision/prev-rev/'],
}
# then
self.assertEqual(actual_revision, expected_revision)
mock_flask.url_for.assert_has_calls(
[call('api_revision',
sha1_git='prev-rev'),
call('api_revision',
sha1_git='rev-id'),
call('api_revision_log',
sha1_git='rev-id'),
call('api_revision_log',
sha1_git='rev-id',
prev_sha1s='prev-rev'),
call('api_revision',
sha1_git='123',
context='prev-rev/rev-id'),
call('api_revision',
sha1_git='456')])
@patch('swh.web.ui.utils.flask')
@istest
def enrich_revision_no_context(self, mock_flask):
# given
def url_for_test(fn, **data):
ardumontUnsubmitted
Done
Inline Actions

Sorry to not be clearer, just pass self._url_for_context_test to mock_flask.url_for.side_effect instruction below.

ardumont: Sorry to not be clearer, just pass self._url_for_context_test to mock_flask.url_for.side_effect…
jbertranAuthorUnsubmitted
Not Done
Inline Actions

That's just me being dense, no worries ^^

jbertran: That's just me being dense, no worries ^^
if fn == 'api_revision' or fn == 'api_revision_log':
return self.url_for_context_test(fn, **data)
else:
ardumontUnsubmitted
Done
Inline Actions

Same remark as previous comment.

ardumont: Same remark as previous comment.
return '/api/revision/' + data['sha1_git_root'] + '/history/' + data['sha1_git'] + '/' # noqa
mock_flask.url_for.side_effect = url_for_test
# when
actual_revision = utils.enrich_revision({
'id': 'rev-id',
'parents': ['123'], 'parents': ['123'],
'parent_urls': ['/api/revision/sha1_git_root/history/123/'],
'children': ['456'], 'children': ['456'],
'children_urls': ['/api/revision/sha1_git_root/history/456/'],
}) })
expected_revision = {
'id': 'rev-id',
'url': '/api/revision/rev-id/',
'history_url': '/api/revision/rev-id/log/',
'parents': ['123'],
'parent_urls': ['/api/revision/123/prev/rev-id/'],
'children': ['456'],
'children_urls': ['/api/revision/456/']
}
# then
self.assertEqual(actual_revision, expected_revision)
mock_flask.url_for.assert_has_calls(
[call('api_revision',
sha1_git='rev-id'),
call('api_revision_log',
sha1_git='rev-id'),
call('api_revision',
sha1_git='123',
context='rev-id'),
call('api_revision',
sha1_git='456')])
@patch('swh.web.ui.utils.flask')
@istest
def enrich_revision_context_empty_prev_list(self, mock_flask):
# given
mock_flask.url_for.side_effect = self.url_for_context_test
# when
expected_revision = {
'id': 'rev-id',
'url': '/api/revision/rev-id/',
'history_url': '/api/revision/rev-id/log/',
'history_context_url': ('/api/revision/rev-id/'
'prev/prev-rev/log/'),
'parents': ['123'],
'parent_urls': ['/api/revision/123/prev/prev-rev/rev-id/'],
'children': ['456'],
'children_urls': ['/api/revision/456/', '/api/revision/prev-rev/'],
}
actual_revision = utils.enrich_revision({
'id': 'rev-id',
'url': '/api/revision/rev-id/',
'parents': ['123'],
'children': ['456']}, context='prev-rev')
# then
self.assertEqual(actual_revision, expected_revision)
mock_flask.url_for.assert_has_calls( mock_flask.url_for.assert_has_calls(
[call('api_revision', [call('api_revision',
sha1_git='prev-rev'),
call('api_revision',
sha1_git='rev-id'), sha1_git='rev-id'),
call('api_revision_log', call('api_revision_log',
sha1_git='rev-id'), sha1_git='rev-id'),
call('api_revision_history', call('api_revision_log',
sha1_git_root='sha1_git_root', sha1_git='rev-id',
sha1_git='123'), prev_sha1s='prev-rev'),
call('api_revision_history', call('api_revision',
sha1_git_root='sha1_git_root', sha1_git='123',
context='prev-rev/rev-id'),
call('api_revision',
sha1_git='456')])
@patch('swh.web.ui.utils.flask')
@istest
def enrich_revision_context_some_prev_list(self, mock_flask):
# given
mock_flask.url_for.side_effect = self.url_for_context_test
# when
expected_revision = {
'id': 'rev-id',
'url': '/api/revision/rev-id/',
'history_url': '/api/revision/rev-id/log/',
'history_context_url': ('/api/revision/rev-id/'
'prev/prev1-rev/prev0-rev/log/'),
'parents': ['123'],
'parent_urls': ['/api/revision/123/prev/'
'prev1-rev/prev0-rev/rev-id/'],
'children': ['456'],
'children_urls': ['/api/revision/456/',
'/api/revision/prev0-rev/prev/prev1-rev/'],
}
actual_revision = utils.enrich_revision({
'id': 'rev-id',
'parents': ['123'],
'children': ['456']}, context='prev1-rev/prev0-rev')
# then
self.assertEqual(actual_revision, expected_revision)
mock_flask.url_for.assert_has_calls(
[call('api_revision',
sha1_git='prev0-rev',
context='prev1-rev'),
call('api_revision',
sha1_git='rev-id'),
call('api_revision_log',
sha1_git='rev-id'),
call('api_revision_log',
sha1_git='rev-id',
prev_sha1s='prev1-rev/prev0-rev'),
call('api_revision',
sha1_git='123',
context='prev1-rev/prev0-rev/rev-id'),
call('api_revision',
sha1_git='456')]) sha1_git='456')])
@nottest @nottest
def _url_for_rev_message_test(self, fn, **data): def _url_for_rev_message_test(self, fn, **data):
if fn == 'api_revision': if fn == 'api_revision':
return '/api/revision/' + data['sha1_git'] + '/' if 'context' in data and data['context'] is not None:
return '/api/revision/%s/prev/%s/' % (data['sha1_git'], data['context']) # noqa
else:
return '/api/revision/%s/' % data['sha1_git']
elif fn == 'api_revision_log': elif fn == 'api_revision_log':
return '/api/revision/' + data['sha1_git'] + '/log/' if 'prev_sha1s' in data and data['prev_sha1s'] is not None:
return '/api/revision/%s/prev/%s/log/' % (data['sha1_git'], data['prev_sha1s']) # noqa
else:
return '/api/revision/%s/log/' % data['sha1_git']
elif fn == 'api_revision_raw_message': elif fn == 'api_revision_raw_message':
return '/api/revision/' + data['sha1_git'] + '/raw/' return '/api/revision/' + data['sha1_git'] + '/raw/'
else: else:
return '/api/revision/' + data['sha1_git_root'] + '/history/' + data['sha1_git'] + '/' # noqa return '/api/revision/' + data['sha1_git_root'] + '/history/' + data['sha1_git'] + '/' # noqa
@patch('swh.web.ui.utils.flask') @patch('swh.web.ui.utils.flask')
@istest @istest
def enrich_revision_with_no_message(self, mock_flask): def enrich_revision_with_no_message(self, mock_flask):
# given # given
mock_flask.url_for.side_effect = self._url_for_rev_message_test mock_flask.url_for.side_effect = self._url_for_rev_message_test
# when # when
actual_revision = utils.enrich_revision({ expected_revision = {
'id': 'rev-id', 'id': 'rev-id',
'url': '/api/revision/rev-id/',
'history_url': '/api/revision/rev-id/log/',
'history_context_url': ('/api/revision/rev-id/'
'prev/prev-rev/log/'),
'message': None, 'message': None,
'parents': ['123'], 'parents': ['123'],
'parent_urls': ['/api/revision/123/prev/prev-rev/rev-id/'],
'children': ['456'], 'children': ['456'],
}, context='sha1_git_root') 'children_urls': ['/api/revision/456/', '/api/revision/prev-rev/'],
}
# then actual_revision = utils.enrich_revision({
self.assertEqual(actual_revision, {
'id': 'rev-id', 'id': 'rev-id',
'url': '/api/revision/rev-id/',
'message': None, 'message': None,
'history_url': '/api/revision/rev-id/log/',
'parents': ['123'], 'parents': ['123'],
'parent_urls': ['/api/revision/sha1_git_root/history/123/'],
'children': ['456'], 'children': ['456'],
'children_urls': ['/api/revision/sha1_git_root/history/456/'], }, context='prev-rev')
})
# then
self.assertEqual(actual_revision, expected_revision)
mock_flask.url_for.assert_has_calls( mock_flask.url_for.assert_has_calls(
[call('api_revision', [call('api_revision',
sha1_git='prev-rev'),
call('api_revision',
sha1_git='rev-id'), sha1_git='rev-id'),
call('api_revision_log', call('api_revision_log',
sha1_git='rev-id'), sha1_git='rev-id'),
call('api_revision_history', call('api_revision_log',
sha1_git_root='sha1_git_root', sha1_git='rev-id',
sha1_git='123'), prev_sha1s='prev-rev'),
call('api_revision_history', call('api_revision',
sha1_git_root='sha1_git_root', sha1_git='123',
context='prev-rev/rev-id'),
call('api_revision',
sha1_git='456')]) sha1_git='456')])
@patch('swh.web.ui.utils.flask') @patch('swh.web.ui.utils.flask')
@istest @istest
def enrich_revision_with_invalid_message(self, mock_flask): def enrich_revision_with_invalid_message(self, mock_flask):
# given # given
mock_flask.url_for.side_effect = self._url_for_rev_message_test mock_flask.url_for.side_effect = self._url_for_rev_message_test
# when # when
actual_revision = utils.enrich_revision({ actual_revision = utils.enrich_revision({
'id': 'rev-id', 'id': 'rev-id',
'message': None, 'message': None,
'message_decoding_failed': True, 'message_decoding_failed': True,
'parents': ['123'], 'parents': ['123'],
'children': ['456'], 'children': ['456'],
}, context='sha1_git_root') }, context='prev-rev')
# then expected_revision = {
self.assertEqual(actual_revision, {
'id': 'rev-id', 'id': 'rev-id',
'url': '/api/revision/rev-id/', 'url': '/api/revision/rev-id/',
'history_url': '/api/revision/rev-id/log/',
'history_context_url': ('/api/revision/rev-id/'
'prev/prev-rev/log/'),
'message': None, 'message': None,
'message_decoding_failed': True, 'message_decoding_failed': True,
'message_url': '/api/revision/rev-id/raw/', 'message_url': '/api/revision/rev-id/raw/',
'history_url': '/api/revision/rev-id/log/',
'parents': ['123'], 'parents': ['123'],
'parent_urls': ['/api/revision/sha1_git_root/history/123/'], 'parent_urls': ['/api/revision/123/prev/prev-rev/rev-id/'],
'children': ['456'], 'children': ['456'],
'children_urls': ['/api/revision/sha1_git_root/history/456/'], 'children_urls': ['/api/revision/456/', '/api/revision/prev-rev/'],
}) }
# then
self.assertEqual(actual_revision, expected_revision)
mock_flask.url_for.assert_has_calls( mock_flask.url_for.assert_has_calls(
[call('api_revision', [call('api_revision',
sha1_git='prev-rev'),
call('api_revision',
sha1_git='rev-id'), sha1_git='rev-id'),
call('api_revision_log', call('api_revision_log',
sha1_git='rev-id'), sha1_git='rev-id'),
call('api_revision_history', call('api_revision_log',
sha1_git_root='sha1_git_root', sha1_git='rev-id',
sha1_git='123'), prev_sha1s='prev-rev'),
call('api_revision_history', call('api_revision',
sha1_git_root='sha1_git_root', sha1_git='123',
context='prev-rev/rev-id'),
call('api_revision',
sha1_git='456')]) sha1_git='456')])
About · Developer information · Legal