Re: Review Request 49841: Reorganized the routing tests into basic and advanced groups.

[Avinash sridharan]

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


Ship it!




Ship It!

- Avinash sridharan


On July 8, 2016, 11:57 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49841/
> ---
> 
> (Updated July 8, 2016, 11:57 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Advanced tests need some higher version of libnl.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/routing_tests.cpp 
> 452a56da84f7508709d6e71f121bcf6219f992e6 
> 
> Diff: https://reviews.apache.org/r/49841/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49841: Reorganized the routing tests into basic and advanced groups.

[Jie Yu]

> On July 11, 2016, 2:46 p.m., Qian Zhang wrote:
> > So even we reorganize the routing tests into basic and advanced groups, all 
> > these tests are still in the same file (routing_tests.cpp which is 
> > currently guarded by `WITH_NETWORK_ISOLATOR` in Makefile.am) which means 
> > they will be compiled as a whole. So what if user just wants to run basic 
> > tests but not the advanded tests? In this case, they also need the newer 
> > version of libnl? Maybe we should reorganize them into two separate files, 
> > e.g., routing_tests.cpp and advanced_routing_tests.cpp, the former can work 
> > with older version of libnl, and the later has to work with newer version 
> > of libnl.

I can use #ifdef blocks to solve that for now.


- Jie


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


On July 8, 2016, 11:57 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49841/
> ---
> 
> (Updated July 8, 2016, 11:57 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Advanced tests need some higher version of libnl.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/routing_tests.cpp 
> 452a56da84f7508709d6e71f121bcf6219f992e6 
> 
> Diff: https://reviews.apache.org/r/49841/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49841: Reorganized the routing tests into basic and advanced groups.

[Jie Yu]

> On July 11, 2016, 3:42 p.m., Avinash sridharan wrote:
> > src/tests/containerizer/routing_tests.cpp, line 258
> > 
> >
> > Instead of failing over here should we be disabling these tests if the 
> > libnl version is not correct?
> > 
> > That's the convention we have using with the other tests 
> > (cgroups/netcls). Does it make sense to add a filter for these tests?

Yeah, we should follow up with a patch. This patch just do the splitting.


- Jie


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


On July 8, 2016, 11:57 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49841/
> ---
> 
> (Updated July 8, 2016, 11:57 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Advanced tests need some higher version of libnl.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/routing_tests.cpp 
> 452a56da84f7508709d6e71f121bcf6219f992e6 
> 
> Diff: https://reviews.apache.org/r/49841/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49841: Reorganized the routing tests into basic and advanced groups.

[Avinash sridharan]

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




src/tests/containerizer/routing_tests.cpp (line 207)


Instead of failing over here should we be disabling these tests if the 
libnl version is not correct?

That's the convention we have using with the other tests (cgroups/netcls). 
Does it make sense to add a filter for these tests?


- Avinash sridharan


On July 8, 2016, 11:57 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49841/
> ---
> 
> (Updated July 8, 2016, 11:57 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Advanced tests need some higher version of libnl.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/routing_tests.cpp 
> 452a56da84f7508709d6e71f121bcf6219f992e6 
> 
> Diff: https://reviews.apache.org/r/49841/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49841: Reorganized the routing tests into basic and advanced groups.

[Qian Zhang]

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



So even we reorganize the routing tests into basic and advanced groups, all 
these tests are still in the same file (routing_tests.cpp which is currently 
guarded by `WITH_NETWORK_ISOLATOR` in Makefile.am) which means they will be 
compiled as a whole. So what if user just wants to run basic tests but not the 
advanded tests? In this case, they also need the newer version of libnl? Maybe 
we should reorganize them into two separate files, e.g., routing_tests.cpp and 
advanced_routing_tests.cpp, the former can work with older version of libnl, 
and the later has to work with newer version of libnl.

- Qian Zhang


On July 9, 2016, 7:57 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49841/
> ---
> 
> (Updated July 9, 2016, 7:57 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Advanced tests need some higher version of libnl.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/routing_tests.cpp 
> 452a56da84f7508709d6e71f121bcf6219f992e6 
> 
> Diff: https://reviews.apache.org/r/49841/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49841: Reorganized the routing tests into basic and advanced groups.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49835, 49839, 49840, 49841]

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 8, 2016, 11:57 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49841/
> ---
> 
> (Updated July 8, 2016, 11:57 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Advanced tests need some higher version of libnl.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/routing_tests.cpp 
> 452a56da84f7508709d6e71f121bcf6219f992e6 
> 
> Diff: https://reviews.apache.org/r/49841/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>