mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 42362: Added persistent volume endpoint test without authentication.
Date Thu, 28 Jan 2016 17:56:00 GMT


> On Jan. 28, 2016, 5:49 p.m., Alexander Rukletsov wrote:
> > src/tests/persistent_volume_endpoints_tests.cpp, lines 1118-1119
> > <https://reviews.apache.org/r/42362/diff/5-6/?file=1224265#file1224265line1118>
> >
> >     I advocate the practice when each block wrapped in curly braces is prepended
with a comment. Effectively, each such block is a test case inside a test, so since we strive
to provide a comment for each test we should do the same for each block. I think you had great
comments that you could reused here. The most important bit is to drag people's attention
to the absence of credentials. Does it make sense?

Yep, it makes sense. Anand advocated removing the bit about not including authentication headers
because it was self-explanatory, but I do think that it's better to call it out explicitly,
since if you don't know the function signature of `createBasicAuthHeaders`, it's not immediately
obvious. Thanks, will update.


- Greg


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


On Jan. 28, 2016, 4:43 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2016, 4:43 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
>     https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction when HTTP
authentication is disabled; this patch introduces a test for this scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -----
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 22e18758ee91a649486725473d9e50fae9d43b01

> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> -------
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to the persistent
volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


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