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