> On Oct. 23, 2014, 5:48 p.m., Jie Yu wrote:
> > src/tests/slave_tests.cpp, lines 431-436
> > <https://reviews.apache.org/r/26622/diff/2/?file=719278#file719278line431>
> >
> > It would be great if you can write an end-to-end test so that we can
> > verify that the 'command' actually runs correctly with shell=false and has
> > an argument list (refer to the following test).
>
> R.B. Boyer wrote:
> I would love to try writing an e2e test to test this, but I spent several
> hours on it before caving and just writing this GetExecutorInfo test. I have
> about zero experience writing C++ code let alone C++ unit tests so I was
> fighting with the project more than fighting with the bug.
>
> Timothy Chen wrote:
> You can pretty much copy the MesosExecutorWithOverride test in
> slave_tests.cpp, and just modify the CommandInfo accordingly. Let us know if
> you still don't know what to do and spend too much time figuring out.
>
> R.B. Boyer wrote:
> I think what I had trouble with there when last I was working on that was
> actually asserting that mesos-executor died (exit != 0) when passed
> double-dash arguments. I could get the test to pass, but it also passed
> without the fix.
So this is the part of that unit test I have no idea how to adapt:
// Expect the launch and just assume it was sucessful since we'll be
// launching the executor ourselves manually below.
Future<Nothing> launch;
EXPECT_CALL(containerizer, launch(_, _, _, _, _, _, _))
.WillOnce(DoAll(FutureSatisfy(&launch),
Return(true)));
...and more ...
In my case I need to strongly assert that the command launched without failing
(which requires a mesos-executor binary to actually run) OR inject a Capture of
some sort here and trap the exec to assert it is sane without finishing the e2e
part of this.
- R.B.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26622/#review58149
-----------------------------------------------------------
On Oct. 17, 2014, 11:40 p.m., R.B. Boyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26622/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2014, 11:40 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy Chen.
>
>
> Bugs: MESOS-1873
> https://issues.apache.org/jira/browse/MESOS-1873
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Basically if you use "shell=false" with a non-empty argument list and the
> Command Executor it is completely broken.
>
> When we clone the env vars to fork "mesos-executor" all of the original cmd
> args are cloned as well (unintentionally) due to some protocol-buffer merge
> shenanigans.
>
> Don't pass task-related arguments to mesos-executor.
>
> The description on the linked jira ticket goes into more detail.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp 0e342ed
> src/tests/slave_tests.cpp f585bdd
>
> Diff: https://reviews.apache.org/r/26622/diff/
>
>
> Testing
> -------
>
> make check
>
> added new test "SlaveTest.GetExecutorInfo" verifying the explicit desired
> behavior.
>
>
> Thanks,
>
> R.B. Boyer
>
>
