Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2735#discussion_r197652249
--- Diff: storm-core/test/clj/org/apache/storm/metrics_test.clj ---
@@ -87,12 +91,22 @@
(first) ;; pick first task in the list, ignore other tasks' metric
data.
(second)
(or [])))
-
-(defmacro assert-buckets! [comp-id metric-name expected cluster]
- `(do
- (let [N# (count ~expected)]
- (wait-for-atleast-N-buckets! N# ~comp-id ~metric-name ~cluster)
- (is (= ~expected (subvec (lookup-bucket-by-comp-id-&-metric-name!
~comp-id ~metric-name) 0 N#))))))
+
+(defn assert-metric-running-sum! [comp-id metric-name expected min-buckets
cluster]
+ (try
+ (do
+ (wait-for-atleast-N-buckets! min-buckets comp-id metric-name cluster)
+ (.until
--- End diff --
The part about the metric value not being added to exactly bucket N is
right. The reason we call `wait-for-atleast-N-buckets` is to preserve the
existing test logic as much as possible. For example, one of the replaced
`assert-buckets` were asserting that a metric had buckets `[1 0 0 0 0 0]`. The
new assert would just assert that the running sum is 1. By also waiting for 6
buckets, we are able to assert that the running sum is 1 within the first 6
buckets, rather than potentially stopping the check as soon as the first bucket
is available.
---
