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( | ||||
vlorentz: You shouldn't need an `if` in a test.
If you expect `content_find` to return an object, then… | |||||
Done Inline ActionsYou should also test the length of the list returned by content_find. (same comment on the calls below) vlorentz: You should also test the length of the list returned by `content_find`.
(same comment on the… | |||||
{'sha1': cont['sha1']} | |||||
) | |||||
self.assertEqual(1, len(actually_present)) | |||||
actually_present[0].pop('ctime') | |||||
actually_present.pop('ctime') | self.assertEqual(actually_present[0], { | ||||
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']}) | ||||
self.assertEqual(1, len(actually_present)) | |||||
actually_present.pop('ctime') | actually_present[0].pop('ctime') | ||||
self.assertEqual(actually_present, { | self.assertEqual(actually_present[0], { | ||||
'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']}) | ||||
self.assertEqual(1, len(actually_present)) | |||||
actually_present.pop('ctime') | actually_present[0].pop('ctime') | ||||
self.assertEqual(actually_present, { | self.assertEqual(actually_present[0], { | ||||
'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'], | ||||
}) | }) | ||||
self.assertEqual(1, len(actually_present)) | |||||
actually_present.pop('ctime') | actually_present[0].pop('ctime') | ||||
self.assertEqual(actually_present, { | self.assertEqual(actually_present[0], { | ||||
'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']} | |||||
actual_result = list(self.storage.content_find(finder)) | |||||
cont1.pop('data') | |||||
duplicate_cont.pop('data') | |||||
actual_result[0].pop('ctime') | |||||
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. | |||||
actual_result[1].pop('ctime') | |||||
expected_result = [ | |||||
cont1, duplicate_cont | |||||
] | |||||
self.assertCountEqual(expected_result, actual_result) | |||||
def test_content_find_with_duplicate_sha256(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) | |||||
blake2s256_array = bytearray(duplicate_cont['blake2s256']) | |||||
blake2s256_array[0] += 1 | |||||
duplicate_cont['blake2s256'] = bytes(blake2s256_array) | |||||
self.storage.content_add([cont1, duplicate_cont]) | |||||
finder = { | |||||
'sha256': duplicate_cont['sha256'] | |||||
} | |||||
actual_result = list(self.storage.content_find(finder)) | |||||
cont1.pop('data') | |||||
duplicate_cont.pop('data') | |||||
actual_result[0].pop('ctime') | |||||
actual_result[1].pop('ctime') | |||||
expected_result = [ | |||||
cont1, duplicate_cont | |||||
] | |||||
self.assertCountEqual(expected_result, actual_result) | |||||
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. | |||||
# Find with both sha256 and blake2s256 | |||||
finder = { | |||||
'sha256': duplicate_cont['sha256'], | |||||
'blake2s256': duplicate_cont['blake2s256'] | |||||
} | |||||
actual_result = list(self.storage.content_find(finder)) | |||||
actual_result[0].pop('ctime') | |||||
expected_result = [ | |||||
duplicate_cont | |||||
] | |||||
self.assertCountEqual(expected_result, actual_result) | |||||
def test_content_find_with_duplicate_blake2s256(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) | |||||
sha256_array = bytearray(duplicate_cont['sha256']) | |||||
sha256_array[0] += 1 | |||||
duplicate_cont['sha256'] = bytes(sha256_array) | |||||
self.storage.content_add([cont1, duplicate_cont]) | |||||
finder = { | |||||
'blake2s256': duplicate_cont['blake2s256'] | |||||
} | |||||
actual_result = list(self.storage.content_find(finder)) | |||||
cont1.pop('data') | |||||
duplicate_cont.pop('data') | |||||
actual_result[0].pop('ctime') | |||||
actual_result[1].pop('ctime') | |||||
expected_result = [ | |||||
cont1, duplicate_cont | |||||
] | |||||
self.assertCountEqual(expected_result, actual_result) | |||||
# Find with both sha256 and blake2s256 | |||||
finder = { | |||||
'sha256': duplicate_cont['sha256'], | |||||
'blake2s256': duplicate_cont['blake2s256'] | |||||
} | |||||
actual_result = list(self.storage.content_find(finder)) | |||||
actual_result[0].pop('ctime') | |||||
expected_result = [ | |||||
duplicate_cont | |||||
] | |||||
self.assertCountEqual(expected_result, actual_result) | |||||
def test_content_find_bad_input(self): | def test_content_find_bad_input(self): | ||||
# 1. with bad input | # 1. with bad input | ||||
Done Inline Actionssame vlorentz: same | |||||
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( | ||||
{'unknown-sha1': 'something'}) # not the right key | {'unknown-sha1': 'something'}) # not the right key | ||||
▲ Show 20 Lines • Show All 698 Lines • Show Last 20 Lines |
You 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)