Re: Review Request 35697: mesos: Fixed test names.
> On June 22, 2015, 8:12 p.m., Ben Mahler wrote: > > Thanks, will commit this for you. > > > > Also we are inconcistent on the "Test" suffix, e.g. "MasterTest" vs > > "CRAMMD5Authentication". See two options: > > > > (1) We can use "Test" to signify that there is a fixture, if that's > > helpful. So remove any "Test" suffixes from non-fixture tests. > > (2) We can use "Test" as a suffix for all tests. > > > > Thoughts? Ah, that's a good catch Ben! (1) I think whether the test is a fixture or not is already clear from the use of `TEST_F` as opposed to `TEST`. So I don't think it's useful to have the `Test` suffix to signify fixtures. (2) The fact that it's a test is obvious from `TEST`, `TEST_F`, so I don't think there's a benefit of having the `Test` suffix for all tests either. However, most of our tests have the `Test` suffix, and removing all of those would touch pretty much all of the tests. So I would vote for having the `Test` suffix for all tests. There are currently 136 instances of tests without the `Test` suffix. `grep --exclude=./src/tests/script.hpp --exclude=./src/webui/master/static/js/jquery-1.7.1.min.js --exclude-dir=./build -r "TEST.*(.*," . | grep -v "TEST.*(.*Test,"` - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35697/#review88840 --- On June 20, 2015, 7:14 p.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35697/ > --- > > (Updated June 20, 2015, 7:14 p.m.) > > > Review request for mesos, Benjamin Hindman and Till Toenshoff. > > > Repository: mesos > > > Description > --- > > First noticed `JsonTest.parse`, then found the other cases with the following > regex: `TEST.*(.*, [a-z].*`. > > > Diffs > - > > src/tests/cram_md5_authentication_tests.cpp > 992302365682df2c800c8828a21b9d38cb318472 > src/tests/credentials_tests.cpp ed672733ff895b1f252fa75d5653a9f3527d5b5f > src/tests/registrar_tests.cpp 97d0b685dd2744cf92b3a463d586e8bcf4c0af1a > > Diff: https://reviews.apache.org/r/35697/diff/ > > > Testing > --- > > `make check` > > > Thanks, > > Michael Park > >
Re: Review Request 35697: mesos: Fixed test names.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35697/#review88840 --- Ship it! Thanks, will commit this for you. Also we are inconcistent on the "Test" suffix, e.g. "MasterTest" vs "CRAMMD5Authentication". See two options: (1) We can use "Test" to signify that there is a fixture, if that's helpful. So remove any "Test" suffixes from non-fixture tests. (2) We can use "Test" as a suffix for all tests. Thoughts? - Ben Mahler On June 20, 2015, 7:14 p.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35697/ > --- > > (Updated June 20, 2015, 7:14 p.m.) > > > Review request for mesos, Benjamin Hindman and Till Toenshoff. > > > Repository: mesos > > > Description > --- > > First noticed `JsonTest.parse`, then found the other cases with the following > regex: `TEST.*(.*, [a-z].*`. > > > Diffs > - > > src/tests/cram_md5_authentication_tests.cpp > 992302365682df2c800c8828a21b9d38cb318472 > src/tests/credentials_tests.cpp ed672733ff895b1f252fa75d5653a9f3527d5b5f > src/tests/registrar_tests.cpp 97d0b685dd2744cf92b3a463d586e8bcf4c0af1a > > Diff: https://reviews.apache.org/r/35697/diff/ > > > Testing > --- > > `make check` > > > Thanks, > > Michael Park > >
Re: Review Request 35697: mesos: Fixed test names.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35697/#review88808 --- Ship it! Thanks a bunch for these. - Till Toenshoff On June 20, 2015, 7:14 p.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35697/ > --- > > (Updated June 20, 2015, 7:14 p.m.) > > > Review request for mesos, Benjamin Hindman and Till Toenshoff. > > > Repository: mesos > > > Description > --- > > First noticed `JsonTest.parse`, then found the other cases with the following > regex: `TEST.*(.*, [a-z].*`. > > > Diffs > - > > src/tests/cram_md5_authentication_tests.cpp > 992302365682df2c800c8828a21b9d38cb318472 > src/tests/credentials_tests.cpp ed672733ff895b1f252fa75d5653a9f3527d5b5f > src/tests/registrar_tests.cpp 97d0b685dd2744cf92b3a463d586e8bcf4c0af1a > > Diff: https://reviews.apache.org/r/35697/diff/ > > > Testing > --- > > `make check` > > > Thanks, > > Michael Park > >
Re: Review Request 35697: mesos: Fixed test names.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35697/#review88682 --- Patch looks great! Reviews applied: [35697] All tests passed. - Mesos ReviewBot On June 20, 2015, 7:14 p.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35697/ > --- > > (Updated June 20, 2015, 7:14 p.m.) > > > Review request for mesos, Benjamin Hindman and Till Toenshoff. > > > Repository: mesos > > > Description > --- > > First noticed `JsonTest.parse`, then found the other cases with the following > regex: `TEST.*(.*, [a-z].*`. > > > Diffs > - > > src/tests/cram_md5_authentication_tests.cpp > 992302365682df2c800c8828a21b9d38cb318472 > src/tests/credentials_tests.cpp ed672733ff895b1f252fa75d5653a9f3527d5b5f > src/tests/registrar_tests.cpp 97d0b685dd2744cf92b3a463d586e8bcf4c0af1a > > Diff: https://reviews.apache.org/r/35697/diff/ > > > Testing > --- > > `make check` > > > Thanks, > > Michael Park > >
