chenBright commented on code in PR #3611:
URL: https://github.com/apache/brpc/pull/3611#discussion_r4307363193
##########
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:
The cumulative snapshot alone did not prove that the window returned a
non-zero value.
I've changed this test to use an unscheduled sampler and advance sampling
manually in
6f848ba. It now verifies that the same window returns `1` after the first
round and `0` after the next, including the expected sum and bucket counts.
This removes the dependency on thread scheduling. The asynchronous window test
retains its 30-second timeout.
--
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]
