> On Jan. 7, 2017, 12:28 a.m., Kapil Arya wrote:
> > LGTM. Have you run any tests where the existing `CHECK_SOME` would fail and
> > the new code will behave properly? It might be worth running it at least
> > for one test. Maybe, in one of the containerizer value, just set it to
> > `Error()` to validate the idea.
I tested this in internal CI on an incompletly configured machine; after this
fix I was able to see failures in e.g., from `PortMappingIsolatorTest` where
previously the code would have just aborted. As a test I also redefined to be
just `return Error("")` which did not lead to aborts of the test run via
`HTTPCommandExecutorTest.TerminateWithACK` or
`ContainerLoggerTest.DefaultToSandbox` anymore.
> On Jan. 7, 2017, 12:28 a.m., Kapil Arya wrote:
> > src/tests/master_quota_tests.cpp, lines 92-94
> > <https://reviews.apache.org/r/55269/diff/1/?file=1598747#file1598747line92>
> >
> > Should we put it in a separate RR?
I felt this was just a refactoring similar to the instances of `return
Error(...)` I added elsewhere in this patch, so I'd say no. Would you feel this
would warrant its own patch?
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55269/#review160788
-----------------------------------------------------------
On Jan. 6, 2017, 4:16 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55269/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2017, 4:16 p.m.)
>
>
> Review request for mesos and Kapil Arya.
>
>
> Bugs: MESOS-6860
> https://issues.apache.org/jira/browse/MESOS-6860
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Avoided use of CHECK macros.
>
>
> Diffs
> -----
>
> src/tests/command_executor_tests.cpp
> f4e7044b82e8e81d6430551dc0b2a288db10bc3c
> src/tests/container_logger_tests.cpp
> a8bc83449816ec8ba3f2b7e02594e4abe39c265d
> src/tests/containerizer/cgroups_tests.cpp
> 0afaec6ae948cabf1472bf01103210d8f9809cb1
> src/tests/containerizer/port_mapping_tests.cpp
> 207742f165e2c91a917ec293689c9030931f29db
> src/tests/cram_md5_authentication_tests.cpp
> 1f414282b8d2aba7403f8617a30fa15c4a694d02
> src/tests/credentials_tests.cpp ed50609cc5ed1937d2bcd782451245e0b5889ed5
> src/tests/disk_quota_tests.cpp 90c42b301d749e7a9c6d6d42a51e929ec66d761c
> src/tests/fetcher_cache_tests.cpp 03e817d46194d451eceb70f4cebb54dfdcb4c2e7
> src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b
> src/tests/partition_tests.cpp 3afdbba13a9753edce17b24ff6d1898d3cc6de62
> src/tests/paths_tests.cpp a8aa5c26db7bd21f54eabaddf13a86582abaa2c3
> src/tests/persistent_volume_endpoints_tests.cpp
> 1d40380dedbd6f0e1402b9a37d8a231cf73881f7
> src/tests/persistent_volume_tests.cpp
> 8198b6b5ad323d17835ba067c7ff3d34ef948125
> src/tests/reservation_endpoints_tests.cpp
> d94fe29e5972e6ed0011ca00cc56fe1c20cda495
> src/tests/slave_tests.cpp d633a74d6b342239fbca0b44eec281eb315df5ff
>
> Diff: https://reviews.apache.org/r/55269/diff/
>
>
> Testing
> -------
>
> Tested on various Linux configurations in internal CI.
>
>
> Thanks,
>
> Benjamin Bannier
>
>
