mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Qian Zhang <zhq527...@gmail.com>
Subject Re: Review Request 61874: Added a test for IPv6 containers on docker user networks.
Date Thu, 24 Aug 2017 10:21:38 GMT

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


Fix it, then Ship it!





src/tests/containerizer/docker_containerizer_tests.cpp
Lines 4737 (patched)
<https://reviews.apache.org/r/61874/#comment259778>

    To be consistent with `TearDown()`, better to change to:
    ```
    ASSERT_SOME(s) << "Unable to create the docker IPv6 network: "
                       << DOCKER_IPv6_NETWORK;
    ```
    
    Or we can change the message in `TearDown()` to be consistent with the message here, either
is OK to me.



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 4747 (patched)
<https://reviews.apache.org/r/61874/#comment259773>

    To be consistent with the message in `TearDown()`, better to change the message here to:
    ```
    Unable to create the docker IPv6 network
    ```



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 4753 (patched)
<https://reviews.apache.org/r/61874/#comment259774>

    I think we should call this method in the end rather than at the beginning.



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 4771 (patched)
<https://reviews.apache.org/r/61874/#comment259775>

    s/created/deleted/



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 4773-4774 (patched)
<https://reviews.apache.org/r/61874/#comment259776>

    Add a newline between.



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 4776 (patched)
<https://reviews.apache.org/r/61874/#comment259777>

    Better to change to `Unable to delete the docker IPv6 network `.



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 4782-4785 (patched)
<https://reviews.apache.org/r/61874/#comment259779>

    Better to merge these 4 lines into 3 lines.



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 4879-4882 (patched)
<https://reviews.apache.org/r/61874/#comment259780>

    Why do we need to check this?



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 4922 (patched)
<https://reviews.apache.org/r/61874/#comment259781>

    Should it be `ASSERT_EQ`?


- Qian Zhang


On Aug. 24, 2017, 8:59 a.m., Avinash sridharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61874/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2017, 8:59 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-7807
>     https://issues.apache.org/jira/browse/MESOS-7807
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The test creates a IPv6 docker user network. It than launches an alpine
> image on the docker user network. Finally it checks if the IP address
> allocated to the container are reflected correctly in state.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 6856ca22570183b022d401d1fa8f59d819783eed

> 
> 
> Diff: https://reviews.apache.org/r/61874/diff/1/
> 
> 
> Testing
> -------
> 
> sudo bin/mesos-tests.sh --gtest_filter="*.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container"
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from DockerContainerizerIPv6UserNetworkTest
> [ RUN      ] DockerContainerizerIPv6UserNetworkTest.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container
> [       OK ] DockerContainerizerIPv6UserNetworkTest.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container
(15660 ms)
> [----------] 1 test from DockerContainerizerIPv6UserNetworkTest (15664 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (15713 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message