> On March 26, 2018, 2:27 p.m., Jan Schlicht wrote: > > src/tests/storage_local_resource_provider_tests.cpp > > Lines 3127 (patched) > > <https://reviews.apache.org/r/65666/diff/3/?file=1982877#file1982877line3128> > > > > We should wait for an `UpdateSlaveMessage` here. Otherwise we can't be > > sure that an offer will contain resource provider resources. > > I see that you're using the `OffersHaveAnyResources` matcher to work > > around this. The test could be much stricter when using > > `UpdateSlaveMessage` instead as we wouldn't expect offers without RP > > resources after receiving this message, probably even run the test with a > > stopped clock.
Yeah that's true. However if we wait for the `UpdateSlaveMessage` or even stopping the clock, the we'll need to have logic like the following: https://github.com/apache/mesos/blob/master/src/tests/storage_local_resource_provider_tests.cpp#L296-L327 which reveals implementation details about SLRP sending multiple `UpdateSlaveMessage`s and their orders. Since these details are not very relevent to the test, I'd prefer using the current style which mimics how a framework would use SLRP, unless it is necessary to control the clock. - Chun-Hung ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65666/#review199669 ----------------------------------------------------------- On March 20, 2018, 3:27 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65666/ > ----------------------------------------------------------- > > (Updated March 20, 2018, 3:27 a.m.) > > > Review request for mesos, Benjamin Bannier, Greg Mann, Jie Yu, and Jan > Schlicht. > > > Bugs: MESOS-8383 > https://issues.apache.org/jira/browse/MESOS-8383 > > > Repository: mesos > > > Description > ------- > > This patch adds the `ROOT_OperationStateMetrics` test that issues a > `CREATE_VOLUME` followed by two `DESTROY_VOLUME`s. The first one will > fail due to an out-of-band deletion of the actual volume, and the second > one will fail due to modifying the resource version. > > > Diffs > ----- > > src/tests/storage_local_resource_provider_tests.cpp > 264d42b57fe1a8ea04c1de0a10112878c7b45d39 > > > Diff: https://reviews.apache.org/r/65666/diff/3/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Chun-Hung Hsiao > >
