----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41806/#review114086 -----------------------------------------------------------
Thanks for cleaning this up. I've got a question about the overhead of the extra AWAITs, and a few nits. Resolve those and we'll commit it. src/tests/executor_http_api_tests.cpp (line 394) <https://reviews.apache.org/r/41806/#comment174874> Do you think there's any test overhead in doing another AWAIT for a `response` that has already been awaited? I'd think it would be a near instantaneous noop, but if not, the EXPECT_SOME_EQ could be faster. Maybe you could time a few test runs before/after this patch to make sure we're not noticeably slowing things down. src/tests/executor_http_api_tests.cpp (lines 775 - 776) <https://reviews.apache.org/r/41806/#comment174877> If it wraps to another line, then each parameter should get its own line. First parameter can stay on the original line and align other params with it, if that doesn't cause too much "jaggedness". Or you could create a variable for `stringify(contentType)` so that it all fits on one line, since it's used above on line 771 too. src/tests/master_tests.cpp (line 282) <https://reviews.apache.org/r/41806/#comment174879> Why not `using process::http::Response;`? src/tests/master_tests.cpp (line 285) <https://reviews.apache.org/r/41806/#comment174878> Why aren't we `using process::http::OK;` or at least `using process::http`? Is there some other OK conflicting with it? Doesn't look like it. src/tests/repair_tests.cpp (lines 84 - 87) <https://reviews.apache.org/r/41806/#comment174882> Not yours, but could you fix the indentation please? - Adam B On Jan. 12, 2016, 12:47 p.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41806/ > ----------------------------------------------------------- > > (Updated Jan. 12, 2016, 12:47 p.m.) > > > Review request for mesos and Adam B. > > > Repository: mesos > > > Description > ------- > > Used `AWAIT_EXPECT_RESPONSE_HEADER_EQ()` to check the "Content-Type" of > the response, rather than accessing the "headers" field directly, and > used the symbol `APPLICATION_JSON` rather than a string literal. Also > added "Content-Type" checks to a few places that had neglected to make > them, and cleaned up some whitespace style. > > > Diffs > ----- > > src/tests/executor_http_api_tests.cpp > 952aacb720715ad26f91fa08bc386b242b7e007b > src/tests/fault_tolerance_tests.cpp > fd1c3c90101eed9ef9352511e0c72a463cca965c > src/tests/master_tests.cpp 223b9d20a3a8a8194a3a6a605ec2394c37ab5957 > src/tests/metrics_tests.cpp f081fb9b68f25c6d6005f195c34253fba8abc146 > src/tests/monitor_tests.cpp a848d14ebb6cab79c06bcf55bd39f044b41a006e > src/tests/repair_tests.cpp 63ec889c4954c2c60d3466952551aa25b3284ddf > src/tests/scheduler_driver_tests.cpp > 1365d21ccad87923b862fb4942f1fd51630a62b7 > src/tests/scheduler_http_api_tests.cpp > 4d23a5a8368e0ed126469fa4a90a889b339ad004 > src/tests/slave_tests.cpp e4fb490a1d877547fe883c22dbc47bb4969ecef6 > src/tests/status_update_manager_tests.cpp > bd34b97a3559a5fea9a7a253a89e0ac3029f4a33 > src/tests/utils.cpp 877139e97249761658dce3b1058cdc2e2a52367b > > Diff: https://reviews.apache.org/r/41806/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Neil Conway > >
