Yicong-Huang commented on code in PR #8282:
URL: https://github.com/apache/texera/pull/8282#discussion_r4369234899
##########
amber/src/test/python/core/util/buffer/test_timed_buffer.py:
##########
@@ -129,6 +130,28 @@ def test_exceeding_max_message_num_triggers_flush(self,
clock):
class TestFlushOnTime:
+ def test_subsecond_interval_withholds_before_boundary(self, clock):
+ buffer = TimedBuffer(max_message_num=100, max_flush_interval_in_ms=500)
+ buffer.put(_make_message())
+ _advance(clock, 0.4)
+ assert list(buffer.get()) == []
+ assert len(buffer._buffer) == 1
+
+ def test_wall_clock_jump_does_not_change_flush_interval(self, clock):
+ buffer = TimedBuffer(max_message_num=100, max_flush_interval_in_ms=500)
+ message = _make_message()
+ buffer.put(message)
+ clock.wall -= datetime.timedelta(seconds=3600)
+ _advance(clock, 0.5)
+ assert list(buffer.get()) == [message]
+
+ def test_subsecond_interval_triggers_flush(self, clock):
+ buffer = TimedBuffer(max_message_num=100, max_flush_interval_in_ms=500)
+ buffer.put(_make_message())
+ _advance(clock, 0.5)
+ assert len(list(buffer.get())) == 1
+ assert len(buffer._buffer) == 0
+
def test_elapsed_interval_triggers_flush(self, clock):
# Interval of 2000ms -> 2.0s threshold. timedelta.seconds is an
# integer, so 3 whole seconds (>= 2.0) triggers the time-based flush.
Review Comment:
**Polish:**
Keeping this comment makes sense, but its middle clause is no longer true.
After this PR the check at `timed_buffer.py:39` is a float difference of
`monotonic()` readings, so there is no `timedelta.seconds` and no integer
truncation. A reader would conclude that sub-second intervals still round down,
which is the bug this PR fixes. Dropping only that clause keeps the original
wording otherwise intact.
```suggestion
# Interval of 2000ms -> 2.0s threshold, so 3 seconds (>= 2.0)
# triggers the time-based flush.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
