> On Aug. 25, 2016, 3:11 p.m., Jiang Yan Xu wrote: > > src/tests/mesos.hpp, line 754 > > <https://reviews.apache.org/r/45964/diff/12/?file=1463605#file1463605line754> > > > > The original comment was intended for the following three methods. Do > > you think we need to comment on each one? If so at least do it for LAUNCH > > as well and change plurals to singulars.
There are separate comments that exist for `RESERVE` and `UNRESERVE`, so let me add them for all the offer operations. > On Aug. 25, 2016, 3:11 p.m., Jiang Yan Xu wrote: > > src/tests/resources_tests.cpp, line 2871 > > <https://reviews.apache.org/r/45964/diff/12/?file=1463607#file1463607line2871> > > > > This test came up before IIRC. I don't think we need this as the > > existing tests already have "multiple consumers". This test adds the test for the `shared()` API. I moved that to `TEST(SharedResourcesTest, ScalarAdditionShared)`, and also added check for `nonShared()`. > On Aug. 25, 2016, 3:11 p.m., Jiang Yan Xu wrote: > > src/tests/persistent_volume_tests.cpp, line 547 > > <https://reviews.apache.org/r/45964/diff/12/?file=1463606#file1463606line547> > > > > It's unfortunate that we duplicate all the code from > > `PreparePersistentVolume` with only two lines of change. > > > > This applies to `AccessSharedPersistentVolume` as well. > > > > If we want to test whether shared volume works on the slave, we should > > create a test for the more interesting case: multiple tasks accessing the > > same persistent volume. This test would be both necessary and also would > > cover both "preparing" and "accessing" the shared persistent volume > > therefore we don't need to test them separately. Removed `PreparePersistentVolume `. Modified `AccessSharedPersistentVolume` with testing 2 tasks being able to write to the same shared persistent volume simultaneously. > On Aug. 25, 2016, 3:11 p.m., Jiang Yan Xu wrote: > > src/tests/resources_tests.cpp, line 2390 > > <https://reviews.apache.org/r/45964/diff/12/?file=1463607#file1463607line2390> > > > > Also verify `nonShared()`? > > > > Plus, there is no operation (therefore shouldn't be in > > ResourcesOperationTest)? Moved this test to `SharedResourcesTest.Filter`. Added check for `nonShared()` in addition to `shared()`. - Anindya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45964/#review146219 ----------------------------------------------------------- On Aug. 29, 2016, 7:22 a.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45964/ > ----------------------------------------------------------- > > (Updated Aug. 29, 2016, 7:22 a.m.) > > > Review request for mesos and Jiang Yan Xu. > > > Bugs: MESOS-4324 > https://issues.apache.org/jira/browse/MESOS-4324 > > > Repository: mesos > > > Description > ------- > > Add unit tests for sharing of resources. > > > Diffs > ----- > > src/tests/hierarchical_allocator_tests.cpp > ddd48694e5c2d3d01fbff20558a46c1256b3dbc3 > src/tests/master_validation_tests.cpp > e440ff3f6d04f797e65874b9f610ed63d9f28e0e > src/tests/mesos.hpp 4cae54b4df906d4b7e8fe8d40d5b0ad59d260e6f > src/tests/persistent_volume_tests.cpp > a6f97c4bb5fb29d610c01255036095e2b30c44c5 > src/tests/resources_tests.cpp 4932d95f4e78cae764b89472373e13527b4354a2 > > Diff: https://reviews.apache.org/r/45964/diff/ > > > Testing > ------- > > Tests for shared resources added for allocator, resources and sorter. > Tests successful. > > > Thanks, > > Anindya Sinha > >
