-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70156/#review213530
-----------------------------------------------------------



Hum... I wonder if it is worthwhile to make this a normal function call instead.
```
<template T>
bool metricsEqual(string metric, T value) {
  JSON::Object metrics = Metrics();
  if (metrics.count(metric) != 1) {
    return false;
  }
  
  return metrics.at<JSON::Number>(metric)->as<T>() == value;
}
```
With usage like:
```
ASSERT_TRUE(metricsEqual("metric/foo", 1));
```


src/tests/utils.hpp
Lines 39 (patched)
<https://reviews.apache.org/r/70156/#comment299481>

    Nit: A glance at some other macros show that we space-align the `` 
characters to the right side.



src/tests/utils.hpp
Lines 40 (patched)
<https://reviews.apache.org/r/70156/#comment299480>

    Is it necessary to make this a `do { ... } while (0);` instead of just 
enclosing this in a `{ ... }`?



src/tests/utils.hpp
Lines 41 (patched)
<https://reviews.apache.org/r/70156/#comment299482>

    I can imagine a name conflict if the test is particularly metric heavy.  
i.e. if the test defines another `metrics` local variable.


- Joseph Wu


On March 7, 2019, 8:59 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70156/
> -----------------------------------------------------------
> 
> (Updated March 7, 2019, 8:59 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added helper to test for metrics values.
> 
> 
> Diffs
> -----
> 
>   src/tests/utils.hpp b2f22cd4db223d167aa35109cd8de6df82ed1f4d 
> 
> 
> Diff: https://reviews.apache.org/r/70156/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Reply via email to