You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Quinn Leng <qu...@gmail.com> on 2017/07/13 21:11:02 UTC
Re: Review Request 60847: Added test cases for /slaves, /containers,
/frameworks endpoints.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60847/
-----------------------------------------------------------
(Updated July 13, 2017, 9:11 p.m.)
Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
Summary (updated)
-----------------
Added test cases for /slaves, /containers, /frameworks endpoints.
Bugs: MESOS-7630
https://issues.apache.org/jira/browse/MESOS-7630
Repository: mesos
Description (updated)
-------
Added query parameter test cases for '/slaves' and '/frameworks' on
the master, and '/containers' endpoint on the agent.
Diffs
-----
src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
src/tests/slave_tests.cpp 035db18db3a64a9e358c1c54cc18a4bdeb85d8bf
Diff: https://reviews.apache.org/r/60847/diff/1/
Testing
-------
Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 --gtest_break_on_failure'
Thanks,
Quinn Leng
Re: Review Request 60847: Add test cases for /slaves,
/slave/containers, /frameworks endpoints.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60847/#review180762
-----------------------------------------------------------
src/tests/master_tests.cpp
Lines 2593 (patched)
<https://reviews.apache.org/r/60847/#comment255983>
s/slave_id/slaveId/
here and elsewhere
src/tests/master_tests.cpp
Line 5856 (original), 5979 (patched)
<https://reviews.apache.org/r/60847/#comment255984>
s/termintated/terminated/
src/tests/master_tests.cpp
Lines 6047-6048 (patched)
<https://reviews.apache.org/r/60847/#comment256019>
Let's leave a comment here explaining why we need two different expected values.
src/tests/master_tests.cpp
Lines 6048-6076 (patched)
<https://reviews.apache.org/r/60847/#comment256024>
I think that we can find a more elegant way to handle scenarios like this. Consider the following:
```
Try<JSON::Value> frameworkJson1 = JSON::parse(
"{"
"\"id\":\"" + frameworkId1->value() + "\","
"\"name\":\"default\""
"}");
Try<JSON::Value> frameworkJson2 = JSON::parse(
"{"
"\"id\":\"" + frameworkId2->value() + "\","
"\"name\":\"default\""
"}");
ASSERT_SOME(frameworkJson1);
ASSERT_SOME(frameworkJson2);
// Since frameworks are stored in a hashmap, there is no strict guarantee of
// their ordering when listed. For this reason, we test both possibilities.
if (array->values[0].contains(frameworkJson1.get())) {
ASSERT_TRUE(array->values[1].contains(frameworkJson2.get()));
} else {
ASSERT_TRUE(array->values[0].contains(frameworkJson2.get()));
ASSERT_TRUE(array->values[1].contains(frameworkJson1.get()));
}
```
What do you think? We could do something like this here and elsewhere.
src/tests/master_tests.cpp
Lines 6075-6076 (patched)
<https://reviews.apache.org/r/60847/#comment255985>
```
EXPECT_TRUE(
value->contains(expected1.get()) ||
value->contains(expected2.get()));
```
src/tests/master_tests.cpp
Lines 6114 (patched)
<https://reviews.apache.org/r/60847/#comment255986>
s/master has known that the framework is terminated/master has marked the framework as completed/
src/tests/master_tests.cpp
Lines 6120-6121 (patched)
<https://reviews.apache.org/r/60847/#comment256025>
How about:
"Complete the first framework. As a result, it will appear in the response's 'completed_frameworks' field."
src/tests/master_tests.cpp
Lines 6125 (patched)
<https://reviews.apache.org/r/60847/#comment256026>
You can remove this comment.
src/tests/master_tests.cpp
Lines 6128 (patched)
<https://reviews.apache.org/r/60847/#comment256027>
You can remove this comment.
src/tests/slave_tests.cpp
Lines 2403-2404 (patched)
<https://reviews.apache.org/r/60847/#comment256032>
Newline here.
src/tests/slave_tests.cpp
Line 2419 (original), 2459 (patched)
<https://reviews.apache.org/r/60847/#comment256033>
s/status/statistics/
src/tests/slave_tests.cpp
Lines 2470-2473 (patched)
<https://reviews.apache.org/r/60847/#comment256035>
I think you mean to say that this is called twice in the first query.
Also, perhaps this comment could mention that we extract the container ID for use later on. How about:
"Will be called twice during the first request. We extract the assigned container IDs for use when requesting information on a single container."
src/tests/slave_tests.cpp
Line 2421 (original), 2480-2481 (patched)
<https://reviews.apache.org/r/60847/#comment256034>
Newline here.
src/tests/slave_tests.cpp
Lines 2514 (patched)
<https://reviews.apache.org/r/60847/#comment256036>
Both of these `WillOnce` calls are for the first query, right?
How about:
"Will be called twice during the first request."
src/tests/slave_tests.cpp
Lines 2519-2520 (patched)
<https://reviews.apache.org/r/60847/#comment256037>
How about:
"Request information about all containers."
- Greg Mann
On July 14, 2017, 1:43 a.m., Quinn Leng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60847/
> -----------------------------------------------------------
>
> (Updated July 14, 2017, 1:43 a.m.)
>
>
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added query parameter test cases for '/slaves' and '/frameworks' on
> the master, and '/slave/containers' endpoint on the slave.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
> src/tests/slave_tests.cpp 035db18db3a64a9e358c1c54cc18a4bdeb85d8bf
>
>
> Diff: https://reviews.apache.org/r/60847/diff/2/
>
>
> Testing
> -------
>
> Passed 'make check -j48'
> Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
> Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 --gtest_break_on_failure'
>
>
> Thanks,
>
> Quinn Leng
>
>
Re: Review Request 60847: Added test cases for /slaves,
/slave/containers, /frameworks endpoints.
Posted by Quinn Leng <qu...@gmail.com>.
> On July 18, 2017, 8:23 p.m., Greg Mann wrote:
> > src/tests/master_tests.cpp
> > Lines 2523-2524 (patched)
> > <https://reviews.apache.org/r/60847/diff/3/?file=1778679#file1778679line2523>
> >
> > Newline here.
It makes the code look a little strange, one single line of comment surrounded by two empty lines.
> On July 18, 2017, 8:23 p.m., Greg Mann wrote:
> > src/tests/master_tests.cpp
> > Line 5866 (original), 5980-5981 (patched)
> > <https://reviews.apache.org/r/60847/diff/3/?file=1778679#file1778679line6001>
> >
> > Newline after this comment.
It makes the code look a little strange, one single line of comment surrounded by two empty lines.
- Quinn
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60847/#review180852
-----------------------------------------------------------
On July 18, 2017, 6:28 p.m., Quinn Leng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60847/
> -----------------------------------------------------------
>
> (Updated July 18, 2017, 6:28 p.m.)
>
>
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added query parameter test cases for '/slaves' and '/frameworks' on
> the master, and '/slave/containers' endpoint on the slave.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
> src/tests/slave_tests.cpp 035db18db3a64a9e358c1c54cc18a4bdeb85d8bf
>
>
> Diff: https://reviews.apache.org/r/60847/diff/4/
>
>
> Testing
> -------
>
> Passed 'make check -j48'
> Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
> Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 --gtest_break_on_failure'
>
>
> Thanks,
>
> Quinn Leng
>
>
Re: Review Request 60847: Add test cases for /slaves,
/slave/containers, /frameworks endpoints.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60847/#review180852
-----------------------------------------------------------
src/tests/master_tests.cpp
Lines 2513 (patched)
<https://reviews.apache.org/r/60847/#comment256147>
Can remove this comment.
src/tests/master_tests.cpp
Lines 2523-2524 (patched)
<https://reviews.apache.org/r/60847/#comment256146>
Newline here.
src/tests/master_tests.cpp
Lines 2562 (patched)
<https://reviews.apache.org/r/60847/#comment256148>
Can remove this comment.
src/tests/master_tests.cpp
Lines 2587 (patched)
<https://reviews.apache.org/r/60847/#comment256149>
s/recovered_slaves/recovered/
src/tests/master_tests.cpp
Lines 2591 (patched)
<https://reviews.apache.org/r/60847/#comment256150>
Let's use single quotes for consistency:
'recovered_slaves'
src/tests/master_tests.cpp
Lines 3742-3784 (original), 3863-3896 (patched)
<https://reviews.apache.org/r/60847/#comment256151>
Could you break this change out into a separate patch?
src/tests/master_tests.cpp
Line 5866 (original), 5980-5981 (patched)
<https://reviews.apache.org/r/60847/#comment256153>
Newline after this comment.
src/tests/master_tests.cpp
Lines 5985 (patched)
<https://reviews.apache.org/r/60847/#comment256154>
Let's use `driver` instead of `schedDriver` for these variables. This is more consistent with the rest of this file.
src/tests/master_tests.cpp
Lines 6019 (patched)
<https://reviews.apache.org/r/60847/#comment256155>
The word "query" here is ambiguous. How about:
"Request with no query parameter."
src/tests/master_tests.cpp
Lines 6098 (patched)
<https://reviews.apache.org/r/60847/#comment256162>
s/shutdown call/teardown call/
src/tests/slave_tests.cpp
Lines 2429 (patched)
<https://reviews.apache.org/r/60847/#comment256163>
How about something more descriptive for this variable name, maybe `launchedTask1`?
src/tests/slave_tests.cpp
Lines 2453-2475 (original), 2610-2657 (patched)
<https://reviews.apache.org/r/60847/#comment256164>
If we remove the 'executor_id' field from this JSON, can we have just a single `expected` value, and simply do `EXPECT_TRUE(value->contains(expected.get()));`?
- Greg Mann
On July 18, 2017, 6:28 p.m., Quinn Leng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60847/
> -----------------------------------------------------------
>
> (Updated July 18, 2017, 6:28 p.m.)
>
>
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added query parameter test cases for '/slaves' and '/frameworks' on
> the master, and '/slave/containers' endpoint on the slave.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
> src/tests/slave_tests.cpp 035db18db3a64a9e358c1c54cc18a4bdeb85d8bf
>
>
> Diff: https://reviews.apache.org/r/60847/diff/3/
>
>
> Testing
> -------
>
> Passed 'make check -j48'
> Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
> Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 --gtest_break_on_failure'
>
>
> Thanks,
>
> Quinn Leng
>
>
Re: Review Request 60847: Add test cases for /slaves,
/slave/containers, /frameworks endpoints.
Posted by Quinn Leng <qu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60847/
-----------------------------------------------------------
(Updated July 18, 2017, 6:28 p.m.)
Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
Bugs: MESOS-7630
https://issues.apache.org/jira/browse/MESOS-7630
Repository: mesos
Description
-------
Added query parameter test cases for '/slaves' and '/frameworks' on
the master, and '/slave/containers' endpoint on the slave.
Diffs (updated)
-----
src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
src/tests/slave_tests.cpp 035db18db3a64a9e358c1c54cc18a4bdeb85d8bf
Diff: https://reviews.apache.org/r/60847/diff/3/
Changes: https://reviews.apache.org/r/60847/diff/2-3/
Testing
-------
Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 --gtest_break_on_failure'
Thanks,
Quinn Leng
Re: Review Request 60847: Add test cases for /slaves,
/slave/containers, /frameworks endpoints.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60847/#review180620
-----------------------------------------------------------
Patch looks great!
Reviews applied: [60279, 60716, 60820, 60822, 60847]
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 July 14, 2017, 1:43 a.m., Quinn Leng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60847/
> -----------------------------------------------------------
>
> (Updated July 14, 2017, 1:43 a.m.)
>
>
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added query parameter test cases for '/slaves' and '/frameworks' on
> the master, and '/slave/containers' endpoint on the slave.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
> src/tests/slave_tests.cpp 035db18db3a64a9e358c1c54cc18a4bdeb85d8bf
>
>
> Diff: https://reviews.apache.org/r/60847/diff/2/
>
>
> Testing
> -------
>
> Passed 'make check -j48'
> Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
> Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 --gtest_break_on_failure'
>
>
> Thanks,
>
> Quinn Leng
>
>
Re: Review Request 60847: Add test cases for /slaves,
/slave/containers, /frameworks endpoints.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60847/#review180505
-----------------------------------------------------------
Patch looks great!
Reviews applied: [60279, 60716, 60820, 60822, 60847]
Passed command: support\windows-build.bat
- Mesos Reviewbot Windows
On July 14, 2017, 1:43 a.m., Quinn Leng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60847/
> -----------------------------------------------------------
>
> (Updated July 14, 2017, 1:43 a.m.)
>
>
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added query parameter test cases for '/slaves' and '/frameworks' on
> the master, and '/slave/containers' endpoint on the slave.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
> src/tests/slave_tests.cpp 035db18db3a64a9e358c1c54cc18a4bdeb85d8bf
>
>
> Diff: https://reviews.apache.org/r/60847/diff/2/
>
>
> Testing
> -------
>
> Passed 'make check -j48'
> Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
> Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 --gtest_break_on_failure'
>
>
> Thanks,
>
> Quinn Leng
>
>
Re: Review Request 60847: Add test cases for /slaves,
/slave/containers, /frameworks endpoints.
Posted by Quinn Leng <qu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60847/
-----------------------------------------------------------
(Updated July 14, 2017, 1:43 a.m.)
Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
Summary (updated)
-----------------
Add test cases for /slaves, /slave/containers, /frameworks endpoints.
Bugs: MESOS-7630
https://issues.apache.org/jira/browse/MESOS-7630
Repository: mesos
Description (updated)
-------
Added query parameter test cases for '/slaves' and '/frameworks' on
the master, and '/slave/containers' endpoint on the slave.
Diffs (updated)
-----
src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
src/tests/slave_tests.cpp 035db18db3a64a9e358c1c54cc18a4bdeb85d8bf
Diff: https://reviews.apache.org/r/60847/diff/2/
Changes: https://reviews.apache.org/r/60847/diff/1-2/
Testing
-------
Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 --gtest_break_on_failure'
Thanks,
Quinn Leng
Re: Review Request 60847: Added test cases for /slaves, /containers,
/frameworks endpoints.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60847/#review180499
-----------------------------------------------------------
src/tests/slave_tests.cpp
Lines 2526 (patched)
<https://reviews.apache.org/r/60847/#comment255708>
ASSERT_TRUE
- Greg Mann
On July 13, 2017, 9:11 p.m., Quinn Leng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60847/
> -----------------------------------------------------------
>
> (Updated July 13, 2017, 9:11 p.m.)
>
>
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added query parameter test cases for '/slaves' and '/frameworks' on
> the master, and '/containers' endpoint on the agent.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
> src/tests/slave_tests.cpp 035db18db3a64a9e358c1c54cc18a4bdeb85d8bf
>
>
> Diff: https://reviews.apache.org/r/60847/diff/1/
>
>
> Testing
> -------
>
> Passed 'make check -j48'
> Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
> Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 --gtest_break_on_failure'
>
>
> Thanks,
>
> Quinn Leng
>
>
Re: Review Request 60847: Added test cases for /slaves, /containers,
/frameworks endpoints.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60847/#review180498
-----------------------------------------------------------
src/tests/slave_tests.cpp
Lines 2393-2397 (original), 2401-2420 (patched)
<https://reviews.apache.org/r/60847/#comment255707>
Let's namespace this to help readability:
```
// Launch two tasks, each under a different executor.
vector<TaskInfo> tasks;
{
TaskInfo task;
task.set_name("");
task.mutable_task_id()->set_value("1");
task.mutable_slave_id()->MergeFrom(offers->front().slave_id());
task.mutable_resources()->MergeFrom(
Resources::parse("cpus:1;mem:512").get());
task.mutable_executor()->MergeFrom(executor1);
tasks.push_back(task);
}
{
TaskInfo task;
task.set_name("");
task.mutable_task_id()->set_value("2");
task.mutable_slave_id()->MergeFrom(offers->front().slave_id());
task.mutable_resources()->MergeFrom(
Resources::parse("cpus:1;mem:512").get());
task.mutable_executor()->MergeFrom(executor2);
tasks.push_back(task);
}
```
- Greg Mann
On July 13, 2017, 9:11 p.m., Quinn Leng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60847/
> -----------------------------------------------------------
>
> (Updated July 13, 2017, 9:11 p.m.)
>
>
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added query parameter test cases for '/slaves' and '/frameworks' on
> the master, and '/containers' endpoint on the agent.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
> src/tests/slave_tests.cpp 035db18db3a64a9e358c1c54cc18a4bdeb85d8bf
>
>
> Diff: https://reviews.apache.org/r/60847/diff/1/
>
>
> Testing
> -------
>
> Passed 'make check -j48'
> Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
> Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 --gtest_break_on_failure'
>
>
> Thanks,
>
> Quinn Leng
>
>
Re: Review Request 60847: Added test cases for /slaves, /containers,
/frameworks endpoints.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60847/#review180487
-----------------------------------------------------------
src/tests/slave_tests.cpp
Line 2368 (original), 2368 (patched)
<https://reviews.apache.org/r/60847/#comment255685>
Let's explain a bit more:
"Create two executors so that we can launch tasks in two separate containers."
src/tests/slave_tests.cpp
Line 2408 (original), 2441 (patched)
<https://reviews.apache.org/r/60847/#comment255690>
Could we use `offers->front().id()` instead here?
src/tests/slave_tests.cpp
Lines 2416-2417 (original), 2467-2470 (patched)
<https://reviews.apache.org/r/60847/#comment255699>
Split this into two EXPECT_CALL invocations; one here and one below for the second request.
src/tests/slave_tests.cpp
Lines 2419-2434 (original), 2472-2504 (patched)
<https://reviews.apache.org/r/60847/#comment255697>
This formatting is a bit hard to read. How about:
```
// Construct the container statuses to be returned. Note that
// these container IDs will be different than the actual container
// IDs assigned by the agent, but creating them here allows us to
// easily confirm the output of '/containers'.
ContainerStatus containerStatus1;
ContainerStatus containerStatus2;
{
ContainerID parent;
parent.set_value("parent");
ContainerID child;
child.set_value("child1");
child.mutable_parent()->CopyFrom(parent);
containerStatus1.mutable_container_id()->CopyFrom(child);
CgroupInfo* cgroupInfo = containerStatus1.mutable_cgroup_info();
CgroupInfo::NetCls* netCls1 = cgroupInfo->mutable_net_cls();
netCls->set_classid(42);
NetworkInfo* networkInfo = containerStatus1.add_network_infos();
NetworkInfo::IPAddress* ipAddr = networkInfo->add_ip_addresses();
ipAddr->set_ip_address("192.168.1.20");
}
{
ContainerID child;
child.set_value("child2");
child.mutable_parent()->CopyFrom(parent);
containerStatus2.mutable_container_id()->CopyFrom(child);
CgroupInfo* cgroupInfo = containerStatus1.mutable_cgroup_info();
CgroupInfo::NetCls* netCls = cgroupInfo->mutable_net_cls();
netCls->set_classid(42);
NetworkInfo* networkInfo = containerStatus2.add_network_infos();
NetworkInfo::IPAddress* ipAddr = networkInfo->add_ip_addresses();
ipAddr->set_ip_address("192.168.1.21");
}
```
src/tests/slave_tests.cpp
Lines 2436-2437 (original), 2506-2510 (patched)
<https://reviews.apache.org/r/60847/#comment255698>
Split this into two `EXPECT_CALL` invocations. One here, and another one below.
src/tests/slave_tests.cpp
Lines 2614 (patched)
<https://reviews.apache.org/r/60847/#comment255684>
containerId1->value()
here and elsewhere
- Greg Mann
On July 13, 2017, 9:11 p.m., Quinn Leng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60847/
> -----------------------------------------------------------
>
> (Updated July 13, 2017, 9:11 p.m.)
>
>
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added query parameter test cases for '/slaves' and '/frameworks' on
> the master, and '/containers' endpoint on the agent.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
> src/tests/slave_tests.cpp 035db18db3a64a9e358c1c54cc18a4bdeb85d8bf
>
>
> Diff: https://reviews.apache.org/r/60847/diff/1/
>
>
> Testing
> -------
>
> Passed 'make check -j48'
> Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
> Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 --gtest_break_on_failure'
>
>
> Thanks,
>
> Quinn Leng
>
>
Re: Review Request 60847: Added test cases for /slaves, /containers,
/frameworks endpoints.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60847/#review180496
-----------------------------------------------------------
src/tests/slave_tests.cpp
Lines 2405 (patched)
<https://reviews.apache.org/r/60847/#comment255703>
offers->front()
src/tests/slave_tests.cpp
Lines 2413 (patched)
<https://reviews.apache.org/r/60847/#comment255704>
offers->front()
- Greg Mann
On July 13, 2017, 9:11 p.m., Quinn Leng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60847/
> -----------------------------------------------------------
>
> (Updated July 13, 2017, 9:11 p.m.)
>
>
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added query parameter test cases for '/slaves' and '/frameworks' on
> the master, and '/containers' endpoint on the agent.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
> src/tests/slave_tests.cpp 035db18db3a64a9e358c1c54cc18a4bdeb85d8bf
>
>
> Diff: https://reviews.apache.org/r/60847/diff/1/
>
>
> Testing
> -------
>
> Passed 'make check -j48'
> Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
> Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 --gtest_break_on_failure'
>
>
> Thanks,
>
> Quinn Leng
>
>
Re: Review Request 60847: Added test cases for /slaves, /containers,
/frameworks endpoints.
Posted by Quinn Leng <qu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60847/#review180479
-----------------------------------------------------------
src/tests/master_tests.cpp
Lines 2545 (patched)
<https://reviews.apache.org/r/60847/#comment255659>
s/std::string/string/ if possible, here and elsewhere.
src/tests/master_tests.cpp
Lines 2556-2557 (patched)
<https://reviews.apache.org/r/60847/#comment255660>
Fits on one line
src/tests/master_tests.cpp
Lines 2559-2560 (patched)
<https://reviews.apache.org/r/60847/#comment255661>
Newline.
src/tests/master_tests.cpp
Lines 2580 (patched)
<https://reviews.apache.org/r/60847/#comment255662>
s/slave/agent/
src/tests/master_tests.cpp
Lines 2604-2605 (patched)
<https://reviews.apache.org/r/60847/#comment255663>
One line
src/tests/master_tests.cpp
Lines 6013 (patched)
<https://reviews.apache.org/r/60847/#comment255664>
s/Ensure/Ensures/
src/tests/master_tests.cpp
Lines 6050 (patched)
<https://reviews.apache.org/r/60847/#comment255667>
Add a request with no query parameter to verify that both frameworks are returned.
If you remove the previous test, rename this one something like "FrameworkEndpointMultipleFrameworks"
src/tests/master_tests.cpp
Lines 6085-6117 (patched)
<https://reviews.apache.org/r/60847/#comment255666>
Kill this.
src/tests/master_tests.cpp
Lines 6121-6123 (patched)
<https://reviews.apache.org/r/60847/#comment255665>
Try to wait until master has marked framework as completed.
src/tests/slave_tests.cpp
Line 2361 (original), 2361 (patched)
<https://reviews.apache.org/r/60847/#comment255668>
s/executors. And/executors, and/
- Quinn Leng
On July 13, 2017, 9:11 p.m., Quinn Leng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60847/
> -----------------------------------------------------------
>
> (Updated July 13, 2017, 9:11 p.m.)
>
>
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added query parameter test cases for '/slaves' and '/frameworks' on
> the master, and '/containers' endpoint on the agent.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
> src/tests/slave_tests.cpp 035db18db3a64a9e358c1c54cc18a4bdeb85d8bf
>
>
> Diff: https://reviews.apache.org/r/60847/diff/1/
>
>
> Testing
> -------
>
> Passed 'make check -j48'
> Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
> Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 --gtest_break_on_failure'
>
>
> Thanks,
>
> Quinn Leng
>
>