mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 67958: Added LibprocessTest for easily configuring the library for a test.
Date Tue, 06 Nov 2018 05:58:30 GMT

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



Ah, unfortunately, the use of `os::setenv` is going to break our parallel test runner :(

We could have reinitialize take a flags object?


3rdparty/libprocess/include/process/gtest.hpp
Lines 38-44 (patched)
<https://reviews.apache.org/r/67958/#comment295001>

    Add an extra newline on top and bottom of this?



3rdparty/libprocess/include/process/gtest.hpp
Lines 38-40 (patched)
<https://reviews.apache.org/r/67958/#comment295002>

    Is this best framed as a forward declaration? It seems to be better described as us exposing
the declaration of the internal function since we need it for this? I'm left wondering after
reading this whether it's exposed in some header or we're just surfacing it from it's .cpp
file.



3rdparty/libprocess/include/process/gtest.hpp
Lines 52-71 (patched)
<https://reviews.apache.org/r/67958/#comment295004>

    It looks like this doesn't handle being called twice (teardown then re-initializes back
to the 2nd last call to re-initialize, rather than the original state).
    
    We could fix that or crash if it's called twice?



3rdparty/libprocess/include/process/gtest.hpp
Lines 62 (patched)
<https://reviews.apache.org/r/67958/#comment295003>

    This prevents running tests in parallel :(
    
    Is there any way to avoid this?


- Benjamin Mahler


On July 18, 2018, 1:19 a.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67958/
> -----------------------------------------------------------
> 
> (Updated July 18, 2018, 1:19 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> There was not a generic way for configuring and then reinitializing
> the libprocess library in a test and the LibprocessTest fixture
> provides this functionality.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/gtest.hpp fd4de8ab7c79940519b2e890a9b0b615ca9ca292

> 
> 
> Diff: https://reviews.apache.org/r/67958/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


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