lmmx / range-streams

Streaming range requests in Python
https://range-streams.readthedocs.io/en/latest/
MIT License
8 stars 0 forks source link

Review correctness and simplify tests #20

Closed lmmx closed 3 years ago

lmmx commented 3 years ago

Head overlaps:

Tail overlaps:

I think some new fixtures would help simplify the testing (perhaps even break them out into a module for simplicity?)

lmmx commented 3 years ago

I… don’t think the internal _ranges keys are meant to be different from the RangeResponse range, but they are? And this is the source of the bug in the tail overlap case, which arises when the tail mark gets applied multiple times (once internally, I think in some place where internal and external get mixed up maybe)

This does not happen in the head-overlap case (the _ranges keys match the ranges on the values at those keys in the RangeResponse)

lmmx commented 3 years ago

I at first suspected the bug is coming from this line:

https://github.com/lmmx/range-streams/blob/56956498cf30ab27820dd2bbd6c64a8c858d24c5/src/range_streams/overlaps.py#L70

I also think that when the test for handle_overlaps stopped working I should have spotted that sign, and not replaced it with a working test(!)

The old test can be found here:

https://github.com/lmmx/range-streams/commit/56956498cf30ab27820dd2bbd6c64a8c858d24c5?branch=56956498cf30ab27820dd2bbd6c64a8c858d24c5&diff=split#diff-5c553c0de613ae24a06f65d61cabb177adf5d727c24a67c6a72975dffa8dbfd5

However note that that test is for overlaps at the head, and the error is arising from an overlap at the tail? So maybe I’m wrong