----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32463/#review77738 -----------------------------------------------------------
src/tests/master_tests.cpp <https://reviews.apache.org/r/32463/#comment125931> I noticed you do drive-by clean-up of some `process::` occurrences. Do you want to go further and remove all of them or prefer creating a newbie JIRA for that? There are e.g. `process::Message` and `Process::Time`. src/tests/master_tests.cpp <https://reviews.apache.org/r/32463/#comment125936> s/ensure/ensures Not a native speaker, but how about a one liner: `// This test ensures miscellaneous keys in state.json are present.` src/tests/master_tests.cpp <https://reviews.apache.org/r/32463/#comment125938> AFAIK, we agreed to omit a whitespace between closing angle brackets. src/tests/master_tests.cpp <https://reviews.apache.org/r/32463/#comment125944> Does it make sense to use `Clock::now().secs()` and compare doubles to avoid multiple conversions and mimic the way it is actually done in master code? src/tests/master_tests.cpp <https://reviews.apache.org/r/32463/#comment125940> Drop `process::` prefix? src/tests/master_tests.cpp <https://reviews.apache.org/r/32463/#comment125941> This fails on my Mac. Also, since you're checking start time, you can add one more `EXPECT_GT` check caching time before starting master. src/tests/master_tests.cpp <https://reviews.apache.org/r/32463/#comment125943> Why do you check the type here, but not above for e.g. `DATE`, `TIME`, and `USER`? src/tests/master_tests.cpp <https://reviews.apache.org/r/32463/#comment125961> How about a one-liner here and below: ``` EXPECT_TRUE(state.values["slaves"].as<JSON::Array>().values.empty()); ``` src/tests/slave_tests.cpp <https://reviews.apache.org/r/32463/#comment125932> Not yours, but can you do a drive-by fix and remove `process::`? src/tests/slave_tests.cpp <https://reviews.apache.org/r/32463/#comment125933> Kill `process::` prefix? - Alexander Rukletsov On March 24, 2015, 11:42 p.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32463/ > ----------------------------------------------------------- > > (Updated March 24, 2015, 11:42 p.m.) > > > Review request for mesos, Alexander Rukletsov and Vinod Kone. > > > Repository: mesos > > > Description > ------- > > This improves the slave test, and adds a new test for the master, to help > prevent API regressions. > > > Diffs > ----- > > src/tests/master_tests.cpp e69348be676a80017062e3abbd15b8008a6009d7 > src/tests/slave_tests.cpp fd09d65bf34136c0959419b451e54105300740c4 > > Diff: https://reviews.apache.org/r/32463/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Ben Mahler > >
