Use unittest framework in the Automated Installer Testing Framework.

chrome/test/mini_installer/test_installer.py

Index: chrome/test/mini_installer/test_installer.py
diff --git a/chrome/test/mini_installer/test_installer.py b/chrome/test/mini_installer/test_installer.py
index a59bf9164074f3706728b0b4042490dfee34f2c3..bf35341a596cb354764f5ebbc38cb0d412b3cc75 100644
--- a/chrome/test/mini_installer/test_installer.py
+++ b/chrome/test/mini_installer/test_installer.py
@@ -13,9 +13,9 @@ import argparse
 import json
 import os
 import subprocess
+import unittest
 
-import settings
-import verifier
+from verifier import Verifier
 
 
 class Config:
@@ -34,6 +34,88 @@ class Config:
     self.tests = []
 
 
+class InstallerTest(unittest.TestCase):
+  """Tests a test case in the config file."""

[Mathieu, 2013/08/09 17:25:15]

The guide says you should describe class attributes if they are public (i.e. not
preceded by a _). I think there is no harm in having them private (you're not
accessing the attributes from outside the class, right?).

[sukolsak, 2013/08/09 21:12:38]

Done.

+
+  def __init__(self, test, config):
+    """Constructor.
+
+    Args:
+      test: An array of alternating state names and action names, starting and
+          ending with state names.
+      config: The Config object.
+    """
+    super(InstallerTest, self).__init__()

[Mathieu, 2013/08/09 17:25:15]

initialize self._current_state to None. It's best to declare every class
attribute that you're going to use.

[sukolsak, 2013/08/09 21:12:38]

Done.

+    self.test = test

[Mathieu, 2013/08/09 17:25:15]

therefore

self._test
self._config
self._verifier

[sukolsak, 2013/08/09 21:12:38]

Done.

+    self.config = config
+    self.verifier = Verifier(self)

[Mathieu, 2013/08/09 16:08:43]

you are passing the whole object in the Verifier constructor?

[gab, 2013/08/09 16:16:11]

We would love to avoid this! Is there any way to instead statically assert
without requiring the TestCase object to do say testcase.fail()...?

[Mathieu, 2013/08/09 17:25:15]

How about throwing an exception with a useful message (a VerificationError let's
say), that the caller below would intercept and fail() on the message. It would
be much preferable to passing the whole testcase to verifier. This works because
execution needs to stop when there is one fail (i.e. you're not collecting
fails, if that makes sense).

[sukolsak, 2013/08/09 21:19:40]

I have removed |testcase| from verifier (and thus no need to make it a class)
and made it throw an exception when an error occurs. I'm not sure if this
addresses your comment. Could you take a look?

+
+  def runTest(self):
+    """Run the test case."""
+    test = self.test

[Mathieu, 2013/08/09 17:25:15]

use self._test throughout

[sukolsak, 2013/08/09 21:12:38]

Done.

+
+    # |test| is an array of alternating state names and action names, starting
+    # and ending with state names. Therefore, its length must be odd.
+    self.assertEqual(len(test) % 2, 1, 'The length of test array must be odd')

[Mathieu, 2013/08/09 17:25:15]

-expectation should be 1st param

self.assertEqual(1, len(self._test) % 2, '...')

[sukolsak, 2013/08/09 21:12:38]

Done.

+
+    self._RunResetCommand()
+
+    self.current_state = test[0]
+    self._VerifyCurrentState()
+
+    for i in range(1, len(test), 2):

[Mathieu, 2013/08/09 17:25:15]

Add a comment saying exactly what you're looping over by jumping by 2 at every
iteration (is it states, actions?).

Example:
# Starting at index 1, we loop through pairs of (..., ...).

[sukolsak, 2013/08/09 21:12:38]

Done.

+      action = test[i]
+      self._RunCommand(self.config.actions[action])
+
+      self.current_state = test[i + 1]
+      self._VerifyCurrentState()
+
+  def __str__(self):

[Mathieu, 2013/08/09 17:25:15]

nit: bring it up to be next to __init__?

[sukolsak, 2013/08/09 21:12:38]

Done.

+    """Returns a string representing the test case.
+
+    This method returns a string created by joining state names and action names

[Mathieu, 2013/08/09 17:25:15]

"This method returns" -> 

Returns:
  A string created...

[sukolsak, 2013/08/09 21:12:38]

Done.

+    together with ' -> ', for example,
+    'Test: clean -> install chrome -> chrome_installed'.
+    """
+    return 'Test: %s' % (' -> '.join(self.test))
+
+  def shortDescription(self):
+    """Returns None as the short description to suppress its printing.
+

[Mathieu, 2013/08/09 17:25:15]

Add a line saying:

Overriden from unittest.TestCase.

also in failureException

[sukolsak, 2013/08/09 21:12:38]

Done.

+    The default implementation of this method returns the docstring of the
+    runTest method, which is not useful since it's the same for every test case.
+    The description from the __str__ method is informative enough.
+    """
+    return None
+
+  def failureException(self, msg):
+    """Returns an AssertionError with the current state information.
+
+    The default implementation of this method returns AssertionError(msg).
+    We add the current state information to the message so that we know where
+    the test fails.
+
+    Args:
+      msg: The error message.
+
+    Returns:
+      An AssertionError.
+    """
+    return AssertionError("In state '%s', %s" % (self.current_state, msg))
+
+  def _VerifyCurrentState(self):
+    """Verifies that the current machine state matches the expected machine

[Mathieu, 2013/08/09 17:25:15]

Make it fit on one line

[sukolsak, 2013/08/09 21:12:38]

Done.

+    state."""
+    self.verifier.Verify(self.config.states[self.current_state])
+
+  def _RunCommand(self, command):
+    subprocess.call(command, shell=True)
+
+  def _RunResetCommand(self):
+    # TODO(sukolsak): Need to figure how exactly we want to reset.

[Mathieu, 2013/08/09 17:25:15]

nit: put this TODO in place of _RunResetCommand, above and remove this function.
Once you add it, you'll have a better idea of what it does and be able to
provide good doc for it.

[sukolsak, 2013/08/09 21:12:38]

Done.

+    pass
+
+
 def MergePropertyDictionaries(current_property, new_property):
   """Merges the new property dictionary into the current property dictionary.
 
@@ -100,54 +182,16 @@ def ParseConfigFile(filename):
   return config
 
 
-def VerifyState(config, state):
-  """Verifies that the current machine states match the given machine states.
-
-  Args:
-    config: A Config object.
-    state: The current state.
-  """
-  # TODO(sukolsak): Think of ways of preserving the log when the test fails but
-  # not printing these when the test passes.
-  print settings.PRINT_STATE_PREFIX + state
-  verifier.Verify(config.states[state])
-
-
-def RunCommand(command):
-  print settings.PRINT_COMMAND_PREFIX + command
-  subprocess.call(command, shell=True)
-
-
-def RunResetCommand():
-  print settings.PRINT_COMMAND_PREFIX + 'Reset'
-  # TODO(sukolsak): Need to figure how exactly we want to reset.
-
-
 def Test(config):

[Mathieu, 2013/08/09 17:25:15]

rename to RunTests? Having test everywhere makes my head spin :)

[sukolsak, 2013/08/09 21:12:38]

Done.

   """Tests the installer using the given Config object.
 
   Args:
     config: A Config object.
   """
+  suite = unittest.TestSuite()
   for test in config.tests:
-    print settings.PRINT_TEST_PREFIX + ' -> '.join(test)
-
-    # A Test object is an array of alternating state names and action names.
-    # The array starts and ends with states. Therefore, the length must be odd.
-    assert(len(test) % 2 == 1)
-
-    RunResetCommand()
-
-    current_state = test[0]
-    VerifyState(config, current_state)
-    # TODO(sukolsak): Quit the test early if VerifyState fails at any point.
-
-    for i in range(1, len(test), 2):
-      action = test[i]
-      RunCommand(config.actions[action])
-
-      current_state = test[i + 1]
-      VerifyState(config, current_state)
+    suite.addTest(InstallerTest(test, config))
+  unittest.TextTestRunner(verbosity=2).run(suite)
 
 
 def main():