----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49520/#review140386 -----------------------------------------------------------
Could you verify on testing done section that it's been run on both macOS and Linux? We missed something last time :) https://github.com/apache/mesos/commit/6619503c0b6f9c83a4af24c5d3556d4ee68d0e65 src/tests/gc_tests.cpp (lines 965 - 968) <https://reviews.apache.org/r/49520/#comment205778> I think we should timebox this wait. ``` // Wait for the task to create these paths. Timeout timeout = Timeout::in(Seconds(15)); while (!os::exists(path::join(sandbox, mountPoint_) || !os::exists(path::join(sandbox, regularFile) || !timeout.expired()) { os::sleep(Milliseconds(10)); } ASSERT_TRUE(os::exists(path::join(sandbox, mountPoint_))); ASSERT_TRUE(os::exists(path::join(sandbox, regularFile))); ``` Because we can exist the loop due to timeout, we need to ASSERT here. src/tests/gc_tests.cpp (lines 971 - 972) <https://reviews.apache.org/r/49520/#comment205781> Use `ASSERT_EQ` here for both. (Sorry I commented on this in rmdir_tests.cpp earlier but overlooked it in gc_tests.cpp) Theses are assertions because we don't want the test to proceed if these aren't met. They are part of the setup. - Jiang Yan Xu On July 1, 2016, 9:39 a.m., Megha Sharma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49520/ > ----------------------------------------------------------- > > (Updated July 1, 2016, 9:39 a.m.) > > > Review request for mesos and Jiang Yan Xu. > > > Bugs: MESOS-5752 > https://issues.apache.org/jira/browse/MESOS-5752 > > > Repository: mesos > > > Description > ------- > > In ROOT_GarbageCollectorUndeletableFilesTest.BusyMountPoint there is > a race between the task creating a file and the test looking for it > which sometimes leads to assertion failure on the existence of the > file. > > > Diffs > ----- > > src/tests/gc_tests.cpp 2e23f04ea908aadcefb21e11203b95b94ec3c60b > > Diff: https://reviews.apache.org/r/49520/diff/ > > > Testing > ------- > > > Thanks, > > Megha Sharma > >
