You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gastón Kleiman <ga...@mesosphere.io> on 2017/08/30 01:05:02 UTC
Review Request 61982: Cleaned up DefaultExecutor tests.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61982/
-----------------------------------------------------------
Review request for mesos, Anand Mazumdar and Greg Mann.
Bugs: MESOS-7916
https://issues.apache.org/jira/browse/MESOS-7916
Repository: mesos
Description (updated)
-------
Update the DefaultExecutor tests to use test helpers where possible.
Also make the boilerplate initialization code consistent across tests.
Diffs (updated)
-----
src/tests/default_executor_tests.cpp afe0afabf784fb65eb833beadd3c584722c321e1
Diff: https://reviews.apache.org/r/61982/diff/1/
Testing (updated)
-------
`sudo bin/mesos-tests.sh --gtest_filter="*DefaultExecutor*"` succeeds on GNU/Linux
Thanks,
Gastón Kleiman
Re: Review Request 61982: Cleaned up DefaultExecutor tests.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61982/#review184853
-----------------------------------------------------------
src/tests/default_executor_tests.cpp
Line 178 (original), 166 (patched)
<https://reviews.apache.org/r/61982/#comment261054>
Hmmm similar to my comment below, I wonder if a new overload could also accept the ExecutorID directly. This might require another template parameter for `createExecutorInfo`? Not sure if it's worth it.
src/tests/default_executor_tests.cpp
Lines 168 (patched)
<https://reviews.apache.org/r/61982/#comment261050>
Hmmmm I wonder if we should add another overload for `createExecutorInfo` so that we don't need to stringify the resources here?
src/tests/default_executor_tests.cpp
Line 180 (original), 171 (patched)
<https://reviews.apache.org/r/61982/#comment261057>
Would it also make sense to add a FrameworkID param to the helper to avoid this line?
- Greg Mann
On Aug. 30, 2017, 1:05 a.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61982/
> -----------------------------------------------------------
>
> (Updated Aug. 30, 2017, 1:05 a.m.)
>
>
> Review request for mesos, Anand Mazumdar and Greg Mann.
>
>
> Bugs: MESOS-7916
> https://issues.apache.org/jira/browse/MESOS-7916
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Update the DefaultExecutor tests to use test helpers where possible.
> Also make the boilerplate initialization code consistent across tests.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 186b8333c02ba3b9257e19437c6d689761085362
>
>
> Diff: https://reviews.apache.org/r/61982/diff/3/
>
>
> Testing
> -------
>
> `sudo bin/mesos-tests.sh --gtest_filter="*DefaultExecutor*"` succeeds on GNU/Linux
>
>
> Thanks,
>
> Gastón Kleiman
>
>
Re: Review Request 61982: Cleaned up DefaultExecutor tests.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61982/#review184392
-----------------------------------------------------------
Patch looks great!
Reviews applied: [61920, 61921, 61982]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Aug. 30, 2017, 3:05 a.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61982/
> -----------------------------------------------------------
>
> (Updated Aug. 30, 2017, 3:05 a.m.)
>
>
> Review request for mesos, Anand Mazumdar and Greg Mann.
>
>
> Bugs: MESOS-7916
> https://issues.apache.org/jira/browse/MESOS-7916
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Update the DefaultExecutor tests to use test helpers where possible.
> Also make the boilerplate initialization code consistent across tests.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 186b8333c02ba3b9257e19437c6d689761085362
>
>
> Diff: https://reviews.apache.org/r/61982/diff/2/
>
>
> Testing
> -------
>
> `sudo bin/mesos-tests.sh --gtest_filter="*DefaultExecutor*"` succeeds on GNU/Linux
>
>
> Thanks,
>
> Gastón Kleiman
>
>
Re: Review Request 61982: Cleaned up DefaultExecutor tests.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61982/#review184117
-----------------------------------------------------------
Failed to apply patch!
Reviews applied: [61920, 61921, 61982]
Logs available here: http://104.210.40.105/logs/master/61982
- Mesos Reviewbot Windows
On Aug. 30, 2017, 1:05 a.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61982/
> -----------------------------------------------------------
>
> (Updated Aug. 30, 2017, 1:05 a.m.)
>
>
> Review request for mesos, Anand Mazumdar and Greg Mann.
>
>
> Bugs: MESOS-7916
> https://issues.apache.org/jira/browse/MESOS-7916
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Update the DefaultExecutor tests to use test helpers where possible.
> Also make the boilerplate initialization code consistent across tests.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp afe0afabf784fb65eb833beadd3c584722c321e1
>
>
> Diff: https://reviews.apache.org/r/61982/diff/1/
>
>
> Testing
> -------
>
> `sudo bin/mesos-tests.sh --gtest_filter="*DefaultExecutor*"` succeeds on GNU/Linux
>
>
> Thanks,
>
> Gastón Kleiman
>
>
Re: Review Request 61982: Cleaned up DefaultExecutor tests.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61982/#review185221
-----------------------------------------------------------
Ship it!
- Greg Mann
On Sept. 9, 2017, 12:03 a.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61982/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2017, 12:03 a.m.)
>
>
> Review request for mesos, Anand Mazumdar and Greg Mann.
>
>
> Bugs: MESOS-7916
> https://issues.apache.org/jira/browse/MESOS-7916
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Update the DefaultExecutor tests to use test helpers where possible.
> Also make the boilerplate initialization code consistent across tests.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 186b8333c02ba3b9257e19437c6d689761085362
>
>
> Diff: https://reviews.apache.org/r/61982/diff/4/
>
>
> Testing
> -------
>
> `sudo bin/mesos-tests.sh --gtest_filter="*DefaultExecutor*"` succeeds on GNU/Linux
>
>
> Thanks,
>
> Gastón Kleiman
>
>
Re: Review Request 61982: Cleaned up DefaultExecutor tests.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61982/
-----------------------------------------------------------
(Updated Sept. 9, 2017, 12:03 a.m.)
Review request for mesos, Anand Mazumdar and Greg Mann.
Changes
-------
Used the new `createExecutorInfo` overloads.
Bugs: MESOS-7916
https://issues.apache.org/jira/browse/MESOS-7916
Repository: mesos
Description
-------
Update the DefaultExecutor tests to use test helpers where possible.
Also make the boilerplate initialization code consistent across tests.
Diffs (updated)
-----
src/tests/default_executor_tests.cpp 186b8333c02ba3b9257e19437c6d689761085362
Diff: https://reviews.apache.org/r/61982/diff/4/
Changes: https://reviews.apache.org/r/61982/diff/3-4/
Testing
-------
`sudo bin/mesos-tests.sh --gtest_filter="*DefaultExecutor*"` succeeds on GNU/Linux
Thanks,
Gastón Kleiman