----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57336/#review168056 -----------------------------------------------------------
Fix it, then Ship it! Looks good, wonder if we can just loop over the master and agent pids and use the same code to check the contents since they should be the same? src/tests/upgrade_tests.cpp Lines 475-543 (patched) <https://reviews.apache.org/r/57336/#comment240120> Hm.. have you considered having checking both the agent and master endpoint result with the same code? E.g. ``` // Check that the master and agent endpoints reflect the // update to use the `roles` field. foreach (const UPID& upid, { master.get()->pid, agent.get()->pid }) { // ... } ``` src/tests/upgrade_tests.cpp Lines 489-505 (patched) <https://reviews.apache.org/r/57336/#comment240100> Have you tried using the contains member function to clean this up? ``` // We check that the following is contained within // the result: // { // "frameworks": // [ // { // "roles": ["role"] // } // ] // } ``` But since we can't do the following: ``` JSON::Object expected = { {"frameworks", { {{"roles", {"foo"} }} }} }; ``` It ends up being pretty tedious. src/tests/upgrade_tests.cpp Lines 489-505 (patched) <https://reviews.apache.org/r/57336/#comment240111> Do you want to guard the .as calls with some .is ASSERTs? src/tests/upgrade_tests.cpp Lines 508-509 (patched) <https://reviews.apache.org/r/57336/#comment240110> You might want a Clock::settle here to ensure it's processed should the handling of updateFrameworkMessage become asynchronous in the agent. src/tests/upgrade_tests.cpp Lines 519-541 (patched) <https://reviews.apache.org/r/57336/#comment240113> Ditto here. - Benjamin Mahler On March 6, 2017, 1:19 p.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57336/ > ----------------------------------------------------------- > > (Updated March 6, 2017, 1:19 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > Updated `MultiRoleSchedulerUpgrade` to test framework updates. > > > Diffs > ----- > > src/tests/upgrade_tests.cpp 6cdd6d989df14af5f17b41af0bf631471feae00e > > > Diff: https://reviews.apache.org/r/57336/diff/2/ > > > Testing > ------- > > > Thanks, > > Michael Park > >
