> 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?
> 
> Greg Mann wrote:
>     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.

Ah sorry, my mistake. Looking at Anand's comment above, he was suggesting 
something similar :-)


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

Reply via email to