You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrei Sekretenko <as...@mesosphere.io> on 2019/05/17 15:19:21 UTC

Re: Review Request 70534: Added tests for the V1 UPDATE_FRAMEWORK call.

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

(Updated May 17, 2019, 3:19 p.m.)


Review request for mesos and Benjamin Mahler.


Summary (updated)
-----------------

Added tests for the V1 UPDATE_FRAMEWORK call.


Bugs: MESOS-7258
    https://issues.apache.org/jira/browse/MESOS-7258


Repository: mesos


Description (updated)
-------

Added tests for the V1 UPDATE_FRAMEWORK call.


Diffs (updated)
-----

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

Changes: https://reviews.apache.org/r/70534/diff/4-5/


Testing
-------


Thanks,

Andrei Sekretenko


Re: Review Request 70534: Added tests for the V1 UPDATE_FRAMEWORK call.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70534/#review215335
-----------------------------------------------------------



Patch looks great!

Reviews applied: [70532, 70663, 70664, 70665, 70666, 70667, 70668, 70669, 70670, 70533, 70671, 70534]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 17, 2019, 8:19 a.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70534/
> -----------------------------------------------------------
> 
> (Updated May 17, 2019, 8:19 a.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/5/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


Re: Review Request 70534: Added tests for the V1 UPDATE_FRAMEWORK call.

Posted by Andrei Sekretenko <as...@mesosphere.io>.

> On May 24, 2019, 3:59 p.m., Benjamin Mahler wrote:
> > src/tests/master/update_framework_tests.cpp
> > Lines 417 (patched)
> > <https://reviews.apache.org/r/70534/diff/7/?file=2146074#file2146074line417>
> >
> >     Can you explain what this is testing?

Split this test into two, tried to come up with better names for them and added a comment to each - I hope now it is more readable.


> On May 24, 2019, 3:59 p.m., Benjamin Mahler wrote:
> > src/tests/master/update_framework_tests.cpp
> > Lines 429-430 (patched)
> > <https://reviews.apache.org/r/70534/diff/7/?file=2146074#file2146074line429>
> >
> >     Why is this customization needed?

Not needed at all - removed it. Also tried to remove other remaining customization wherever possible.


- Andrei


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


On May 28, 2019, 5:36 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70534/
> -----------------------------------------------------------
> 
> (Updated May 28, 2019, 5:36 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/8/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


Re: Review Request 70534: Added tests for the V1 UPDATE_FRAMEWORK call.

Posted by Andrei Sekretenko <as...@mesosphere.io>.

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


Re: Review Request 70534: Added tests for the V1 UPDATE_FRAMEWORK call.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70534/#review215500
-----------------------------------------------------------



Overall looking pretty good, thanks! Can you be sure to run the test in repetition to ensure there's no flakyness?


src/tests/master/update_framework_tests.cpp
Lines 88 (patched)
<https://reviews.apache.org/r/70534/#comment302217>

    {}; can go on the previous line



src/tests/master/update_framework_tests.cpp
Lines 93 (patched)
<https://reviews.apache.org/r/70534/#comment302213>

    It seems strange to pass the `id` here.. can't we use the one in info and ensure that callers set it?



src/tests/master/update_framework_tests.cpp
Lines 98-103 (patched)
<https://reviews.apache.org/r/70534/#comment302212>

    In general I would suggest we prefer the following syntax pattern:
    
    ```
     *call.mutable_framework_id() = id;
     *call.mutable_update_framework()->mutable_framework_info() = info;
     *call.mutable_update_framework()->mutable_framework_info()
       ->mutable_id() = id;
    ```
    
    Because it allows moves to happen when the rhs is an rvalue:
    
    ```
    *msg.mutable_foo() = func(); // will move :)
    msg.mutable_foo()->CopyFrom(func()); // will copy :(
    ```



src/tests/master/update_framework_tests.cpp
Lines 124 (patched)
<https://reviews.apache.org/r/70534/#comment302214>

    pull this up into the previous line?



src/tests/master/update_framework_tests.cpp
Lines 131 (patched)
<https://reviews.apache.org/r/70534/#comment302215>

    newline



src/tests/master/update_framework_tests.cpp
Lines 134 (patched)
<https://reviews.apache.org/r/70534/#comment302216>

    newline



src/tests/master/update_framework_tests.cpp
Lines 145 (patched)
<https://reviews.apache.org/r/70534/#comment302218>

    s/C/c/



src/tests/master/update_framework_tests.cpp
Lines 147-149 (patched)
<https://reviews.apache.org/r/70534/#comment302211>

    How about a CHECK_EQ with a helpful `<<` message so that the reader knows why this is failing from the logs without having to come in here and read the code?



src/tests/master/update_framework_tests.cpp
Lines 176 (patched)
<https://reviews.apache.org/r/70534/#comment302219>

    s/FrameworkInfosDiff/diff/?



src/tests/master/update_framework_tests.cpp
Lines 182 (patched)
<https://reviews.apache.org/r/70534/#comment302220>

    This seems odd, can't the caller fill this in for the initial FrameworkInfo so that we don't ignore it? if the `id` is set to two different values, we want to catch that.



src/tests/master/update_framework_tests.cpp
Lines 197-198 (patched)
<https://reviews.apache.org/r/70534/#comment302221>

    Can you declare this lower down closer to where it's used? We can also pass in the DEFAULT_FRAMEWORK_INFO **with** `id` assigned based on the subscription.
    
    Ditto for the other tests



src/tests/master/update_framework_tests.cpp
Lines 219 (patched)
<https://reviews.apache.org/r/70534/#comment302222>

    newline, ditto for the other tests



src/tests/master/update_framework_tests.cpp
Lines 230 (patched)
<https://reviews.apache.org/r/70534/#comment302236>

    newline, ditto other tests



src/tests/master/update_framework_tests.cpp
Lines 325 (patched)
<https://reviews.apache.org/r/70534/#comment302237>

    A role not in the whitelist? Don't we have a test for that somewhere already? Or are you thinking specifically when an UPDATE_FRAMEWORK uses a non-whitelisted role?



src/tests/master/update_framework_tests.cpp
Lines 326 (patched)
<https://reviews.apache.org/r/70534/#comment302238>

    Ditto here, I guess we test this already somewhere (maybe) but this is just to make sure the UPDATE_FRAMEWORK path goes throug that check?



src/tests/master/update_framework_tests.cpp
Lines 332 (patched)
<https://reviews.apache.org/r/70534/#comment302239>

    exit code?



src/tests/master/update_framework_tests.cpp
Lines 391 (patched)
<https://reviews.apache.org/r/70534/#comment302234>

    remove this?



src/tests/master/update_framework_tests.cpp
Lines 397 (patched)
<https://reviews.apache.org/r/70534/#comment302235>

    newline



src/tests/master/update_framework_tests.cpp
Lines 417 (patched)
<https://reviews.apache.org/r/70534/#comment302224>

    Can you explain what this is testing?



src/tests/master/update_framework_tests.cpp
Lines 421 (patched)
<https://reviews.apache.org/r/70534/#comment302223>

    newline



src/tests/master/update_framework_tests.cpp
Lines 424-425 (patched)
<https://reviews.apache.org/r/70534/#comment302228>

    See my comment below, it seems we don't need this?



src/tests/master/update_framework_tests.cpp
Lines 429-430 (patched)
<https://reviews.apache.org/r/70534/#comment302226>

    Why is this customization needed?



src/tests/master/update_framework_tests.cpp
Lines 434-438 (patched)
<https://reviews.apache.org/r/70534/#comment302227>

    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.



src/tests/master/update_framework_tests.cpp
Lines 454 (patched)
<https://reviews.apache.org/r/70534/#comment302229>

    For consistency, we put the .WillOnce, .Times, etc on the next line, can you do a sweep in this file?



src/tests/master/update_framework_tests.cpp
Lines 456 (patched)
<https://reviews.apache.org/r/70534/#comment302230>

    I hadn't considered the case where a framework has no roles, I suppose we can allow that..



src/tests/master/update_framework_tests.cpp
Lines 469 (patched)
<https://reviews.apache.org/r/70534/#comment302232>

    newline



src/tests/master/update_framework_tests.cpp
Lines 472 (patched)
<https://reviews.apache.org/r/70534/#comment302231>

    s/.get()./->/ (please do a scan of this file)
    
    Seems like 1 offer is expected given the single rescind expectation below, might as well ensure it's 1 here?



src/tests/master/update_framework_tests.cpp
Lines 499 (patched)
<https://reviews.apache.org/r/70534/#comment302233>

    Pass `masterFlags.allocation_interval` instead


- Benjamin Mahler


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


Re: Review Request 70534: Added tests for the V1 UPDATE_FRAMEWORK call.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70534/#review215460
-----------------------------------------------------------



Patch looks great!

Reviews applied: [70532, 70663, 70664, 70665, 70668, 70669, 70670, 70533, 70671, 70534]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


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


Re: Review Request 70534: Added tests for the V1 UPDATE_FRAMEWORK call.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70534/#review215411
-----------------------------------------------------------



Patch looks great!

Reviews applied: [70532, 70663, 70664, 70665, 70668, 70669, 70670, 70533, 70671, 70534]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


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


Re: Review Request 70534: Added tests for the V1 UPDATE_FRAMEWORK call.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70534/#review215774
-----------------------------------------------------------


Fix it, then Ship it!




Nice tests, thank you Andrei!


src/tests/master/update_framework_tests.cpp
Lines 150 (patched)
<https://reviews.apache.org/r/70534/#comment302617>

    s/update/newInfo/



src/tests/master/update_framework_tests.cpp
Lines 150 (patched)
<https://reviews.apache.org/r/70534/#comment302618>

    how about: s/update/newInfo/



src/tests/master/update_framework_tests.cpp
Lines 157-158 (patched)
<https://reviews.apache.org/r/70534/#comment302614>

    This seems a little brittle, as we probably should be failing FrameworkInfo validation when there is an UNKNOWN capability. Seems like a bug that it's not getting checked
    
    How about REGION_AWARE?



src/tests/master/update_framework_tests.cpp
Lines 164-169 (patched)
<https://reviews.apache.org/r/70534/#comment302615>

    Remove "without_resources"? I'm not sure what it's trying to communicate, but it seems to carry too much context from somewhere else, is it named this way because of a test?



src/tests/master/update_framework_tests.cpp
Lines 170 (patched)
<https://reviews.apache.org/r/70534/#comment302619>

    newline



src/tests/master/update_framework_tests.cpp
Lines 174 (patched)
<https://reviews.apache.org/r/70534/#comment302620>

    s/std::string/string/ here and below



src/tests/master/update_framework_tests.cpp
Lines 274 (patched)
<https://reviews.apache.org/r/70534/#comment302621>

    As an aside, the error above says "Updating" and this one says "Changing", should probably make the language consistent



src/tests/master/update_framework_tests.cpp
Lines 370-375 (patched)
<https://reviews.apache.org/r/70534/#comment302622>

    This looks racy..?
    
    The agent may get added before the subscriber subscribes, in which case the AGENT_ADDED event wouldn't be sent because the agent gets included in the initial SUBSCRIBE state.



src/tests/master/update_framework_tests.cpp
Lines 493 (patched)
<https://reviews.apache.org/r/70534/#comment302624>

    Why resume and re-pause the clock?



src/tests/master/update_framework_tests.cpp
Lines 512 (patched)
<https://reviews.apache.org/r/70534/#comment302625>

    Why resume?



src/tests/master/update_framework_tests.cpp
Lines 562-565 (patched)
<https://reviews.apache.org/r/70534/#comment302631>

    ```
      Future<Event::Rescind> rescind;
      EXPECT_CALL(*scheduler, rescind(_, _))
        .WillOnce(FutureArg<1>(&rescind));
    ```



src/tests/master/update_framework_tests.cpp
Lines 569-570 (patched)
<https://reviews.apache.org/r/70534/#comment302629>

    ```
      // TODO(asekretenko): Add a more in-depth check that the
      // allocator does what it should.
    ```


- Benjamin Mahler


On June 3, 2019, 8:14 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70534/
> -----------------------------------------------------------
> 
> (Updated June 3, 2019, 8:14 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/9/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


Re: Review Request 70534: Added tests for the V1 UPDATE_FRAMEWORK call.

Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70534/
-----------------------------------------------------------

(Updated June 3, 2019, 8:14 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 (updated)
-----

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

Changes: https://reviews.apache.org/r/70534/diff/8-9/


Testing
-------


Thanks,

Andrei Sekretenko


Re: Review Request 70534: Added tests for the V1 UPDATE_FRAMEWORK call.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70534/#review215592
-----------------------------------------------------------



Patch looks great!

Reviews applied: [70532, 70663, 70664, 70665, 70668, 70669, 70670, 70533, 70671, 70534]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 28, 2019, 5:36 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70534/
> -----------------------------------------------------------
> 
> (Updated May 28, 2019, 5:36 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/8/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


Re: Review Request 70534: Added tests for the V1 UPDATE_FRAMEWORK call.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70534/#review215553
-----------------------------------------------------------



Patch looks great!

Reviews applied: [70532, 70663, 70664, 70665, 70668, 70669, 70670, 70533, 70671, 70534]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 28, 2019, 5:36 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70534/
> -----------------------------------------------------------
> 
> (Updated May 28, 2019, 5:36 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/8/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


Re: Review Request 70534: Added tests for the V1 UPDATE_FRAMEWORK call.

Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70534/
-----------------------------------------------------------

(Updated May 28, 2019, 5:36 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 (updated)
-----

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

Changes: https://reviews.apache.org/r/70534/diff/7-8/


Testing
-------


Thanks,

Andrei Sekretenko


Re: Review Request 70534: Added tests for the V1 UPDATE_FRAMEWORK call.

Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
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 (updated)
-----

  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/

Changes: https://reviews.apache.org/r/70534/diff/6-7/


Testing
-------


Thanks,

Andrei Sekretenko


Re: Review Request 70534: Added tests for the V1 UPDATE_FRAMEWORK call.

Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70534/
-----------------------------------------------------------

(Updated May 21, 2019, 1:56 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 (updated)
-----

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

Changes: https://reviews.apache.org/r/70534/diff/5-6/


Testing
-------


Thanks,

Andrei Sekretenko