test/test262: update testcfg.py for new test262

test/test262/testcfg.py

Index: test/test262/testcfg.py
diff --git a/test/test262/testcfg.py b/test/test262/testcfg.py
index de3c9ad7b9f3645b7081806e137f133884ef3c31..9ee4933c6b29792164360fe294b4b2ee99fde905 100644
--- a/test/test262/testcfg.py
+++ b/test/test262/testcfg.py
@@ -36,21 +36,36 @@ from testrunner.local import testsuite
 from testrunner.local import utils
 from testrunner.objects import testcase
 
-
-TEST_262_ARCHIVE_REVISION = "fbba29f"  # This is the r365 revision.
-TEST_262_ARCHIVE_MD5 = "e1ff0db438cc12de8fb6da80621b4ef6"
+TEST_262_ARCHIVE_REVISION = "595a36b"  # This is the 2014-Aug-10 revision.
+TEST_262_ARCHIVE_MD5 = "89dfc8458f474cdb7e8e178f3215dd06"
 TEST_262_URL = "https://github.com/tc39/test262/tarball/%s"
-TEST_262_HARNESS = ["sta.js", "testBuiltInObject.js", "testIntl.js"]
+TEST_262_HARNESS = ["sta.js"]
+
+TEST_262_ROOT = ["data", "test"]
+TEST_262_SUITE_ROOT = TEST_262_ROOT + ["suite"]
+TEST_262_HARNESS_ROOT = TEST_262_ROOT + ["harness"]
+TEST_262_PYTHON_ROOT = ["data", "tools", "packaging"]
 
+# import parseTestRecord

[Jakob Kummerow, 2014/08/18 11:55:47]

Nit: comments should have proper capitalization and punctuation such as a
trailing full stop. (Again several times below.)

[smikes, 2014/08/18 16:04:26]

Removed needless comment.

+sys.path.append(os.path.abspath(os.path.join('test', 'test262', *TEST_262_PYTHON_ROOT)))

[Jakob Kummerow, 2014/08/18 11:55:47]

nit: 80col please.

Also, if we need to muck with sys.path at all, please restore it when done,
i.e.:

original_sys_path = sys.path
sys.path.append(...)
import ...
sys.path = original_sys_path

[smikes, 2014/08/18 16:04:27]

Fixed both.  Is there a more idiomatic way of importing a file
from a directory not in sys.path?

[Jakob Kummerow, 2014/08/18 16:14:10]

There is imp, https://docs.python.org/2/library/imp.html, which we use in
https://code.google.com/p/v8/source/browse/branches/bleeding_edge/tools/testr....
It's more verbose but less hacky, does that count as idiomatic?

+try:
+  from parseTestRecord import parseTestRecord

[Jakob Kummerow, 2014/08/18 11:55:47]

nit: "... as ParseTestRecord" to adhere to CapitalizedFunctionNameStyle

[smikes, 2014/08/18 16:04:26]

Done.

+except ImportError:
+  print "Need to --download-data for test262"
 
 class Test262TestSuite(testsuite.TestSuite):
 
   def __init__(self, name, root):
     super(Test262TestSuite, self).__init__(name, root)
-    self.testroot = os.path.join(root, "data", "test", "suite")
-    self.harness = [os.path.join(self.root, "data", "test", "harness", f)
-                    for f in TEST_262_HARNESS]
-    self.harness += [os.path.join(self.root, "harness-adapt.js")]
+    # set up paths
+    self.testroot = os.path.join(self.root, *TEST_262_SUITE_ROOT)
+    self.harnesspath = os.path.join(self.root, *TEST_262_HARNESS_ROOT)
+
+    # basic test harness consists of TEST_262_HARNESS files
+    # and our harness adapter
+    self.harness = [os.path.join(self.harnesspath, f)
+                    for f in TEST_262_HARNESS] + [

[Jakob Kummerow, 2014/08/18 11:55:47]

Please move the last '[' to the next line for better readability (or simply
stick with the way the previous code split lines using a second += assignment).

[smikes, 2014/08/18 16:04:26]

Done.

+                    os.path.join(self.root, "harness-adapt.js")]
 
   def CommonTestName(self, testcase):
     return testcase.path.split(os.path.sep)[-1]
@@ -74,15 +89,32 @@ class Test262TestSuite(testsuite.TestSuite):
 
   def GetFlagsForTestCase(self, testcase, context):
     return (testcase.flags + context.mode_flags + self.harness +
+            self.GetIncludesForTest(testcase) +
             [os.path.join(self.testroot, testcase.path + ".js")])
 
+  def GetIncludesForTest(self, testcase):
+    try:

[Jakob Kummerow, 2014/08/18 11:55:47]

The V8 project as a whole is not too fond of exceptions, especially for
regularly occuring non-error conditions. In this case the try..except seems
easily avoidable:

test_record = ParseTestRecord(self.GetSourceForTest(testcase),
                              testcase.path)
if "includes" in test_record:
  return [os.path.join(self.harnesspath, f)
          for f in test_record["includes"])]
else:
  return []

[smikes, 2014/08/18 16:04:27]

Done.

+      testRecord = parseTestRecord(self.GetSourceForTest(testcase), testcase.path)

[Jakob Kummerow, 2014/08/18 11:55:47]

nit: 80col

+      return [os.path.join(self.harnesspath, f)
+              for f in testRecord['includes']]
+    except KeyError:
+      return []
+
   def GetSourceForTest(self, testcase):
-    filename = os.path.join(self.testroot, testcase.path + ".js")
-    with open(filename) as f:
-      return f.read()
+    try:
+      return testcase.source

[Jakob Kummerow, 2014/08/18 11:55:47]

No need to cache this; the framework is careful not to call GetSourceForTest
repeatedly (as it assumes that this operation might be expensive).

[smikes, 2014/08/18 16:04:26]

Yes, but I was calling GetSourceForTest repeatedly (including
from IsNegativeTest).  New function GetTestRecord
calls ParseTestRecord once and stores test_record attribute on
testcase (see updated patch, soon)

+    except AttributeError:
+      filename = os.path.join(self.testroot, testcase.path + ".js")
+      with open(filename) as f:
+        testcase.source = f.read()
+      return testcase.source
 
   def IsNegativeTest(self, testcase):
-    return "@negative" in self.GetSourceForTest(testcase)
+    testRecord = parseTestRecord(self.GetSourceForTest(testcase), testcase.path)

[Jakob Kummerow, 2014/08/18 11:55:47]

nit: s/testRecord/test_record/

[smikes, 2014/08/18 16:04:27]

Done.

+    try:

[Jakob Kummerow, 2014/08/18 11:55:47]

Again, I'd prefer 'if "negative" in test_record:' over try..except.

Also, if you want to cache something, cache this, as IsNegativeTest() is
actually called twice for each test case. (Use "if hasattr(...)" rather than
"except AttributeError", please.)

[smikes, 2014/08/18 16:04:27]

Done.

+      return testRecord['negative']
+    except KeyError:
+      return False
 
   def IsFailureOutput(self, output, testpath):
     if output.exit_code != 0: