PTAL. I'm back to dictionaries and implemented your suggestion of filtering
names first and letting the test suite turn the names into flags by a custom
method. The generator class stayed to capture the state and the
precomputation
in there.
My measurements show that the dictionary lookups aren's significant,
therefore I
use now dicts and sets everywhere.
https://codereview.chromium.org/1245623005/diff/40001/tools/testrunner/local/testsuite.py
File tools/testrunner/local/testsuite.py (right):
https://codereview.chromium.org/1245623005/diff/40001/tools/testrunner/local/testsuite.py#newcode55
tools/testrunner/local/testsuite.py:55: def FilterVariants(variants,
variant_filter):
nit: VariantsFilterBuilder would be a more appropriate name, no?
It's now called VariantsGenerator.
https://codereview.chromium.org/1245623005/diff/40001/tools/testrunner/local/testsuite.py#newcode65
tools/testrunner/local/testsuite.py:65: def DefaultVariant(self,
testcase):
nit: this method and its friends below are private to this class, right?
Please
let their names begin with an underscore. (Don't forget to update the
overrides
too.)
Redesigned.
https://codereview.chromium.org/1245623005/diff/40001/tools/testrunner/local/testsuite.py#newcode79
tools/testrunner/local/testsuite.py:79: def __call__(self, testcase):
nit: instead of messing with Python object internals, can you just call
this
function Build(self, testcase)?
Redesigned.
https://codereview.chromium.org/1245623005/diff/40001/tools/testrunner/local/testsuite.py#newcode83
tools/testrunner/local/testsuite.py:83: Returns: An iterable over variant
tuples
(name, list of flags).
nit: IIUC this comment is a lie. It returns a callable that returns an
iterable.
Which kind of proves that this design is overly complex.
Redesigned.
https://codereview.chromium.org/1245623005/diff/40001/tools/testrunner/local/testsuite.py#newcode137
tools/testrunner/local/testsuite.py:137: def VariantFlagsBuilder(self):
nit: I don't like clashing names of unrelated things. Please either rename
this
to GetVariantFlagsBuilder, or the class above to
DefaultVariantFlagsBuilder,
or
similar.
This is now renamed to be a factory method.
https://codereview.chromium.org/1245623005/diff/40001/tools/testrunner/local/testsuite.py#newcode141
tools/testrunner/local/testsuite.py:141: def VariantFlags(self,
variant_filter):
nit: this doesn't return flags any more, but a function producing flags.
Please
rename accordingly. Maybe VariantFlagsGenerator or VariantFlagsFactory or
something to that effect?
Renamed to CreateVariantGenerator.
https://codereview.chromium.org/1245623005/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
