> On April 25, 2017, 5:59 p.m., Benjamin Mahler wrote: > > src/tests/upgrade_tests.cpp > > Lines 537-539 (patched) > > <https://reviews.apache.org/r/58689/diff/1/?file=1698681#file1698681line537> > > > > This comment seems to be inconsistent with what the test is doing? > > > > Also, this doesn't look like an upgrade test, should we make it a > > master test?
Updated the comment to more accurately reflect the test. I was considering this to be a kind of an upgrade test, in the sense that we're upgrading the framework from one config to another. I'm not sure if we have some clear criteria for what qualifies as an upgrade test. But I'm also fine with this living in master test. > On April 25, 2017, 5:59 p.m., Benjamin Mahler wrote: > > src/tests/upgrade_tests.cpp > > Lines 612 (patched) > > <https://reviews.apache.org/r/58689/diff/1/?file=1698681#file1698681line612> > > > > Hm.. not obvious to me why this was needed, can you add a comment? I don't think this is necessary. Removed. > On April 25, 2017, 5:59 p.m., Benjamin Mahler wrote: > > src/tests/upgrade_tests.cpp > > Lines 657 (patched) > > <https://reviews.apache.org/r/58689/diff/1/?file=1698681#file1698681line657> > > > > Do you want to just pause the clock for the whole test to avoid relying > > on timers? Done! > On April 25, 2017, 5:59 p.m., Benjamin Mahler wrote: > > src/tests/upgrade_tests.cpp > > Lines 682-683 (patched) > > <https://reviews.apache.org/r/58689/diff/1/?file=1698681#file1698681line682> > > > > ... and the master will track this role under the framework, but the > > framework should not receive offers for this role? > > > > Should we test for that? Added queries to the master to test that the framework is re-tracked. - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58689/#review172968 ----------------------------------------------------------- On April 26, 2017, 3:47 p.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58689/ > ----------------------------------------------------------- > > (Updated April 26, 2017, 3:47 p.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway. > > > Bugs: MESOS-7323 > https://issues.apache.org/jira/browse/MESOS-7323 > > > Repository: mesos > > > Description > ------- > > MESOS-7323: Made `addSlave` conditionally activate the framework. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.cpp > 984a0a4e2671ee7bb2a3515849342f49f2c4e3aa > src/tests/upgrade_tests.cpp b5a28b5161d896dff250b8ad012c8aac91c3f861 > > > Diff: https://reviews.apache.org/r/58689/diff/2/ > > > Testing > ------- > > `make check` and added a test. > > > Thanks, > > Michael Park > >
