mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 51715: Added a parallel gtest runner.
Date Sat, 01 Oct 2016 19:06:55 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message