> On Oct. 1, 2016, 7:43 p.m., Kevin Klues wrote: > > support/mesos-gtest-runner.py, lines 5-12 > > <https://reviews.apache.org/r/51715/diff/3/?file=1517705#file1517705line5> > > > > I would consider rewording this slightly, as it's not exactly clear to > > me what the difference between a GoogleTest suite and a GoogleTest test is. > > I'm also fine with leaving it as is though.
I renamed `suite` -> `programs`, so the lingo is now completely consistent with what GoogleTest uses in their docs (for posterity: https://github.com/google/googletest/blob/master/googletest/docs/Primer.md#basic-concepts). > On Oct. 1, 2016, 7:43 p.m., Kevin Klues wrote: > > support/mesos-gtest-runner.py, lines 211-229 > > <https://reviews.apache.org/r/51715/diff/3/?file=1517705#file1517705line211> > > > > I would reword the comment below as: > > > > ``` > > # Grab the number of failed tests and print the results from > > each test run. > > # > > # NOTE: The RESULTS array stores the results from each > > `run_test()` > > # invocation as a tuple (success, output). > > NFAILED = len([success for success, __ in RESULTS if not > > success]) > > > > for result in RESULTS: > > if not result[0]: > > if OPTIONS.verbosity > 0: > > print(result[1], file=sys.stderr) > > else: > > if OPTIONS.verbosity > 1: > > print(result[1], file=sys.stdout) > > > > if NFAILED > 0: > > print(Bcolors.colorize('\n[FAIL]', Bcolors.FAIL, > > Bcolors.BOLD), > > file=sys.stderr) > > else: > > print(Bcolors.colorize('\n[PASS]', Bcolors.OKGREEN, > > Bcolors.BOLD)) > > > > sys.exit(NFAILED) > > ``` Adopted the `NOTE`, but still `count` instead of `grab`. > On Oct. 1, 2016, 7:43 p.m., Kevin Klues wrote: > > support/mesos-gtest-runner.py, line 223 > > <https://reviews.apache.org/r/51715/diff/3/?file=1517705#file1517705line223> > > > > Can we be explicit about `NFAILED > 0`? I thing there's little value in distinguishing between all the possible `False` values here (e.g., `[]`, `None`, `0`), but I agree that this explicit check matches Mesos' C++ style more closely. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51715/#review151118 ----------------------------------------------------------- On Oct. 1, 2016, 9:06 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51715/ > ----------------------------------------------------------- > > (Updated Oct. 1, 2016, 9:06 p.m.) > > > Review request for mesos, Kevin Klues and Till Toenshoff. > > > Bugs: MESOS-6140 > https://issues.apache.org/jira/browse/MESOS-6140 > > > Repository: mesos > > > Description > ------- > > This adds `b814987b28cfb65a7c9635c83399545e423e690a` of > https://github.com/bbannier/gtest-shrdr. > > > Diffs > ----- > > support/mesos-gtest-runner.py PRE-CREATION > > Diff: https://reviews.apache.org/r/51715/diff/ > > > Testing > ------- > > Tested with e.g., `stout-tests` > > $ ./support/mesos-gtest-runner.py build/3rdparty/stout/stout-tests > ............ > [PASS] > > > Thanks, > > Benjamin Bannier > >
