Re: Review Request 53950: Avoided hardcoding ports in some tests.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53950/#review156907
---


Ship it!




Ship It!

- Alexander Rukletsov


On Nov. 24, 2016, 5:39 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53950/
> ---
> 
> (Updated Nov. 24, 2016, 5:39 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided hardcoding ports in some tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
>   src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 
> 
> Diff: https://reviews.apache.org/r/53950/diff/
> 
> 
> Testing
> ---
> 
> * `make check` (SSL build OS X clang trunk w/ optimizations; default build 
> ubuntu-14.04 gcc-4.8.4 w/o optimizations)
> * tested with in-tree parallel test runner: `GTEST_FILTER='HealthCheckTest.*' 
>  ~/src/mesos/support/mesos-gtest-runner.py ./mesos-tests` which would fail 
> about for about 50% before this fix, and not measurably anymore after.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53950/#review156898
---



Patch looks great!

Reviews applied: [53948, 53986, 53949, 53950]

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 Nov. 24, 2016, 5:39 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53950/
> ---
> 
> (Updated Nov. 24, 2016, 5:39 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided hardcoding ports in some tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
>   src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 
> 
> Diff: https://reviews.apache.org/r/53950/diff/
> 
> 
> Testing
> ---
> 
> * `make check` (SSL build OS X clang trunk w/ optimizations; default build 
> ubuntu-14.04 gcc-4.8.4 w/o optimizations)
> * tested with in-tree parallel test runner: `GTEST_FILTER='HealthCheckTest.*' 
>  ~/src/mesos/support/mesos-gtest-runner.py ./mesos-tests` which would fail 
> about for about 50% before this fix, and not measurably anymore after.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[Benjamin Bannier]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53950/
---

(Updated Nov. 24, 2016, 6:39 p.m.)


Review request for mesos, Alexander Rukletsov and haosdent huang.


Bugs: MESOS-6618
https://issues.apache.org/jira/browse/MESOS-6618


Repository: mesos


Description
---

Avoided hardcoding ports in some tests.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
  src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 

Diff: https://reviews.apache.org/r/53950/diff/


Testing (updated)
---

* `make check` (SSL build OS X clang trunk w/ optimizations; default build 
ubuntu-14.04 gcc-4.8.4 w/o optimizations)
* tested with in-tree parallel test runner: `GTEST_FILTER='HealthCheckTest.*'  
~/src/mesos/support/mesos-gtest-runner.py ./mesos-tests` which would fail about 
for about 50% before this fix, and not measurably anymore after.


Thanks,

Benjamin Bannier

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[haosdent huang]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53950/#review156693
---


Ship it!




Ship It!

- haosdent huang


On Nov. 22, 2016, 7:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53950/
> ---
> 
> (Updated Nov. 22, 2016, 7:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided hardcoding ports in some tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
>   src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 
> 
> Diff: https://reviews.apache.org/r/53950/diff/
> 
> 
> Testing
> ---
> 
> make check (SSL build OS X clang trunk w/ optimizations; default build 
> ubuntu-14.04 gcc-4.8.4 w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[Gastón Kleiman]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53950/#review156623
---


Ship it!




Ship It!

- Gastón Kleiman


On Nov. 22, 2016, 7:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53950/
> ---
> 
> (Updated Nov. 22, 2016, 7:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided hardcoding ports in some tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
>   src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 
> 
> Diff: https://reviews.apache.org/r/53950/diff/
> 
> 
> Testing
> ---
> 
> make check (SSL build OS X clang trunk w/ optimizations; default build 
> ubuntu-14.04 gcc-4.8.4 w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[Benjamin Bannier]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53950/
---

(Updated Nov. 22, 2016, 8:49 p.m.)


Review request for mesos, Alexander Rukletsov and haosdent huang.


Changes
---

Addressed gaston's comment.


Bugs: MESOS-6618
https://issues.apache.org/jira/browse/MESOS-6618


Repository: mesos


Description
---

Avoided hardcoding ports in some tests.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
  src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 

Diff: https://reviews.apache.org/r/53950/diff/


Testing
---

make check (SSL build OS X clang trunk w/ optimizations; default build 
ubuntu-14.04 gcc-4.8.4 w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[Benjamin Bannier]

> On Nov. 22, 2016, 6:18 p.m., Gastón Kleiman wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp, lines 3972-3974
> > 
> >
> > Why is this done in a different way than in the other file?
> > 
> > As far as I can see, `testPort_.get()` will abort if there is an error.

In the end all these styles are equivalent, with above variant being marginally 
more verbose. Using the simplified approach to immediately `get` the value now 
like elsewhere.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53950/#review156589
---


On Nov. 22, 2016, 8:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53950/
> ---
> 
> (Updated Nov. 22, 2016, 8:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided hardcoding ports in some tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
>   src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 
> 
> Diff: https://reviews.apache.org/r/53950/diff/
> 
> 
> Testing
> ---
> 
> make check (SSL build OS X clang trunk w/ optimizations; default build 
> ubuntu-14.04 gcc-4.8.4 w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[Gastón Kleiman]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53950/#review156589
---




src/tests/containerizer/docker_containerizer_tests.cpp (lines 3972 - 3974)


Why is this done in a different way than in the other file?

As far as I can see, `testPort_.get()` will abort if there is an error.


- Gastón Kleiman


On Nov. 22, 2016, 3:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53950/
> ---
> 
> (Updated Nov. 22, 2016, 3:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided hardcoding ports in some tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
>   src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 
> 
> Diff: https://reviews.apache.org/r/53950/diff/
> 
> 
> Testing
> ---
> 
> make check (SSL build OS X clang trunk w/ optimizations; default build 
> ubuntu-14.04 gcc-4.8.4 w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[Benjamin Bannier]

> On Nov. 22, 2016, 2:47 p.m., haosdent huang wrote:
> > src/tests/health_check_tests.cpp, line 105
> > 
> >
> > I am thinking if we could 
> > 
> > ```
> > const uint16_t testPort = getFreePort().get();
> > ```
> > 
> > Since no all the health check test cases require a free port.

Makes sense, code adjusted.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53950/#review156571
---


On Nov. 22, 2016, 4:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53950/
> ---
> 
> (Updated Nov. 22, 2016, 4:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided hardcoding ports in some tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
>   src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 
> 
> Diff: https://reviews.apache.org/r/53950/diff/
> 
> 
> Testing
> ---
> 
> make check (SSL build OS X clang trunk w/ optimizations; default build 
> ubuntu-14.04 gcc-4.8.4 w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[Benjamin Bannier]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53950/
---

(Updated Nov. 22, 2016, 4:49 p.m.)


Review request for mesos, Alexander Rukletsov and haosdent huang.


Changes
---

Addressed haosdent's comment.


Bugs: MESOS-6618
https://issues.apache.org/jira/browse/MESOS-6618


Repository: mesos


Description
---

Avoided hardcoding ports in some tests.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
  src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 

Diff: https://reviews.apache.org/r/53950/diff/


Testing
---

make check (SSL build OS X clang trunk w/ optimizations; default build 
ubuntu-14.04 gcc-4.8.4 w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[haosdent huang]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53950/#review156571
---




src/tests/health_check_tests.cpp (line 105)


I am thinking if we could 

```
const uint16_t testPort = getFreePort().get();
```

Since no all the health check test cases require a free port.


- haosdent huang


On Nov. 21, 2016, 6:15 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53950/
> ---
> 
> (Updated Nov. 21, 2016, 6:15 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided hardcoding ports in some tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
>   src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 
> 
> Diff: https://reviews.apache.org/r/53950/diff/
> 
> 
> Testing
> ---
> 
> make check (SSL build OS X clang trunk w/ optimizations; default build 
> ubuntu-14.04 gcc-4.8.4 w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[Benjamin Bannier]

> On Nov. 21, 2016, 8:01 p.m., Gastón Kleiman wrote:
> > `src/tests/containerizer/port_mapping_tests.cpp` also uses a hard-coded 
> > port (they were the "inspiration" behind tht health check tests), I guess 
> > we should update those as well.

Thanks for the pointer, these tests look indeed problematic as well. Since 
fixing these seems to be more involved, I created 
https://issues.apache.org/jira/browse/MESOS-6620 to track these independently. 
We should still try to make sure that the infrastructure created in 
https://reviews.apache.org/r/53949/ is suitable for use cases in 
`port_mapping_tests.cpp`.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53950/#review156489
---


On Nov. 21, 2016, 7:15 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53950/
> ---
> 
> (Updated Nov. 21, 2016, 7:15 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided hardcoding ports in some tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
>   src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 
> 
> Diff: https://reviews.apache.org/r/53950/diff/
> 
> 
> Testing
> ---
> 
> make check (SSL build OS X clang trunk w/ optimizations; default build 
> ubuntu-14.04 gcc-4.8.4 w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53950/#review156501
---



Patch looks great!

Reviews applied: [53948, 53949, 53950]

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 Nov. 21, 2016, 6:15 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53950/
> ---
> 
> (Updated Nov. 21, 2016, 6:15 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided hardcoding ports in some tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
>   src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 
> 
> Diff: https://reviews.apache.org/r/53950/diff/
> 
> 
> Testing
> ---
> 
> make check (SSL build OS X clang trunk w/ optimizations; default build 
> ubuntu-14.04 gcc-4.8.4 w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[Gastón Kleiman]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53950/#review156489
---



`src/tests/containerizer/port_mapping_tests.cpp` also uses a hard-coded port 
(they were the "inspiration" behind tht health check tests), I guess we should 
update those as well.

- Gastón Kleiman


On Nov. 21, 2016, 6:15 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53950/
> ---
> 
> (Updated Nov. 21, 2016, 6:15 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided hardcoding ports in some tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
>   src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 
> 
> Diff: https://reviews.apache.org/r/53950/diff/
> 
> 
> Testing
> ---
> 
> make check (SSL build OS X clang trunk w/ optimizations; default build 
> ubuntu-14.04 gcc-4.8.4 w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>