mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Sekretenko <asekrete...@mesosphere.io>
Subject Re: Review Request 70534: Added tests for the V1 UPDATE_FRAMEWORK call.
Date Fri, 24 May 2019 19:33:04 GMT


> On May 24, 2019, 3:59 p.m., Benjamin Mahler wrote:
> > src/tests/master/update_framework_tests.cpp
> > Lines 434-438 (patched)
> > <https://reviews.apache.org/r/70534/diff/7/?file=2146074#file2146074line434>
> >
> >     This looks racy?
> >     
> >     The expectation should be set up prior to the agent starting, but do we even
care to test that there's an event for it in this test? It seems to be the responsibility
of other tests to care about agent lifecycle events.

Indeed, this is misplaced and has at least two races: a high-level one (setting expectation
vs the actual event) and a low-level one (calling EXPECT_CALL vs calling the mock method).
I'll fix this and take care of similar problems elsewhere in these patches.

The intention behind this AWAIT was to keep a fixed ordering between `allocator->addSlave()`
and `allocator->updateFramework()`. 
I do not think it is a good idea to have a test in which these two calls occur in some arbitrary
(not random, but arbitrary!) order.
Currently only the case "update after a slave is added" is tested. If we need to test a second
case, it probably makes sense to have a second test.

I'm also not sure if waiting for AGENT_ADDED is the best way to ensure the ordering (waiting
for dispatch on `MesosAllocatorProcess::addSlave` could probably be another option).

Even if we settle on waiting for AGENT_ADDED, I'll have to write a better comment.


- Andrei


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


On May 21, 2019, 2:10 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70534/
> -----------------------------------------------------------
> 
> (Updated May 21, 2019, 2:10 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7258
>     https://issues.apache.org/jira/browse/MESOS-7258
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added tests for the V1 UPDATE_FRAMEWORK call.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 5f97523fbe2d80733fbdcc7706f2761f5a071f9f 
>   src/tests/CMakeLists.txt e6b1d8a097246f0921467d7f778034d6754fcff6 
>   src/tests/master/update_framework_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70534/diff/7/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


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