----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70621/#review215248 -----------------------------------------------------------
src/examples/test_csi_plugin.cpp Lines 215 (patched) <https://reviews.apache.org/r/70621/#comment301878> Let's add some additional context to this assertion. src/examples/test_csi_plugin.cpp Line 226 (original), 223 (patched) <https://reviews.apache.org/r/70621/#comment301879> Ditto. src/examples/test_csi_plugin.cpp Lines 228 (patched) <https://reviews.apache.org/r/70621/#comment301880> Ditto. src/examples/test_csi_plugin.cpp Line 1253 (original), 1244 (patched) <https://reviews.apache.org/r/70621/#comment301881> It would be great to have some testing that `getVolumePath -> parseVolumePath` roundtrips. Let's either add a simple dedicated test or, since this is test code, just add an assertion in e.g., `getVolumePath` that the created path would roundtrip. src/examples/test_csi_plugin.cpp Lines 1250 (patched) <https://reviews.apache.org/r/70621/#comment301882> Can we do that "normalization" when we initialize `workDir` instead? src/examples/test_csi_plugin.cpp Lines 1257-1261 (original), 1259-1264 (patched) <https://reviews.apache.org/r/70621/#comment301883> Not added here, but I feel `strings::tokenize` would have made this more readible, e.g., ```` vector<string> components = strings::tokenize(basename, "-", 2); if (components.size() != 2) { return Error("Cannot find delimiter '-' in " + basename); // FYI: Made this error more explicit. } Try<Bytes> bytes = Bytes::parse(components[1]); ```` src/examples/test_csi_plugin.cpp Line 1269 (original), 1269 (patched) <https://reviews.apache.org/r/70621/#comment301884> We are leaving `id` default-initialized here. - Benjamin Bannier On May 10, 2019, 3:17 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70621/ > ----------------------------------------------------------- > > (Updated May 10, 2019, 3:17 a.m.) > > > Review request for mesos and Benjamin Bannier. > > > Bugs: MESOS-9395 > https://issues.apache.org/jira/browse/MESOS-9395 > > > Repository: mesos > > > Description > ------- > > The full paths of simulated volumes are now in their ID instead of their > contextual information. This simplifies SLRP tests, and makes it cleaner > if we want to customize the contextual information in the future. > > > Diffs > ----- > > src/examples/test_csi_plugin.cpp 03f782ead136a79c4c5b099496072933f6737598 > src/tests/storage_local_resource_provider_tests.cpp > ec2222d7aeef1cb3d1a5b4b3419dfd912d41a8c6 > > > Diff: https://reviews.apache.org/r/70621/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Chun-Hung Hsiao > >
