Copilot commented on code in PR #3611:
URL: https://github.com/apache/brpc/pull/3611#discussion_r4306865297
##########
test/bvar_histogram_unittest.cpp:
##########
@@ -734,11 +736,46 @@ TEST_F(HistogramTest, window_forgets_old_samples) {
bvar::Histogram h(bvar::Histogram::BucketSchema({10, 20, 30}));
bvar::Window<bvar::Histogram> w(&h, 1);
h << 5;
- sleep(2);
- // The value recorded 2 seconds ago has left the 1-second window.
- ASSERT_EQ(0, w.get_value().num);
+
+ // Check the latest cumulative snapshot: it still includes the write even
+ // if the one-second window has expired before this thread can poll it.
+ // Allow up to 30 seconds at 100 ms per retry on busy test hosts.
+ bvar::Histogram::sampler_type* sampler = h.get_sampler();
+ int64_t sampled_count = 0;
+ for (int i = 0; i < 300; ++i) {
+ {
+ BAIDU_SCOPED_LOCK(sampler->_mutex);
+ const auto* latest = sampler->_q.bottom();
+ if (latest != nullptr) {
+ sampled_count = latest->data.num;
+ }
+ }
+ if (sampled_count == 1) {
+ break;
+ }
+ usleep(100 * 1000);
+ }
+ ASSERT_EQ(1, sampled_count);
Review Comment:
This phase only proves that the underlying sampler produced a cumulative
snapshot (`latest->data.num == 1`); it never verifies that the window observed
a non-zero value before the expiration phase. A regression where
`w.get_value()` always returned an empty histogram would therefore pass
immediately through the next loop and satisfy the final zero assertions.
Poll/assert the window's `num == 1` before waiting for it to become zero, while
retaining the timeout coordination needed to avoid missing that sample under
load.
--
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]
