> On June 5, 2018, 1:30 a.m., Jie Yu wrote: > > src/tests/containerizer/nested_mesos_containerizer_tests.cpp > > Lines 583-585 (original), 584-586 (patched) > > <https://reviews.apache.org/r/67398/diff/2/?file=2034482#file2034482line608> > > > > I think there's a race here. If the process is spawned, but the > > initailize is not called yet, the command might return a failure? > > > > I think a better way is to have a helper in the Waiter so that we can > > wait until the route has been setup.
I added an explicitly promise for users to detect whether the process was initialized. > On June 5, 2018, 1:30 a.m., Jie Yu wrote: > > src/tests/containerizer/nested_mesos_containerizer_tests.cpp > > Line 1435 (original), 1426-1427 (patched) > > <https://reviews.apache.org/r/67398/diff/2/?file=2034482#file2034482line1472> > > > > Hum, I think if the route is not setup, the wget will get a 404? > > > > My suggestion is to make this more explicit. For instance, have a > > rourte called `/trigger` and a Promise called `triggered` (for client to > > trigger an event that test is waiting), and have another route called > > `/wait` and have a Promise called `notify` (for test to notify the client). This is not needed anymore since we use named pipes in this test now. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67398/#review204288 ----------------------------------------------------------- On June 7, 2018, 2:40 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67398/ > ----------------------------------------------------------- > > (Updated June 7, 2018, 2:40 p.m.) > > > Review request for mesos, Gilbert Song and Jie Yu. > > > Bugs: MESOS-8917 > https://issues.apache.org/jira/browse/MESOS-8917 > > > Repository: mesos > > > Description > ------- > > Some tests of nested container functionality used pipes passed to > launched tasks to detect whether a task has actually started executing > the workload (`TASK_RUNNING` updates might be sent before the task > workload is actually started). > > Once we avoid leaking unspecified file descriptors into forked > processes, this test setup will be broken. In this patch we replace > the use of pipes for synchronization with HTTP requests to an actor > running in the tests, or wait on other observable side effects. > > > Diffs > ----- > > src/tests/check_tests.cpp d48febfca220a9633b9884963bcf5a205db7f5e5 > src/tests/containerizer/nested_mesos_containerizer_tests.cpp > 6050e6ebed87249382d56aedb6d98d3cf9812bb9 > > > Diff: https://reviews.apache.org/r/67398/diff/3/ > > > Testing > ------- > > `sudo make check` > > Tested in internal CI on a number of platforms. > > > Thanks, > > Benjamin Bannier > >
