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




src/tests/containerizer/linux_seccomp_config.hpp
Lines 17 (patched)
<https://reviews.apache.org/r/69420/#comment296917>

    I would suggest `__TEST_LINUX_SECCOMP_CONFIG_HPP__` as what we did in the 
other header files under `src/test/containerizer/`.
    
    And should this file named as `linux_seccomp_profile.hpp`? Or do we really 
this file? Can we just define the default profile in 
`linux_seccomp_isolator_tests.cpp`?



src/tests/containerizer/linux_seccomp_config.hpp
Lines 24 (patched)
<https://reviews.apache.org/r/69420/#comment296918>

    Can you please mention this is Docker's default seccomp profile and the 
link to it?



src/tests/containerizer/linux_seccomp_config.hpp
Lines 26 (patched)
<https://reviews.apache.org/r/69420/#comment296916>

    I would suggest to replace the tab with two spaces.



src/tests/containerizer/linux_seccomp_isolator_tests.cpp
Lines 47 (patched)
<https://reviews.apache.org/r/69420/#comment296922>

    Can we add some tests to verify `includes`, `excludes` and the feature 
interaction with capability?



src/tests/containerizer/linux_seccomp_isolator_tests.cpp
Lines 227 (patched)
<https://reviews.apache.org/r/69420/#comment296919>

    Should we check `EXPECT_WTERMSIG_EQ(SIGKILL, wait.get()->status());`?



src/tests/containerizer/linux_seccomp_isolator_tests.cpp
Lines 262 (patched)
<https://reviews.apache.org/r/69420/#comment296920>

    I think we should not do this for this test because what we want to verify 
is **overriding** the default profile.


- Qian Zhang


On Nov. 28, 2018, 7:47 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69420/
> -----------------------------------------------------------
> 
> (Updated Nov. 28, 2018, 7:47 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9409
>     https://issues.apache.org/jira/browse/MESOS-9409
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 5da75d3ef263b86af8d914d82cae889830097042 
>   src/tests/CMakeLists.txt 553516ad66cab4480b7211950fc726b7d9a3869b 
>   src/tests/containerizer/linux_seccomp_config.hpp PRE-CREATION 
>   src/tests/containerizer/linux_seccomp_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69420/diff/3/
> 
> 
> Testing
> -------
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Reply via email to