Changeset View
Changeset View
Standalone View
Standalone View
swh/storage/tests/test_storage.py
Show First 20 Lines • Show All 2,402 Lines • ▼ Show 20 Lines | def test_content_find_ctime(self): | ||||
cont = self.cont.copy() | cont = self.cont.copy() | ||||
del cont['data'] | del cont['data'] | ||||
now = datetime.datetime.now(tz=datetime.timezone.utc) | now = datetime.datetime.now(tz=datetime.timezone.utc) | ||||
cont['ctime'] = now | cont['ctime'] = now | ||||
self.storage.content_add_metadata([cont]) | self.storage.content_add_metadata([cont]) | ||||
actually_present = self.storage.content_find({'sha1': cont['sha1']}) | actually_present = self.storage.content_find({'sha1': cont['sha1']}) | ||||
self.assertEqual(actually_present, { | self.assertEqual(actually_present[0], { | ||||
'ctime': now, | 'ctime': now, | ||||
'sha1': cont['sha1'], | 'sha1': cont['sha1'], | ||||
'sha256': cont['sha256'], | 'sha256': cont['sha256'], | ||||
'sha1_git': cont['sha1_git'], | 'sha1_git': cont['sha1_git'], | ||||
'blake2s256': cont['blake2s256'], | 'blake2s256': cont['blake2s256'], | ||||
'length': cont['length'], | 'length': cont['length'], | ||||
'status': 'visible' | 'status': 'visible' | ||||
}) | }) | ||||
def test_content_find_with_present_content(self): | def test_content_find_with_present_content(self): | ||||
# 1. with something to find | # 1. with something to find | ||||
cont = self.cont | cont = self.cont | ||||
self.storage.content_add([cont]) | self.storage.content_add([cont]) | ||||
actually_present = self.storage.content_find({'sha1': cont['sha1']}) | actually_present = self.storage.content_find( | ||||
{'sha1': cont['sha1']} | |||||
)[0] | |||||
vlorentz: You should also test the length of the list returned by `content_find`.
(same comment on the… | |||||
actually_present.pop('ctime') | actually_present.pop('ctime') | ||||
Done Inline ActionsYou shouldn't need an if in a test. If you expect content_find to return an object, then assume it returns an object. (same comment on all the conditionals below) vlorentz: You shouldn't need an `if` in a test.
If you expect `content_find` to return an object, then… | |||||
self.assertEqual(actually_present, { | self.assertEqual(actually_present, { | ||||
'sha1': cont['sha1'], | 'sha1': cont['sha1'], | ||||
'sha256': cont['sha256'], | 'sha256': cont['sha256'], | ||||
'sha1_git': cont['sha1_git'], | 'sha1_git': cont['sha1_git'], | ||||
'blake2s256': cont['blake2s256'], | 'blake2s256': cont['blake2s256'], | ||||
'length': cont['length'], | 'length': cont['length'], | ||||
'status': 'visible' | 'status': 'visible' | ||||
}) | }) | ||||
# 2. with something to find | # 2. with something to find | ||||
actually_present = self.storage.content_find( | actually_present = self.storage.content_find( | ||||
{'sha1_git': cont['sha1_git']}) | {'sha1_git': cont['sha1_git']})[0] | ||||
actually_present.pop('ctime') | actually_present.pop('ctime') | ||||
self.assertEqual(actually_present, { | self.assertEqual(actually_present, { | ||||
'sha1': cont['sha1'], | 'sha1': cont['sha1'], | ||||
'sha256': cont['sha256'], | 'sha256': cont['sha256'], | ||||
'sha1_git': cont['sha1_git'], | 'sha1_git': cont['sha1_git'], | ||||
'blake2s256': cont['blake2s256'], | 'blake2s256': cont['blake2s256'], | ||||
'length': cont['length'], | 'length': cont['length'], | ||||
'status': 'visible' | 'status': 'visible' | ||||
}) | }) | ||||
# 3. with something to find | # 3. with something to find | ||||
actually_present = self.storage.content_find( | actually_present = self.storage.content_find( | ||||
{'sha256': cont['sha256']}) | {'sha256': cont['sha256']})[0] | ||||
actually_present.pop('ctime') | actually_present.pop('ctime') | ||||
self.assertEqual(actually_present, { | self.assertEqual(actually_present, { | ||||
'sha1': cont['sha1'], | 'sha1': cont['sha1'], | ||||
'sha256': cont['sha256'], | 'sha256': cont['sha256'], | ||||
'sha1_git': cont['sha1_git'], | 'sha1_git': cont['sha1_git'], | ||||
'blake2s256': cont['blake2s256'], | 'blake2s256': cont['blake2s256'], | ||||
'length': cont['length'], | 'length': cont['length'], | ||||
'status': 'visible' | 'status': 'visible' | ||||
}) | }) | ||||
# 4. with something to find | # 4. with something to find | ||||
actually_present = self.storage.content_find({ | actually_present = self.storage.content_find({ | ||||
'sha1': cont['sha1'], | 'sha1': cont['sha1'], | ||||
'sha1_git': cont['sha1_git'], | 'sha1_git': cont['sha1_git'], | ||||
'sha256': cont['sha256'], | 'sha256': cont['sha256'], | ||||
'blake2s256': cont['blake2s256'], | 'blake2s256': cont['blake2s256'], | ||||
}) | })[0] | ||||
actually_present.pop('ctime') | actually_present.pop('ctime') | ||||
self.assertEqual(actually_present, { | self.assertEqual(actually_present, { | ||||
'sha1': cont['sha1'], | 'sha1': cont['sha1'], | ||||
'sha256': cont['sha256'], | 'sha256': cont['sha256'], | ||||
'sha1_git': cont['sha1_git'], | 'sha1_git': cont['sha1_git'], | ||||
'blake2s256': cont['blake2s256'], | 'blake2s256': cont['blake2s256'], | ||||
'length': cont['length'], | 'length': cont['length'], | ||||
'status': 'visible' | 'status': 'visible' | ||||
}) | }) | ||||
def test_content_find_with_non_present_content(self): | def test_content_find_with_non_present_content(self): | ||||
# 1. with something that does not exist | # 1. with something that does not exist | ||||
missing_cont = self.missing_cont | missing_cont = self.missing_cont | ||||
actually_present = self.storage.content_find( | actually_present = self.storage.content_find( | ||||
{'sha1': missing_cont['sha1']}) | {'sha1': missing_cont['sha1']}) | ||||
self.assertIsNone(actually_present) | self.assertIsNone(actually_present) | ||||
# 2. with something that does not exist | # 2. with something that does not exist | ||||
actually_present = self.storage.content_find( | actually_present = self.storage.content_find( | ||||
{'sha1_git': missing_cont['sha1_git']}) | {'sha1_git': missing_cont['sha1_git']}) | ||||
self.assertIsNone(actually_present) | self.assertIsNone(actually_present) | ||||
# 3. with something that does not exist | # 3. with something that does not exist | ||||
actually_present = self.storage.content_find( | actually_present = self.storage.content_find( | ||||
{'sha256': missing_cont['sha256']}) | {'sha256': missing_cont['sha256']}) | ||||
self.assertIsNone(actually_present) | self.assertIsNone(actually_present) | ||||
def test_content_find_with_duplicate_input(self): | |||||
cont1 = self.cont | |||||
duplicate_cont = cont1.copy() | |||||
# Create fake data with colliding sha256 and blake2s256 | |||||
sha1_array = bytearray(duplicate_cont['sha1']) | |||||
sha1_array[0] += 1 | |||||
duplicate_cont['sha1'] = bytes(sha1_array) | |||||
sha1git_array = bytearray(duplicate_cont['sha1_git']) | |||||
sha1git_array[0] += 1 | |||||
duplicate_cont['sha1_git'] = bytes(sha1git_array) | |||||
# Inject the data | |||||
self.storage.content_add([cont1, duplicate_cont]) | |||||
finder = {'blake2s256': duplicate_cont['blake2s256'], | |||||
'sha256': duplicate_cont['sha256']} | |||||
con = list(self.storage.content_find(finder)) | |||||
# con might return more than two colliding hashes sometimes | |||||
self.assertEqual(con[0]['blake2s256'], con[1]['blake2s256']) | |||||
self.assertEqual(con[0]['sha256'], con[1]['sha256']) | |||||
Done Inline ActionsYou know the length of the expected return value of self.storage.content_find (2), no need for a for loop that runs only twice. (Because, if there is a bug in content_find that makes it return only a single element, the for loop will not run, and the test won't catch the bug) vlorentz: You know the length of the expected return value of `self.storage.content_find` (2), no need… | |||||
Done Inline Actionsonly once * vlorentz: only once * | |||||
Done Inline ActionsYou should fully test the return value of content_find: result = list(self.storage.content_find(finder)) expected_result = [ { ... }, { ... }, ] self.assertEqual(expected_result, actual_result) vlorentz: You should fully test the return value of `content_find`:
```
result = list(self.storage. | |||||
def test_content_find_bad_input(self): | def test_content_find_bad_input(self): | ||||
# 1. with bad input | # 1. with bad input | ||||
with self.assertRaises(ValueError): | with self.assertRaises(ValueError): | ||||
self.storage.content_find({}) # empty is bad | self.storage.content_find({}) # empty is bad | ||||
# 2. with bad input | # 2. with bad input | ||||
with self.assertRaises(ValueError): | with self.assertRaises(ValueError): | ||||
self.storage.content_find( | self.storage.content_find( | ||||
Show All 17 Lines | def test_object_find_by_sha1_git(self): | ||||
sha1_gits.append(self.dir['id']) | sha1_gits.append(self.dir['id']) | ||||
expected[self.dir['id']] = [{ | expected[self.dir['id']] = [{ | ||||
'sha1_git': self.dir['id'], | 'sha1_git': self.dir['id'], | ||||
'type': 'directory', | 'type': 'directory', | ||||
'id': self.dir['id'], | 'id': self.dir['id'], | ||||
}] | }] | ||||
self.storage.revision_add([self.revision]) | self.storage.revision_add([self.revision]) | ||||
sha1_gits.append(self.revision['id']) | sha1_gits.append(self.revision['id']) | ||||
Done Inline ActionsYou don't need to compare the length, assertCountEqual does it already. vlorentz: You don't need to compare the length, assertCountEqual does it already. | |||||
expected[self.revision['id']] = [{ | expected[self.revision['id']] = [{ | ||||
'sha1_git': self.revision['id'], | 'sha1_git': self.revision['id'], | ||||
'type': 'revision', | 'type': 'revision', | ||||
'id': self.revision['id'], | 'id': self.revision['id'], | ||||
}] | }] | ||||
self.storage.release_add([self.release]) | self.storage.release_add([self.release]) | ||||
sha1_gits.append(self.release['id']) | sha1_gits.append(self.release['id']) | ||||
▲ Show 20 Lines • Show All 41 Lines • ▼ Show 20 Lines | def test_tool_add_multiple(self): | ||||
tool = { | tool = { | ||||
'name': 'some-unknown-tool', | 'name': 'some-unknown-tool', | ||||
'version': 'some-version', | 'version': 'some-version', | ||||
'configuration': {"debian-package": "some-package"}, | 'configuration': {"debian-package": "some-package"}, | ||||
} | } | ||||
actual_tools = list(self.storage.tool_add([tool])) | actual_tools = list(self.storage.tool_add([tool])) | ||||
self.assertEqual(len(actual_tools), 1) | self.assertEqual(len(actual_tools), 1) | ||||
Done Inline Actionssame vlorentz: same | |||||
new_tools = [tool, { | new_tools = [tool, { | ||||
'name': 'yet-another-tool', | 'name': 'yet-another-tool', | ||||
'version': 'version', | 'version': 'version', | ||||
'configuration': {}, | 'configuration': {}, | ||||
}] | }] | ||||
actual_tools = self.storage.tool_add(new_tools) | actual_tools = self.storage.tool_add(new_tools) | ||||
self.assertEqual(len(actual_tools), 2) | self.assertEqual(len(actual_tools), 2) | ||||
▲ Show 20 Lines • Show All 608 Lines • Show Last 20 Lines |
You should also test the length of the list returned by content_find.
(same comment on the calls below)