> On Sept. 21, 2018, 10:05 a.m., Benjamin Bannier wrote: > > src/tests/agent_resource_provider_config_api_tests.cpp > > Line 35 (original), 35 (patched) > > <https://reviews.apache.org/r/68762/diff/2/?file=2090657#file2090657line35> > > > > Like you wrote in your _Testing Done_ section, this test is broken > > without the next patch https://reviews.apache.org/r/68763/. Let's only > > commit it in the same patch as the feature it is testing, or after it; > > otherwise we might break e.g., `git bisect` workflows without benefit.
Hmm I thought it would be fine as long as we commit them together. I'll adjust the commit order. > On Sept. 21, 2018, 10:05 a.m., Benjamin Bannier wrote: > > src/tests/agent_resource_provider_config_api_tests.cpp > > Lines 932 (patched) > > <https://reviews.apache.org/r/68762/diff/2/?file=2090657#file2090657line932> > > > > Nit: We could also write this as `pluginContainer[0]`. I don't think so because `pluginContainers` is a `Future<hashset<...>>` ;) - Chun-Hung ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68762/#review208754 ----------------------------------------------------------- On Sept. 20, 2018, 11:15 p.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68762/ > ----------------------------------------------------------- > > (Updated Sept. 20, 2018, 11:15 p.m.) > > > Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht. > > > Bugs: MESOS-9228 > https://issues.apache.org/jira/browse/MESOS-9228 > > > Repository: mesos > > > Description > ------- > > Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`. > > > Diffs > ----- > > src/tests/agent_resource_provider_config_api_tests.cpp > e6a68bae1a9e3e773ea45deae4951664ab81a857 > > > Diff: https://reviews.apache.org/r/68762/diff/2/ > > > Testing > ------- > > make check > > The test will fail without the next patch. > > NOTE: It might be more appropriate to not reuse this test for container > cleanup. Instead, I'm considering making a similar test in > `StorageLocalResourceProviderTest`. > > > Thanks, > > Chun-Hung Hsiao > >
