You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2016/03/23 23:13:14 UTC
Review Request 45249: Added new '/files' endpoints tests using
authentication.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45249/
-----------------------------------------------------------
Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.
Bugs: MESOS-4956
https://issues.apache.org/jira/browse/MESOS-4956
Repository: mesos
Description
-------
New tests were added to the FilesTests, and existing tests were modified, to probe the endpoints' behavior with and without HTTP authentication. Tests were also added for `/flags/debug`, as there were no pre-existing tests for that endpoint.
Diffs
-----
src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e
Diff: https://reviews.apache.org/r/45249/diff/
Testing
-------
`sudo make check` was used to test on OSX and CentOS 7.
Thanks,
Greg Mann
Re: Review Request 45249: Added a new '/files' endpoints test using
authentication.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45249/#review125611
-----------------------------------------------------------
Oops. Found a bug!
src/tests/files_tests.cpp (line 81)
<https://reviews.apache.org/r/45249/#comment188483>
Don't you need to call `TemporaryDirectoryTest::TearDown();` at the end of this?
- Adam B
On March 25, 2016, 5:07 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45249/
> -----------------------------------------------------------
>
> (Updated March 25, 2016, 5:07 p.m.)
>
>
> Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.
>
>
> Bugs: MESOS-4956
> https://issues.apache.org/jira/browse/MESOS-4956
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A new test was added (FilesTest.AuthenticationTest) to probe the `/files/*` endpoints' behavior when HTTP authentication is enabled.
>
>
> Diffs
> -----
>
> src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e
>
> Diff: https://reviews.apache.org/r/45249/diff/
>
>
> Testing
> -------
>
> `sudo make check` and `GTEST_FILTER="FilesTest.AuthenticationTest" build/bin/mesos-tests.sh --gtest_repeat=1000 --gtest_break_on_failure=1` were used to test on OSX and CentOS 7.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 45249: Added a new '/files' endpoints test using
authentication.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45249/#review125606
-----------------------------------------------------------
Fix it, then Ship it!
Thanks! I'll move the namespace= and indent those EXPECT params, then commit it
src/tests/files_tests.cpp (line 51)
<https://reviews.apache.org/r/45249/#comment188478>
namespace= declarations always seem to go above the using's, right after includes.
src/tests/files_tests.cpp (line 357)
<https://reviews.apache.org/r/45249/#comment188479>
Not sure why this needed to be its own function if it's only used once. Maybe you anticipate more tests using it in the future?
src/tests/files_tests.cpp (line 375)
<https://reviews.apache.org/r/45249/#comment188482>
Let's align this with `response`, since that won't create too much jaggedness. Otherwise, we'd prefer each parameter get its own line.
- Adam B
On March 25, 2016, 5:07 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45249/
> -----------------------------------------------------------
>
> (Updated March 25, 2016, 5:07 p.m.)
>
>
> Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.
>
>
> Bugs: MESOS-4956
> https://issues.apache.org/jira/browse/MESOS-4956
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A new test was added (FilesTest.AuthenticationTest) to probe the `/files/*` endpoints' behavior when HTTP authentication is enabled.
>
>
> Diffs
> -----
>
> src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e
>
> Diff: https://reviews.apache.org/r/45249/diff/
>
>
> Testing
> -------
>
> `sudo make check` and `GTEST_FILTER="FilesTest.AuthenticationTest" build/bin/mesos-tests.sh --gtest_repeat=1000 --gtest_break_on_failure=1` were used to test on OSX and CentOS 7.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 45249: Added a new '/files' endpoints test using
authentication.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45249/
-----------------------------------------------------------
(Updated March 26, 2016, 12:07 a.m.)
Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.
Changes
-------
Removed unnecessary blank line.
Bugs: MESOS-4956
https://issues.apache.org/jira/browse/MESOS-4956
Repository: mesos
Description
-------
A new test was added (FilesTest.AuthenticationTest) to probe the `/files/*` endpoints' behavior when HTTP authentication is enabled.
Diffs (updated)
-----
src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e
Diff: https://reviews.apache.org/r/45249/diff/
Testing
-------
`sudo make check` and `GTEST_FILTER="FilesTest.AuthenticationTest" build/bin/mesos-tests.sh --gtest_repeat=1000 --gtest_break_on_failure=1` were used to test on OSX and CentOS 7.
Thanks,
Greg Mann
Re: Review Request 45249: Added a new '/files' endpoints test using
authentication.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45249/
-----------------------------------------------------------
(Updated March 25, 2016, 4:42 p.m.)
Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.
Bugs: MESOS-4956
https://issues.apache.org/jira/browse/MESOS-4956
Repository: mesos
Description (updated)
-------
A new test was added (FilesTest.AuthenticationTest) to probe the `/files/*` endpoints' behavior when HTTP authentication is enabled.
Diffs (updated)
-----
src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e
Diff: https://reviews.apache.org/r/45249/diff/
Testing (updated)
-------
`sudo make check` and `GTEST_FILTER="FilesTest.AuthenticationTest" build/bin/mesos-tests.sh --gtest_repeat=1000 --gtest_break_on_failure=1` were used to test on OSX and CentOS 7.
Thanks,
Greg Mann
Re: Review Request 45249: Added a new '/files' endpoints test using
authentication.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45249/
-----------------------------------------------------------
(Updated March 24, 2016, 11:59 p.m.)
Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.
Changes
-------
Removed the RAII wrapper :-) Manage the authenticator lifecycle in the test fixture setup/teardown.
Summary (updated)
-----------------
Added a new '/files' endpoints test using authentication.
Bugs: MESOS-4956
https://issues.apache.org/jira/browse/MESOS-4956
Repository: mesos
Description (updated)
-------
A new test was added (FilesTest.AuthenticationTest) to the FilesTests, and existing tests were modified, to probe the endpoints' behavior with and without HTTP authentication.
Diffs (updated)
-----
src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e
Diff: https://reviews.apache.org/r/45249/diff/
Testing
-------
`sudo make check` was used to test on OSX and CentOS 7.
Thanks,
Greg Mann
Re: Review Request 45249: Added new '/files' endpoints tests using
authentication.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45249/
-----------------------------------------------------------
(Updated March 24, 2016, 8:33 p.m.)
Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.
Changes
-------
Put the HTTP authenticator in an RAII wrapper.
Bugs: MESOS-4956
https://issues.apache.org/jira/browse/MESOS-4956
Repository: mesos
Description
-------
New tests were added to the FilesTests, and existing tests were modified, to probe the endpoints' behavior with and without HTTP authentication. Tests were also added for `/flags/debug`, as there were no pre-existing tests for that endpoint.
Diffs (updated)
-----
src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e
Diff: https://reviews.apache.org/r/45249/diff/
Testing
-------
`sudo make check` was used to test on OSX and CentOS 7.
Thanks,
Greg Mann
Re: Review Request 45249: Added new '/files' endpoints tests using
authentication.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45249/#review125209
-----------------------------------------------------------
Patch looks great!
Reviews applied: [44678, 44703, 44515, 44523, 44553, 44554, 45088, 45248, 45249]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On March 24, 2016, 1:05 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45249/
> -----------------------------------------------------------
>
> (Updated March 24, 2016, 1:05 a.m.)
>
>
> Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.
>
>
> Bugs: MESOS-4956
> https://issues.apache.org/jira/browse/MESOS-4956
>
>
> Repository: mesos
>
>
> Description
> -------
>
> New tests were added to the FilesTests, and existing tests were modified, to probe the endpoints' behavior with and without HTTP authentication. Tests were also added for `/flags/debug`, as there were no pre-existing tests for that endpoint.
>
>
> Diffs
> -----
>
> src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e
>
> Diff: https://reviews.apache.org/r/45249/diff/
>
>
> Testing
> -------
>
> `sudo make check` was used to test on OSX and CentOS 7.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 45249: Added new '/files' endpoints tests using
authentication.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45249/
-----------------------------------------------------------
(Updated March 24, 2016, 1:05 a.m.)
Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.
Changes
-------
Removed unnecessary tests.
Bugs: MESOS-4956
https://issues.apache.org/jira/browse/MESOS-4956
Repository: mesos
Description
-------
New tests were added to the FilesTests, and existing tests were modified, to probe the endpoints' behavior with and without HTTP authentication. Tests were also added for `/flags/debug`, as there were no pre-existing tests for that endpoint.
Diffs (updated)
-----
src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e
Diff: https://reviews.apache.org/r/45249/diff/
Testing
-------
`sudo make check` was used to test on OSX and CentOS 7.
Thanks,
Greg Mann