Looking pretty good now; I have a few more comments.
https://codereview.chromium.org/478163002/diff/20001/test/test262/test262.status
File test/test262/test262.status (right):
https://codereview.chromium.org/478163002/diff/20001/test/test262/test262.status#newcode47
test/test262/test262.status:47: 'S22.1.3.6_T1': [FAIL],
Can you alpha-sort these please (within each section)?
https://codereview.chromium.org/478163002/diff/20001/test/test262/testcfg.py
File test/test262/testcfg.py (right):
https://codereview.chromium.org/478163002/diff/20001/test/test262/testcfg.py#newcode47
test/test262/testcfg.py:47: TEST_262_PYTHON_ROOT = ["data", "tools",
"packaging"]
Since this is now only used in the line right below, I'd inline it
there. But come to think of it, there's more cleanup we should do:
- os.path.abspath("test/test262") is essentially what's passed in to
GetSuite as "root". We should re-use that rather than hard-code and
duplicate it. Which means the import trickery must go into GetSuite
(which shouldn't hurt, AFAICT).
- It seems a bit arbitrary which variables are pulled out and which
aren't.
- Having four paths named <x>_ROOT is a bit confusing.
Putting it all together gives us something like:
TEST_262_TESTS_PATH = ["data", "test", "suite"]
TEST_262_HARNESS_PATH = ["data", "test", "harness"]
TEST_262_TOOLS_PATH = ["data", "tools", "packaging"]
...
def GetSuite(name, root):
...
sys.path.append(os.path.join(root, *TEST_262_TOOLS_PATH))
# or the 'imp' version you're coming up with
...
return Test262TestSuite(name, root)
https://codereview.chromium.org/478163002/diff/20001/test/test262/testcfg.py#newcode94
test/test262/testcfg.py:94: if hasattr(testcase, "test_record"):
nit: one return statement is more readable, and in this case easy to
achieve:
if not hasattr(testcase, "test_record"):
testcase.test_record = ...
return testcase.test_record
https://codereview.chromium.org/478163002/diff/20001/test/test262/testcfg.py#newcode116
test/test262/testcfg.py:116: return test_record['negative']
one more nit: the rest of this file uses double quotes around strings,
let's be consistent.
https://codereview.chromium.org/478163002/
--
--
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.
