Page MenuHomeSoftware Heritage

Add test for content_get_range.
ClosedPublic

Authored by vlorentz on Jun 5 2019, 1:30 PM.

Diff Detail

Repository
rDSTO Storage manager
Branch
test-content_get_range
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 6030
Build 8292: tox-on-jenkinsJenkins
Build 8291: arc lint + arc unit

Event Timeline

vlorentz created this revision.Jun 5 2019, 1:30 PM
douardda added inline comments.
swh/storage/tests/test_storage.py
3174

probably a stupid question but why do we expect 3 here (instead of 4)?

vlorentz added inline comments.Jun 12 2019, 12:27 PM
swh/storage/tests/test_storage.py
3174

Because we're getting contents in range get_sha1s[1] to get_sha1s[-1] inclusive, so that's all the contents (4) minus the first one.

douardda added inline comments.Jun 13 2019, 2:14 PM
swh/storage/tests/test_storage.py
3174

oh yes indeed.

Wouldn't it be useful (?) to raise the max_size and check the content_get_range against all situations wrt range limits: the beginning, middle and end of the range (like range(0, 2), range(n, n+2) and range(-3, -1) )?

vlorentz added inline comments.Jun 13 2019, 2:17 PM
swh/storage/tests/test_storage.py
3174

That's what the three tests are for (test_generate_content_get_range_start, test_generate_content_get_range_end, test_origin_get_range). I did not want to do a single test for all of them, because it means writing the same logic in the test as in the real code, so testing makes less sense

douardda added inline comments.Jun 13 2019, 2:30 PM
swh/storage/tests/test_storage.py
3174

Oh you mean I must read the whole diff when I review it? what a bama.

So now, why does the range_start test uses sha1[1] as start value?

vlorentz added inline comments.Jun 14 2019, 10:56 AM
swh/storage/tests/test_storage.py
3174

range_start tests that nothing before the start is returned, range_end tests that nothing after the end is returned.

3174

Eh, you're right, I could merge these two tests

vlorentz updated this revision to Diff 5234.Jun 14 2019, 12:21 PM

Merge both tests, and use more input parameters to test all cases at once.

douardda accepted this revision.Mon, Jul 1, 1:47 PM
This revision is now accepted and ready to land.Mon, Jul 1, 1:47 PM
This revision was landed with ongoing or failed builds.Mon, Jul 1, 3:49 PM
This revision was automatically updated to reflect the committed changes.