mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 43629: Especially updated tests to use the updated MesosTest helpers.
Date Wed, 16 Mar 2016 17:17:14 GMT


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


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