----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70755/#review215810 -----------------------------------------------------------
Fix it, then Ship it! src/tests/master/update_framework_tests.cpp Lines 627 (patched) <https://reviews.apache.org/r/70755/#comment302695> .WillOnce on the next line src/tests/master/update_framework_tests.cpp Lines 654-655 (patched) <https://reviews.apache.org/r/70755/#comment302696> No need to create and pass in the agent flags? They're not being used below? src/tests/master/update_framework_tests.cpp Lines 664-666 (patched) <https://reviews.apache.org/r/70755/#comment302698> Same race as the v1 tests, this needs to be before StartSlave, otherwise the agent might show up in the initial SUBSCRIBE state src/tests/master/update_framework_tests.cpp Lines 690-691 (patched) <https://reviews.apache.org/r/70755/#comment302699> V0 scheduler event? I guess you mean the master acknowledging the update? We may never add that, so probably we should just remove the TODO src/tests/master/update_framework_tests.cpp Lines 696 (patched) <https://reviews.apache.org/r/70755/#comment302700> This comment seems a little confusing, it made me think that we're about to test that the ID set gets rejected, but instead it's just adding it for comparision purposes. src/tests/master/update_framework_tests.cpp Lines 714 (patched) <https://reviews.apache.org/r/70755/#comment302701> newline and maybe a comment that we sanity check that it's different from the default? seems a bit odd to check here as opposed to earlier when calling changeAllMutableFields? src/tests/master/update_framework_tests.cpp Lines 749-750 (patched) <https://reviews.apache.org/r/70755/#comment302704> Ditto here. src/tests/master/update_framework_tests.cpp Lines 780 (patched) <https://reviews.apache.org/r/70755/#comment302702> do you need to resume here for the test to pass? src/tests/master/update_framework_tests.cpp Lines 795 (patched) <https://reviews.apache.org/r/70755/#comment302703> does s/1ul/1u/ not work? I think we do this in other tests src/tests/master/update_framework_tests.cpp Lines 809-810 (patched) <https://reviews.apache.org/r/70755/#comment302705> Ditto here. src/tests/master/update_framework_tests.cpp Lines 813-818 (patched) <https://reviews.apache.org/r/70755/#comment302706> ``` MockScheduler sched; // Expect an offer exactly once (after subscribing). Future<std::vector<Offer>> offers; ``` src/tests/master/update_framework_tests.cpp Lines 824 (patched) <https://reviews.apache.org/r/70755/#comment302707> newline src/tests/master/update_framework_tests.cpp Lines 825 (patched) <https://reviews.apache.org/r/70755/#comment302708> Ditto here. src/tests/master/update_framework_tests.cpp Lines 836-837 (patched) <https://reviews.apache.org/r/70755/#comment302709> // TODO(asekretenko): Add a more in-depth check that // the allocator does what it should. - Benjamin Mahler On June 3, 2019, 8:16 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70755/ > ----------------------------------------------------------- > > (Updated June 3, 2019, 8:16 p.m.) > > > Review request for mesos, Benjamin Mahler and Meng Zhu. > > > Bugs: MESOS-9793 > https://issues.apache.org/jira/browse/MESOS-9793 > > > Repository: mesos > > > Description > ------- > > Added tests for the V0 UPDATE_FRAMEWORK scheduler call. > > > Diffs > ----- > > src/tests/master/update_framework_tests.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/70755/diff/2/ > > > Testing > ------- > > > Thanks, > > Andrei Sekretenko > >
