----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65666/#review199669 -----------------------------------------------------------
This is missing a test for `operations_pending`. You could wait with a `DROP_PROTOBUF(UpdateOperationStatusMessage)` after applying an operation, then check that `operations_pending` is "1", then (manually) send the `UpdateOperationStatusMessage` and check the `operation_pending` is back to "0". src/tests/storage_local_resource_provider_tests.cpp Line 59 (original), 59 (patched) <https://reviews.apache.org/r/65666/#comment280476> Remove this line. src/tests/storage_local_resource_provider_tests.cpp Lines 3127 (patched) <https://reviews.apache.org/r/65666/#comment280050> 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. src/tests/storage_local_resource_provider_tests.cpp Lines 3142 (patched) <https://reviews.apache.org/r/65666/#comment280051> s/MONUT/MOUNT src/tests/storage_local_resource_provider_tests.cpp Lines 3194-3195 (patched) <https://reviews.apache.org/r/65666/#comment280052> As this prefix is used in more than one test case, how about making it part of the test fixture? - Jan Schlicht On March 20, 2018, 4: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, 4: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 > >
