mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 55269: Avoided use of CHECK macros.
Date Tue, 10 Jan 2017 13:19:08 GMT


> 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
> 
>


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