Addresses Python style issues in test_configuration.py and test_case.py

tools/testing/test_case.py

Index: tools/testing/test_case.py
diff --git a/tools/testing/test_case.py b/tools/testing/test_case.py
index 1c4f7f0f01ea7020175eb719308ea2a45a90365b..0caa178488ad7a5b0a5aa0e98b656d9c0f8b642b 100644
--- a/tools/testing/test_case.py
+++ b/tools/testing/test_case.py
@@ -2,28 +2,22 @@
 # for details. All rights reserved. Use of this source code is governed by a
 # BSD-style license that can be found in the LICENSE file.
 
-import atexit
-import fileinput
+"""Common TestCase subclasses used to define a single test."""
+
 import os
-import test
-import platform
-import re
-import sys
 import tempfile
 
-import architecture
 import test
-import utils
+from testing import architecture
 
-from os.path import join, exists, basename
-
-import utils
 
 class Error(Exception):
   pass
 
 
 class StandardTestCase(test.TestCase):
+  """A test case defined by a .dart file."""

[ngeoffray, 2011/10/13 08:03:38]

Maybe say a *Test.dart file. But feel free to ignore if you disagree.

[zundel, 2011/10/13 11:07:44]

SGTM, Done.

+
   def __init__(self, context, path, filename, mode, arch):
     super(StandardTestCase, self).__init__(context, path)
     self.filename = filename
@@ -35,13 +29,13 @@ class StandardTestCase(test.TestCase):
       self.run_arch.vm_options.append(flag)
 
   def IsNegative(self):
-    return self.GetName().endswith("NegativeTest")
+    return self.GetName().endswith('NegativeTest')
 
   def GetLabel(self):
-    return "%s%s %s" % (self.mode, self.arch, '/'.join(self.path))
+    return '%s%s %s' % (self.mode, self.arch, '/'.join(self.path))
 
   def GetCommand(self):
-    return self.run_arch.GetRunCommand();
+    return self.run_arch.GetRunCommand()
 
   def GetName(self):
     return self.path[-1]
@@ -55,20 +49,24 @@ class StandardTestCase(test.TestCase):
   def Cleanup(self):
     # TODO(ngeoffray): We run out of space on the build bots for these tests if
     # the temp directories are not removed right after running the test.
-    if not self.context.keep_temporary_files: self.run_arch.Cleanup()
+    if not self.context.keep_temporary_files:
+      self.run_arch.Cleanup()
 
 
 class MultiTestCase(StandardTestCase):
+  """Multiple test cases defined within a single .dart file."""

[ngeoffray, 2011/10/13 08:03:38]

ditto

[zundel, 2011/10/13 11:07:44]

Done.

 
   def __init__(self, context, path, filename, kind, mode, arch):
     super(MultiTestCase, self).__init__(context, path, filename, mode, arch)
     self.kind = kind
 
   def GetCommand(self):
+    """Returns a commandline to execute to perform the test."""
     return self.run_arch.GetRunCommand(
-      fatal_static_type_errors=(self.kind == 'static type error'));
+        fatal_static_type_errors=(self.kind == 'static type error'))
 
   def IsNegative(self):
+    """Returns True if this is a negative test (should return FAIL)."""

[ngeoffray, 2011/10/13 08:03:38]

The comment is parenthesis is confusing, I would put it elsewhere. Maybe where
we check on the test output? Or leave it it but extand it, like:

"""Returns True if this is a negative test. A negative test passes if its output
is FAIL."""

[zundel, 2011/10/13 11:07:44]

Done.

     if self.kind == 'compile-time error':
       return True
     if self.kind == 'runtime error':
@@ -77,16 +75,19 @@ class MultiTestCase(StandardTestCase):
       return self.run_arch.HasFatalTypeErrors()
     return False
 
+
 class BrowserTestCase(StandardTestCase):
+  """A test case that executes inside a browser (or DumpRenderTree)."""
+
   def __init__(self, context, path, filename,
                fatal_static_type_errors, mode, arch):
     super(BrowserTestCase, self).__init__(context, path, filename, mode, arch)
     self.fatal_static_type_errors = fatal_static_type_errors
 
-
   def Run(self):
+    """Optionally compiles and then runs the specified test."""
     command = self.run_arch.GetCompileCommand(self.fatal_static_type_errors)
-    if command != None:
+    if command:
       # We change the directory where dartc will be launched because
       # it is not predictable on the location of the compiled file. In
       # case the test is a web test, we make sure the app file is not
@@ -100,7 +101,7 @@ class BrowserTestCase(StandardTestCase):
       if test_output.output.exit_code != 0:
         return test_output
 
-    command = self.run_arch.GetRunCommand();
+    command = self.run_arch.GetRunCommand()
     test_output = self.RunCommand(command)
     # The return value of DumpRenderedTree does not indicate test failing, but
     # the output does.
@@ -111,7 +112,8 @@ class BrowserTestCase(StandardTestCase):
 
 
 class CompilationTestCase(test.TestCase):
-  """ Run the dartc compiler on a given top level dart file """
+  """Run the dartc compiler on a given top level dart file."""
+
   def __init__(self, path, context, filename, mode, arch):
     super(CompilationTestCase, self).__init__(context, path)
     self.filename = filename
@@ -125,10 +127,11 @@ class CompilationTestCase(test.TestCase):
     return False
 
   def GetLabel(self):
-    return "%s/%s %s" % (self.mode, self.arch, '/'.join(self.path))
+    return '%s/%s %s' % (self.mode, self.arch, '/'.join(self.path))
 
   def GetCommand(self):
-    cmd = self.context.GetDartC(self.mode, self.arch);
+    """Returns a command line to run the test."""
+    cmd = self.context.GetDartC(self.mode, self.arch)
     cmd += self.context.flags
     cmd += ['-check-only',
             '-fatal-type-errors',