Page MenuHomeSoftware Heritage

Add test for content_get_range.

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

Diff Detail

rDSTO Storage manager
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vlorentz created this revision.Jun 5 2019, 1:30 PM
douardda added inline comments.

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

vlorentz added inline comments.Jun 12 2019, 12:27 PM

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

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

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

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

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


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.Jul 1 2019, 1:47 PM
This revision is now accepted and ready to land.Jul 1 2019, 1:47 PM
This revision was landed with ongoing or failed builds.Jul 1 2019, 3:49 PM
This revision was automatically updated to reflect the committed changes.