I personally don't need this, but if it makes your lives easier, that's
fine,
and I have no objections to adding this functionality.
However: tools/test.py is deprecated, along with
tools/test-wrapper-gypbuild.py.
Please integrate this into the new test driver instead (tools/run-tests.py +
tools/testrunner/local/*.py). You should find that it is much easier to work
with than the old script ;-)
In particular, look at tools/testrunner/local/progress.py (where you can
add the
new indicator) and .../execution.py (to understand how/when its methods are
called).
The comments below assume that you did this, and apply in addition.
https://chromiumcodereview.appspot.com/11000052/diff/1/tools/test.py
File tools/test.py (right):
https://chromiumcodereview.appspot.com/11000052/diff/1/tools/test.py#newcode49
tools/test.py:49: XMLOUT = None
No global variables. Please.
https://chromiumcodereview.appspot.com/11000052/diff/1/tools/test.py#newcode72
tools/test.py:72: if XMLOUT:
This looks weird. The UnitTestProgressIndicator is-a ProgressIndicator
according to its class definition (which is good!). The additional has-a
relation here is counter-intuitive. Why doesn't the
UnitTestProgressIndicator simply replace any other progress indicator in
use? If you want to print progress information to stdout at the same
time as dumping the XML file, you can always derive from one of the
printing progress indicators (and call the superclass methods when you
override them).
This comment also applies to the next few edits below.
https://chromiumcodereview.appspot.com/11000052/diff/1/tools/test.py#newcode320
tools/test.py:320:
nit: two newlines between top-level definitions
https://chromiumcodereview.appspot.com/11000052/diff/1/tools/unittest_output.py
File tools/unittest_output.py (right):
https://chromiumcodereview.appspot.com/11000052/diff/1/tools/unittest_output.py#newcode1
tools/unittest_output.py:1: # Copyright 2008 the V8 project authors. All
rights reserved.
nit: 2012
https://chromiumcodereview.appspot.com/11000052/diff/1/tools/unittest_output.py#newcode33
tools/unittest_output.py:33: import os
nit: please sort imports alphabetically
https://chromiumcodereview.appspot.com/11000052/diff/1/tools/unittest_output.py#newcode34
tools/unittest_output.py:34:
nit: two newlines between top-level definitions
https://chromiumcodereview.appspot.com/11000052/diff/1/tools/unittest_output.py#newcode35
tools/unittest_output.py:35: def getProperTestName(name):
Why do you need to extract the test's name from its command line?
https://chromiumcodereview.appspot.com/11000052/diff/1/tools/unittest_output.py#newcode42
tools/unittest_output.py:42:
nit: two newlines between top-level definitions
https://chromiumcodereview.appspot.com/11000052/diff/1/tools/unittest_output.py#newcode48
tools/unittest_output.py:48: self.starttime = time.time()
No reason to measure execution time here. It is measured anyway and you
can fetch it from the testcase object. Which means you don't need any
custom behavior when a test starts, and can put everything into the
progress indicator's HasRun() method.
https://chromiumcodereview.appspot.com/11000052/diff/1/tools/unittest_output.py#newcode61
tools/unittest_output.py:61: class UnitTestLocal(threading.local):
In the new test driver, the progress indicators are no longer threaded,
which should make this class unnecessary (and some stuff below simpler).
https://chromiumcodereview.appspot.com/11000052/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
