> On April 16, 2019, 1:06 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 3128 (patched)
> > <https://reviews.apache.org/r/69954/diff/3/?file=2138790#file2138790line3128>
> >
> >     Are we checking that the resources get returned to the offer pool? In 
> > that case let's confirm that `offers` looks like what we would expect.
> >     
> >     Alternatively we might not want to test this here and could just remove 
> > this and add a matching ignore when setting up the mock expectation above. 
> > That seems to make more sense to me.
> 
> Chun-Hung Hsiao wrote:
>     It's more about making it deterministic, and also as part of the 
> end-to-end scenario. The contents of the offer has already been checked by 
> the matcher.
>     
>     I could instead ignore the offer by doing a `Times(AtMost(1))`. Do you 
> think this would make the test simpler?

As discussed offline, the offer has been checked with the matcher set in the 
expectation.
Instead of using `AWAIT_READY`, we can use `AWAIT_EXPECT_READY` with logging 
here.


- Chun-Hung


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


On April 12, 2019, 3:15 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69954/
> -----------------------------------------------------------
> 
> (Updated April 12, 2019, 3:15 a.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-9565
>     https://issues.apache.org/jira/browse/MESOS-9565
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Test `CreateDestroyPersistentBlockVolume` verifies that SLRP would fail
> a `CREATE` operation on a BLOCK disk resource, and a followup `DESTROY`
> will be dropped (instead of failing the SLRP).
> 
> 
> Diffs
> -----
> 
>   src/tests/mock_csi_plugin.cpp cbcb59f720642c2801f5b86c438793e24437b414 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 8bf4d2331b59770a7b7f3768499047bec2d67677 
> 
> 
> Diff: https://reviews.apache.org/r/69954/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Reply via email to