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 15:41:00 GMT


> On Nov. 6, 2018, 5:58 a.m., Benjamin Mahler wrote:
> > Ah, unfortunately, the use of `os::setenv` is going to break our parallel test runner
:(
> > 
> > We could have reinitialize take a flags object?
> 
> Benjamin Bannier wrote:
>     Not sure why this would be an issue for parallel test execution as e.g., the in-tree
parallel test runner executes shards as separate processes (as also intended by gtest).
>     
>     I'd be more worried about concurrently executing actors reading from these globals
while we mutate them. We have seen similar constructs in tests cause hard failures before.
Similar issue below, and with `os::unsetenv`. Explicitly passing flags to `reinitialize` could
allow us to pass this information down to the point where no actors are running anymore and
we can safely set them.

As I was lying in bed afterwards I realized it doesn't conflict with the parallel test runner,
and that I had mixed that up in my mind with the tests that concurrently touched getenv/setenv/unsetenv
:)


- Benjamin


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


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