Copilot commented on code in PR #3651:
URL: https://github.com/apache/brpc/pull/3651#discussion_r4369846468
##########
test/bvar_recorder_unittest.cpp:
##########
@@ -222,6 +224,19 @@ class LatencyRecorderWithWindowSample : public
bvar::LatencyRecorder {
bool get_latency_sample(bvar::detail::Sample<bvar::Stat>* sample) const {
return _latency_window.get_span(sample);
}
+
+ bool get_latency_sample(time_t window_size,
+ bvar::detail::Sample<bvar::Stat>* sample) const {
+ return _latency_window.get_span(window_size, sample);
+ }
+
+ bool has_full_latency_window() const {
+ std::vector<bvar::Stat> samples;
+ _latency_window.get_samples(&samples);
+ // get_samples() excludes the oldest snapshot used by get_span().
+ return samples.size() + 1 ==
+ static_cast<size_t>(_latency_window.window_size());
Review Comment:
`get_samples()` does not prove that `window_size + 1` snapshots exist: when
the queue has only `window_size` snapshots, `ReducerSampler::get_samples` falls
back to the oldest available snapshot and returns `window_size - 1` entries, so
this equality is already true. For example, a 2-second recorder is accepted
after only the initial sample plus one sampler tick, and `expected_qps` is then
derived from a one-second span. This makes the test no longer verify the
configured 2/3-second windows; use a check that distinguishes the fallback case
(for example, compare against a `window_size + 1` span or expose the snapshot
count).
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
