Re: Review Request 56213: Added check tests for command executor.

2017-03-23 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/#review169924 --- Ship it! Ship It! - Andrew Schwartzmeyer On March 23, 2017,

Re: Review Request 56213: Added check tests for command executor.

2017-03-20 Thread Alexander Rukletsov
> On March 14, 2017, 6:24 p.m., Andrew Schwartzmeyer wrote: > > src/tests/check_tests.cpp > > Lines 74 (patched) > > > > > > As the resident PowerShell "expert" (heh!), I tested this (and the > > second one) and

Re: Review Request 56213: Added check tests for command executor.

2017-03-20 Thread Alexander Rukletsov
> On Feb. 15, 2017, 1:41 p.m., Gastón Kleiman wrote: > > src/tests/check_tests.cpp > > Lines 620-625 (patched) > > > > > > This has the potential of being flaky on a very busy box. > > > > A more robust

Re: Review Request 56213: Added check tests for command executor.

2017-03-20 Thread Alexander Rukletsov
> On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Lines 62-63 (patched) > > > > > > i don't see tests for these in this review. if they are added in the > > latter part of the chain,

Re: Review Request 56213: Added check tests for command executor.

2017-03-20 Thread Alexander Rukletsov
> On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Lines 73 (patched) > > > > > > Any particular reason for using v1 scheduler API? The general > > recommendation has been to use v0

Re: Review Request 56213: Added check tests for command executor.

2017-03-20 Thread Alexander Rukletsov
> On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Lines 177 (patched) > > > > > > s/agent/slave/ > > > > here and everywhere else. > > > > we are not doing this

Re: Review Request 56213: Added check tests for command executor.

2017-03-17 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/#review169277 --- Ship it! Assuming existing issues are resolved and the file is

Re: Review Request 56213: Added check tests for command executor.

2017-03-16 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/ --- (Updated March 16, 2017, 4:46 p.m.) Review request for mesos, Gastón Kleiman

Re: Review Request 56213: Added check tests for command executor.

2017-03-15 Thread Vinod Kone
> On March 14, 2017, 6:24 p.m., Andrew Schwartzmeyer wrote: > > src/tests/check_tests.cpp > > Lines 74 (patched) > > > > > > As the resident PowerShell "expert" (heh!), I tested this (and the > > second one) and

Re: Review Request 56213: Added check tests for command executor.

2017-03-14 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/#review168922 --- src/tests/check_tests.cpp Lines 74 (patched)

Re: Review Request 56213: Added check tests for command executor.

2017-03-14 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/ --- (Updated March 14, 2017, 2:09 p.m.) Review request for mesos, Gastón Kleiman

Re: Review Request 56213: Added check tests for command executor.

2017-03-07 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/ --- (Updated March 7, 2017, 8:39 p.m.) Review request for mesos, Gastón Kleiman

Re: Review Request 56213: Added check tests for command executor.

2017-03-03 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/#review167860 --- LGTM modulo addressing the existing issues.

Re: Review Request 56213: Added check tests for command executor.

2017-03-02 Thread Vinod Kone
> On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Lines 73 (patched) > > > > > > Any particular reason for using v1 scheduler API? The general > > recommendation has been to use v0

Re: Review Request 56213: Added check tests for command executor.

2017-03-01 Thread Alexander Rukletsov
> On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Lines 73 (patched) > > > > > > Any particular reason for using v1 scheduler API? The general > > recommendation has been to use v0

Re: Review Request 56213: Added check tests for command executor.

2017-03-01 Thread Alexander Rukletsov
> On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Lines 138 (patched) > > > > > > no need for this. For implicit reconciliation, `tasks` are empty, hence we need to ensure the

Re: Review Request 56213: Added check tests for command executor.

2017-03-01 Thread Alexander Rukletsov
> On Feb. 15, 2017, 1:41 p.m., Gastón Kleiman wrote: > > src/tests/check_tests.cpp > > Lines 632 (patched) > > > > > > I'd add: > > > > ``` > > EXPECT_FALSE(updateTaskRunning->status().has_healthy()); >

Re: Review Request 56213: Added check tests for command executor.

2017-03-01 Thread Alexander Rukletsov
> On Feb. 15, 2017, 5:07 p.m., Gastón Kleiman wrote: > > src/tests/check_tests.cpp > > Lines 269 (patched) > > > > > > We should add: > > > > ``` > >

Re: Review Request 56213: Added check tests for command executor.

2017-02-28 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/ --- (Updated Feb. 28, 2017, 3:55 p.m.) Review request for mesos, Gastón Kleiman

Re: Review Request 56213: Added check tests for command executor.

2017-02-16 Thread Gastón Kleiman
> On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp, line 177 > > > > > > s/agent/slave/ > > > > here and everywhere else. > > > > we are not doing this change yet. > >

Re: Review Request 56213: Added check tests for command executor.

2017-02-15 Thread Vinod Kone
> On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp, line 177 > > > > > > s/agent/slave/ > > > > here and everywhere else. > > > > we are not doing this change yet. > >

Re: Review Request 56213: Added check tests for command executor.

2017-02-15 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/#review165724 --- src/tests/check_tests.cpp (line 269)

Re: Review Request 56213: Added check tests for command executor.

2017-02-15 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/#review165568 --- src/tests/check_tests.cpp (line 202)

Re: Review Request 56213: Added check tests for command executor.

2017-02-14 Thread Gastón Kleiman
> On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp, line 177 > > > > > > s/agent/slave/ > > > > here and everywhere else. > > > > we are not doing this change yet. We

Re: Review Request 56213: Added check tests for command executor.

2017-02-09 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/#review165049 --- src/tests/check_tests.cpp (lines 62 - 63)

Re: Review Request 56213: Added check tests for command executor.

2017-02-08 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/ --- (Updated Feb. 8, 2017, 4:56 p.m.) Review request for mesos, Gastón Kleiman and

Re: Review Request 56213: Added check tests for command executor.

2017-02-08 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/ --- (Updated Feb. 8, 2017, 3:18 p.m.) Review request for mesos, Gastón Kleiman and

Review Request 56213: Added check tests for command executor.

2017-02-02 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/ --- Review request for mesos, Gastón Kleiman and Vinod Kone. Bugs: MESOS-6906