> On Dec. 9, 2015, 12:19 a.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/tests/http_tests.cpp, lines 1339-1343
> > <https://reviews.apache.org/r/38000/diff/21/?file=1155956#file1155956line1339>
> >
> >     Hm.. it looks like you're using the principals here to check that the 
> > requests arrive to the endpoint in the right order?
> >     
> >     How about avoiding the captures here and just specifying "principal1" 
> > and "principal2" in the expectation?
> >     
> >     Also, you're returning "1" and "2" but that's not checked at all? Did 
> > you mean to check that the responses below are ordered correctly?

It is not enough to just set the principal in the expectation, since the 
expectation only says that the function will be called with the given 
parameters, but mentions nothing about the ordering.


> On Dec. 9, 2015, 12:19 a.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/tests/http_tests.cpp, lines 1373-1375
> > <https://reviews.apache.org/r/38000/diff/21/?file=1155956#file1155956line1373>
> >
> >     It's not clear why the sleep is here. If I had to guess, it looks like 
> > you wanted to ensure that the both authentication calls were made at this 
> > point? If so, we don't need a sleep for this.
> >     
> >     Let's try to avoid the sleep here if possible.

When pipelining is not enforced, without the sleep the test is just flaky, 
since often the messages will be executed in the right order. I can remove it, 
but as I mentioned, the test passes from failing everytime to only sometimes.


- Alexander


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


On Dec. 8, 2015, 3:38 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38000/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2015, 3:38 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3233
>     https://issues.apache.org/jira/browse/MESOS-3233
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Adds functions which allow libprocess users to register HTTP authenticators.
> Overloads `ProcesBase::route()` to allow for registering of authenticating 
> endpoints.
> Includes tests.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/event.hpp 
> a03824c061c4a0eb865b163999a763635e56744c 
>   3rdparty/libprocess/include/process/process.hpp 
> 81c094414d4d5ac5eb593df2a6d14aaacb19a826 
>   3rdparty/libprocess/src/authentication_router.hpp 
> 5777deafd7420324627802a7ab9da9aaa2b46825 
>   3rdparty/libprocess/src/process.cpp 
> e93709d3bb4ac588457bb9331fc05ec5ab539f6d 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38000/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Reply via email to