----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35728/#review89458 -----------------------------------------------------------
Ship it! Nice work! So, running the command as root first will guarantee that lt-mesos-executor exists before trying to run the task as the test-user `nobody`? We might be able to fix this in a cleaner way, but this looks good enough to me. Just fix the few minor points I raised and update the comments, and then I'll commit it. Please fill out the "Testing Done" section to explain how you verified that this test failed before your change and now passes. src/tests/slave_tests.cpp (line 671) <https://reviews.apache.org/r/35728/#comment142097> Not yours, but please fix: s/precense/presense/ src/tests/slave_tests.cpp (line 708) <https://reviews.apache.org/r/35728/#comment142088> You shouldn't start the driver until after you've set up your expectation for receiving offers, otherwise the scheduler might receive offers before your expectation is in place. src/tests/slave_tests.cpp (line 715) <https://reviews.apache.org/r/35728/#comment142087> You should be able to use the same `offers` variable here as down below. src/tests/slave_tests.cpp (lines 723 - 727) <https://reviews.apache.org/r/35728/#comment142105> Please clarify this comment. You're running `active-user-test-helper` first as root (current os::user, since this is a `ROOT_` test), so that root can create `lt-mesos-executor` before we try to run `active-user-test-helper` as the test_user (`nobody`), correct? src/tests/slave_tests.cpp (lines 731 - 732) <https://reviews.apache.org/r/35728/#comment142090> s/MergeFrom/CopyFrom/ (wherever possible, since there's actually nothing to merge it with here) src/tests/slave_tests.cpp (lines 735 - 737) <https://reviews.apache.org/r/35728/#comment142106> Also, `EXPECT_EQ("root", user)` src/tests/slave_tests.cpp (line 747) <https://reviews.apache.org/r/35728/#comment142093> s/MergeFrom/CopyFrom/ - Adam B On June 22, 2015, 4:34 a.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35728/ > ----------------------------------------------------------- > > (Updated June 22, 2015, 4:34 a.m.) > > > Review request for mesos and Adam B. > > > Bugs: MESOS-2199 > https://issues.apache.org/jira/browse/MESOS-2199 > > > Repository: mesos > > > Description > ------- > > Fix failing test: SlaveTest.ROOT_RunTaskWithCommandInfoWithUser. > > > Diffs > ----- > > src/tests/slave_tests.cpp 50301983c674ef50a64294816db9587bf065aa9e > > Diff: https://reviews.apache.org/r/35728/diff/ > > > Testing > ------- > > > Thanks, > > haosdent huang > >
