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


Looking. Minor issues. Please make sure when you fix an issue, you fix it in 
all the tests.


src/tests/http_api_tests.cpp (lines 72 - 93)
<https://reviews.apache.org/r/37082/#comment149653>

    Can you add a TODO to move these out into common helpers?



src/tests/http_api_tests.cpp (lines 141 - 143)
<https://reviews.apache.org/r/37082/#comment149655>

    Instead of repeating this comment and these 2 lines in every test, add a 
CreaterMasterFlags() overload to the fixture.



src/tests/http_api_tests.cpp (line 178)
<https://reviews.apache.org/r/37082/#comment149654>

    do you need the temp variable here?
    
    here and subsequent tests.



src/tests/http_api_tests.cpp (lines 199 - 200)
<https://reviews.apache.org/r/37082/#comment149667>

    ditto.



src/tests/http_api_tests.cpp (line 221)
<https://reviews.apache.org/r/37082/#comment149656>

    s/subcribedId/frameworkId/



src/tests/http_api_tests.cpp (line 243)
<https://reviews.apache.org/r/37082/#comment149657>

    kill this. ASSERT_SOME() guarantees that it is not an error.



src/tests/http_api_tests.cpp (line 245)
<https://reviews.apache.org/r/37082/#comment149662>

    ditto.



src/tests/http_api_tests.cpp (lines 248 - 249)
<https://reviews.apache.org/r/37082/#comment149658>

    swap the order of the arguments.
    
    when writing ASSERT, EXPECT the convention is that the first argument 
should be the "expected" value and the second one is the "actual" value. this 
is because of the way gmock prints the error message.
    
    please fix this here and everywhere else in this file.



src/tests/http_api_tests.cpp (line 275)
<https://reviews.apache.org/r/37082/#comment149660>

    kill this.



src/tests/http_api_tests.cpp (lines 279 - 280)
<https://reviews.apache.org/r/37082/#comment149661>

    ditto.



src/tests/http_api_tests.cpp (line 287)
<https://reviews.apache.org/r/37082/#comment149664>

    s/pid/PID/



src/tests/http_api_tests.cpp (line 288)
<https://reviews.apache.org/r/37082/#comment149666>

    s/http/HTTP/



src/tests/http_api_tests.cpp (lines 292 - 293)
<https://reviews.apache.org/r/37082/#comment149668>

    ditto.



src/tests/http_api_tests.cpp (line 310)
<https://reviews.apache.org/r/37082/#comment149669>

    s/http/HTTP/



src/tests/http_api_tests.cpp (line 322)
<https://reviews.apache.org/r/37082/#comment149670>

    s/a/an/



src/tests/http_api_tests.cpp (line 357)
<https://reviews.apache.org/r/37082/#comment149671>

    ditto.



src/tests/http_api_tests.cpp (lines 366 - 367)
<https://reviews.apache.org/r/37082/#comment149672>

    ditto.



src/tests/http_api_tests.cpp (line 406)
<https://reviews.apache.org/r/37082/#comment149673>

    s/'force'/the 'force'/


- Vinod Kone


On Aug. 11, 2015, 5:03 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37082/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2015, 5:03 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This implements the tests for http framework subscribe/failover/upgrade from 
> a pid based framework. The test are parameterized on content type and hence 
> test both json/protobuf responses.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 10cc100b1cf10aa7a8a9979a2562a9c5a9e55bd6 
>   src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 
> 
> Diff: https://reviews.apache.org/r/37082/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Reply via email to