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




src/tests/sorter_tests.cpp
Lines 2188 (patched)
<https://reviews.apache.org/r/71313/#comment304592>

    Could we call this RoleTracking and either cover all of the lifecycle cases 
(weight, quota, frameworks, and reservations) or have small TrackingByWeight, 
TrackingByQuota, etc for each one?
    
    It will be a lot less verbose if we avoid the json?
    
    ```
      RoleTree roleTree;
      roleTree.updateWeight("a/b/c", 2.0);
      
      EXPECT_SOME(roleTree.get("a"));
      EXPECT_SOME(roleTree.get("a/b"));
      EXPECT_SOME(roleTree.get("a/b/c"));
      
      roleTree.updateWeight("a/b/c", DEFAULT_WEIGHT);
    
      EXPECT_NONE(roleTree.get("a/b/c"));
      EXPECT_NONE(roleTree.get("a/b"));
      EXPECT_NONE(roleTree.get("a"));
      
      etc
    ```



src/tests/sorter_tests.cpp
Lines 2202-2205 (patched)
<https://reviews.apache.org/r/71313/#comment304591>

    Consider omitting some of these fields and using .contains instead? That 
should be more succinct and we don't care about the other fields for lifecycle 
management?
    
    
https://github.com/apache/mesos/blob/1.8.1/3rdparty/stout/include/stout/json.hpp#L289-L311
    
    You can find examples in other tests too


- Benjamin Mahler


On Aug. 19, 2019, 9:08 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71313/
> -----------------------------------------------------------
> 
> (Updated Aug. 19, 2019, 9:08 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a test for RoleTree for basic add and remove operations.
> 
> 
> Diffs
> -----
> 
>   src/tests/sorter_tests.cpp fc33971758d99192dc2c1e4d78b41edd14eaa1fa 
> 
> 
> Diff: https://reviews.apache.org/r/71313/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Reply via email to