> On March 16, 2016, 5:48 a.m., Michael Park wrote:
> > src/tests/fetcher_cache_tests.cpp, lines 167-169
> > <https://reviews.apache.org/r/43629/diff/8/?file=1300230#file1300230line167>
> >
> >     Could you explain this NOTE? It looks like we do `fetcherProcess = new 
> > MockFetcherProcess();` but we wrap it in an `Owned` and pass it to 
> > `fetcher`?

By "violate" I mean that the tests will use and modify the `fetcherProcess` 
even though it is owned elsewhere.  I've gotten around this pattern in the past 
by instantiating the mock (and the expectations) before passing ownership 
along.  But that doesn't quite work here.

i.e. You'll see things like this in the tests:
```
  EXPECT_CALL(*fetcherProcess, _fetch(_, _, _, _, _, _))
    .WillRepeatedly(
        DoAll(SatisfyOne(&fetchContentionWaypoints),
              Invoke(fetcherProcess, &MockFetcherProcess::unmocked__fetch)));
```

This could be considered a TODO for future cleanup.


- Joseph


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


On March 15, 2016, 1:48 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43629/
> -----------------------------------------------------------
> 
> (Updated March 15, 2016, 1:48 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
>     https://issues.apache.org/jira/browse/MESOS-4633
>     https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Continuation of https://reviews.apache.org/r/43615/ with a slightly different 
> pattern.
> 
> 
> Diffs
> -----
> 
>   src/tests/dynamic_weights_tests.cpp 
> a95663f633f376954d4201b6cfbe693429be9c39 
>   src/tests/fetcher_cache_tests.cpp 776c95267caff7b27cd70c2fa6149d8158c86750 
>   src/tests/resource_offers_tests.cpp 
> 0bad45dd1dabecc88fef1ab46e8ea26718070b33 
>   src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
> 
> Diff: https://reviews.apache.org/r/43629/diff/
> 
> 
> Testing
> -------
> 
> Tests are run at the end of this review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Reply via email to