> On Aug. 21, 2019, 2:28 p.m., Greg Mann wrote: > > src/tests/master_draining_tests.cpp > > Lines 178 (patched) > > <https://reviews.apache.org/r/71316/diff/1/?file=2161830#file2161830line178> > > > > In this case, we probably want to either not set the max grace period, > > or set it to something long, since we're trying to verify that an agent > > with nothing running will immediately be marked drained. While the agent > > won't be _immediately_ marked gone if a task is running and the max grace > > period is set to zero, it would still happen very quickly.
The test techically isn't checking for this grace period. But it doesn't hurt to have a non-zero value here. Increased to 10 seconds. > On Aug. 21, 2019, 2:28 p.m., Greg Mann wrote: > > src/tests/master_draining_tests.cpp > > Lines 222-250 (patched) > > <https://reviews.apache.org/r/71316/diff/1/?file=2161830#file2161830line222> > > > > We probably don't need to verify the state output of all 3 endpoints in > > both versions of this test, I think just one will do. WDYT? That makes sense. I kept the GET_AGENTS one. > On Aug. 21, 2019, 2:28 p.m., Greg Mann wrote: > > src/tests/master_draining_tests.cpp > > Lines 313-325 (patched) > > <https://reviews.apache.org/r/71316/diff/1/?file=2161830#file2161830line313> > > > > Can we do something to verify that the agent has been marked > > unreachable here? Either specifying the type of the registry operation in > > the EXPECT_CALL, or inspecting some API output afterward? I changed the mock to return the operation, which can be type-casted to the appropriate operation subclass (MarkSlaveUnreachable). - Joseph ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71316/#review217362 ----------------------------------------------------------- On Aug. 22, 2019, 11:34 a.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71316/ > ----------------------------------------------------------- > > (Updated Aug. 22, 2019, 11:34 a.m.) > > > Review request for mesos, Benno Evers and Greg Mann. > > > Bugs: MESOS-9892 > https://issues.apache.org/jira/browse/MESOS-9892 > > > Repository: mesos > > > Description > ------- > > This splits the existing agent draining tests into two variants: > 1) where the agent has nothing running, and > 2) where the agent has one task running. > > > Diffs > ----- > > src/tests/master_draining_tests.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/71316/diff/2/ > > > Testing > ------- > > make check > > > Thanks, > > Joseph Wu > >
