You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Till Toenshoff <to...@me.com> on 2015/12/10 16:30:40 UTC
Re: Review Request 40553: Enable mesos tests installation
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40553/#review107994
-----------------------------------------------------------
Great work James, much appreciated.
And yes, it is tedious and hence the results appear unclean at the first glance here and there. I did play around a bit with the introduced helpers / getters, trying to streamline them but in the end could not come with something that had more beauty than your drafted solution.
There are a few style nits in here, but I refrained from commenting on those in this early stage.
src/tests/utils.cpp (lines 71 - 77)
<https://reviews.apache.org/r/40553/#comment167349>
I still don't understand why the oversubscription tests need to have this installed in the LIBDIR and not in MODULE_DIR folder. We need to find some proper reasoning here before making so much extra fuzz for this, I feel.
configure.ac (line 330)
<https://reviews.apache.org/r/40553/#comment169379>
Could you please explain the [1] for the negative case?
src/tests/containerizer/memory_test_helper.cpp (line 187)
<https://reviews.apache.org/r/40553/#comment169384>
Can you explain briefly what you mean here by "too many dependencies" - seemed to me we only "drag" in "stout/json.hpp"?!
src/tests/containerizer/memory_test_helper.cpp (line 188)
<https://reviews.apache.org/r/40553/#comment169383>
`getTestHelper` ?
src/tests/module_tests.cpp (lines 60 - 68)
<https://reviews.apache.org/r/40553/#comment169382>
Seems this could be covered by `getModulePath`, no?
- Till Toenshoff
On Nov. 30, 2015, 6:43 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40553/
> -----------------------------------------------------------
>
> (Updated Nov. 30, 2015, 6:43 p.m.)
>
>
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
>
>
> Bugs: MESOS-3608
> https://issues.apache.org/jira/browse/MESOS-3608
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch enables the installation mesos-tests and its dependencies
> and helper tool. The goal is to allow operators to build a separate
> test package that can be run at deployment time to verify that Mesos
> works in the deployment environment.
>
> Since the build directory is searched first, to run it on a host
> that has a build tree, you need to specify a non-existent tree:
>
> ~ $ $PREFIX/libexec/mesos/tests/mesos-tests --build_dir=/none
>
> - Add --enable-tests-install
> - Fix mesos-tests gmock dependencies
> - Optionally install tests, helpers and test modules
> - Add utility helpers to find various test resources
>
> Current test status:
>
> [==========] 784 tests from 106 test cases ran. (135304 ms total)
> [ PASSED ] 780 tests.
> [ FAILED ] 4 tests, listed below:
> [ FAILED ] ExamplesTest.TestFramework
> [ FAILED ] ExamplesTest.NoExecutorFramework
> [ FAILED ] ExamplesTest.EventCallFramework
> [ FAILED ] ExamplesTest.PersistentVolumeFramework
>
>
> Diffs
> -----
>
> configure.ac b30a8d30076f3068fd7d5fc8ccea1982639e998a
> src/Makefile.am fd38cfa73d81a98c819378f99a766e2ddb7e1a04
> src/tests/containerizer/launch_tests.cpp c7ebe2606e4ff99ced90342dd16e0b4bf02bc504
> src/tests/containerizer/memory_test_helper.cpp 4a3de2e3c887aa6afc604588850e1386f92d8c11
> src/tests/containerizer/mesos_containerizer_tests.cpp fe679354d04d68b68e168cd8c4eab23898f6532f
> src/tests/containerizer/ns_tests.cpp 603e54b7303c5aa15e2c5715dc7a2f7e7d39541b
> src/tests/containerizer/port_mapping_tests.cpp 9bcf05ec071b44156b57d8515f47ee6a8bbfdfa0
> src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf
> src/tests/fetcher_tests.cpp 99c494a2167ea9ec94e63d92980df17a4089f95a
> src/tests/health_check_tests.cpp b1454b085b36bb7c4d8ef012c764cd8466b4fb02
> src/tests/mesos.cpp a75182ec7f6d0c6063bbb36a6ccf4e8a4a81c8dd
> src/tests/module.cpp e272bf0eccb61ae54440ec79adac8efad804c828
> src/tests/module_tests.cpp b5cfbcd4885a4f2c20b19e00958fedda81afa6e0
> src/tests/oversubscription_tests.cpp 0333281c247dd182860a49f39be791c00679bf6b
> src/tests/script.cpp ee44fef29fb40e414d7507168091ee5cd0d15736
> src/tests/slave_tests.cpp 4975bea8a7a701e0414426760692720f73dea7f5
> src/tests/utils.hpp a6cca472f4dfab12cd6eccab6972206d842177aa
> src/tests/utils.cpp 877139e97249761658dce3b1058cdc2e2a52367b
>
> Diff: https://reviews.apache.org/r/40553/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 40553: Enable mesos tests installation
Posted by James Peach <jp...@apache.org>.
> On Dec. 10, 2015, 3:30 p.m., Till Toenshoff wrote:
> > src/tests/utils.cpp, lines 71-77
> > <https://reviews.apache.org/r/40553/diff/1/?file=1134906#file1134906line71>
> >
> > I still don't understand why the oversubscription tests need to have this installed in the LIBDIR and not in MODULE_DIR folder. We need to find some proper reasoning here before making so much extra fuzz for this, I feel.
I made ``PKGMODULEDIR`` a first class installation directory and installed ``lib_fixed_resource_estimator`` there (with compatibility symlinks). This let me clean up and remove ``getModuleDir``.
> On Dec. 10, 2015, 3:30 p.m., Till Toenshoff wrote:
> > configure.ac, line 330
> > <https://reviews.apache.org/r/40553/diff/2/?file=1148521#file1148521line330>
> >
> > Could you please explain the [1] for the negative case?
The ```[1]``` is the argument to ```AC_DEFINE``` not to the ```AS_IF```. I'll split the line to make it clearer.
> On Dec. 10, 2015, 3:30 p.m., Till Toenshoff wrote:
> > src/tests/containerizer/memory_test_helper.cpp, line 187
> > <https://reviews.apache.org/r/40553/diff/2/?file=1148524#file1148524line187>
> >
> > Can you explain briefly what you mean here by "too many dependencies" - seemed to me we only "drag" in "stout/json.hpp"?!
I took another look and I just need to link against ``$(mesos_tests_DEPENDENCIES)``.
> On Dec. 10, 2015, 3:30 p.m., Till Toenshoff wrote:
> > src/tests/module_tests.cpp, lines 60-68
> > <https://reviews.apache.org/r/40553/diff/2/?file=1148533#file1148533line60>
> >
> > Seems this could be covered by `getModulePath`, no?
Yeh I can use ``getModulePath`` here. I think I was trying to minimize the changes :)
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40553/#review107994
-----------------------------------------------------------
On Nov. 30, 2015, 6:43 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40553/
> -----------------------------------------------------------
>
> (Updated Nov. 30, 2015, 6:43 p.m.)
>
>
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
>
>
> Bugs: MESOS-3608
> https://issues.apache.org/jira/browse/MESOS-3608
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch enables the installation mesos-tests and its dependencies
> and helper tool. The goal is to allow operators to build a separate
> test package that can be run at deployment time to verify that Mesos
> works in the deployment environment.
>
> Since the build directory is searched first, to run it on a host
> that has a build tree, you need to specify a non-existent tree:
>
> ~ $ $PREFIX/libexec/mesos/tests/mesos-tests --build_dir=/none
>
> - Add --enable-tests-install
> - Fix mesos-tests gmock dependencies
> - Optionally install tests, helpers and test modules
> - Add utility helpers to find various test resources
>
> Current test status:
>
> [==========] 784 tests from 106 test cases ran. (135304 ms total)
> [ PASSED ] 780 tests.
> [ FAILED ] 4 tests, listed below:
> [ FAILED ] ExamplesTest.TestFramework
> [ FAILED ] ExamplesTest.NoExecutorFramework
> [ FAILED ] ExamplesTest.EventCallFramework
> [ FAILED ] ExamplesTest.PersistentVolumeFramework
>
>
> Diffs
> -----
>
> configure.ac b30a8d30076f3068fd7d5fc8ccea1982639e998a
> src/Makefile.am fd38cfa73d81a98c819378f99a766e2ddb7e1a04
> src/tests/containerizer/launch_tests.cpp c7ebe2606e4ff99ced90342dd16e0b4bf02bc504
> src/tests/containerizer/memory_test_helper.cpp 4a3de2e3c887aa6afc604588850e1386f92d8c11
> src/tests/containerizer/mesos_containerizer_tests.cpp fe679354d04d68b68e168cd8c4eab23898f6532f
> src/tests/containerizer/ns_tests.cpp 603e54b7303c5aa15e2c5715dc7a2f7e7d39541b
> src/tests/containerizer/port_mapping_tests.cpp 9bcf05ec071b44156b57d8515f47ee6a8bbfdfa0
> src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf
> src/tests/fetcher_tests.cpp 99c494a2167ea9ec94e63d92980df17a4089f95a
> src/tests/health_check_tests.cpp b1454b085b36bb7c4d8ef012c764cd8466b4fb02
> src/tests/mesos.cpp a75182ec7f6d0c6063bbb36a6ccf4e8a4a81c8dd
> src/tests/module.cpp e272bf0eccb61ae54440ec79adac8efad804c828
> src/tests/module_tests.cpp b5cfbcd4885a4f2c20b19e00958fedda81afa6e0
> src/tests/oversubscription_tests.cpp 0333281c247dd182860a49f39be791c00679bf6b
> src/tests/script.cpp ee44fef29fb40e414d7507168091ee5cd0d15736
> src/tests/slave_tests.cpp 4975bea8a7a701e0414426760692720f73dea7f5
> src/tests/utils.hpp a6cca472f4dfab12cd6eccab6972206d842177aa
> src/tests/utils.cpp 877139e97249761658dce3b1058cdc2e2a52367b
>
> Diff: https://reviews.apache.org/r/40553/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> James Peach
>
>