> On June 7, 2018, 6:21 p.m., Andrew Schwartzmeyer wrote: > > 3rdparty/stout/tests/os/env_tests.cpp > > Lines 74-77 (original), 75-78 (patched) > > <https://reviews.apache.org/r/67460/diff/1/?file=2035347#file2035347line75> > > > > Yeah, it definitely seems like we should keep this logic. I'm bringing > > @jpeach in, especially considering the test was added somewhat recently.
Using `os::getenv` doesn't capture the distinction between `unsetenv` and `eraseenv`, since `unsetenv` will make the value disappear from the `environ` array, but won't actually destroy the current value, which is what is desired here. - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67460/#review204454 ----------------------------------------------------------- On June 7, 2018, 6:21 p.m., Radhika Jandhyala wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67460/ > ----------------------------------------------------------- > > (Updated June 7, 2018, 6:21 p.m.) > > > Review request for Andrew Schwartzmeyer and James Peach. > > > Bugs: MESOS-5813 > https://issues.apache.org/jira/browse/MESOS-5813 > > > Repository: mesos > > > Description > ------- > > Enable stouttest EnvTest.EraseEnv. > > > Diffs > ----- > > 3rdparty/stout/tests/os/env_tests.cpp > b5b124dc6316e661af6dd90335ade5283c26d9f2 > > > Diff: https://reviews.apache.org/r/67460/diff/1/ > > > Testing > ------- > > Here is one test I could enable for windows. However, one thing I notice is > that the original test specifically wanted to call ::getenv which I had to > change to os::getenv. So the test is a little different now. I can abandon > this review if you think that makes more sense. > > > Thanks, > > Radhika Jandhyala > >
