----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71313/#review217306 -----------------------------------------------------------
Fix it, then Ship it! Thanks! We could probably unit test the `empty()` function (for each case), then for role tracking, we could use one case (e.g. weights) to test more cases. Specifically, right now we are only testing the case wehre removal walks all the way up to the root, but we're missing the case where it has to stop because an ancestor is !empty. src/tests/sorter_tests.cpp Lines 2189 (patched) <https://reviews.apache.org/r/71313/#comment304597> brace on newline s/TrackingRoles/RolesTracking/ src/tests/sorter_tests.cpp Lines 2219-2221 (patched) <https://reviews.apache.org/r/71313/#comment304599> EXPECT_EQ takes expected first and actual second (and will print out accordingly), ditto on flipping all the others in this test - Benjamin Mahler On Aug. 19, 2019, 10:34 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, 10:34 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/2/ > > > Testing > ------- > > make check > > > Thanks, > > Meng Zhu > >
