Initial commit for the Automated Installer Testing Framework.

chrome/test/mini_installer/test_installer.py

+# Copyright 2013 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""This script tests the installer with a series of test cases specified in

[Mathieu, 2013/08/05 18:46:13]

You're doing it right below. One-line description, then one blank line, and the
more detailed description.

+the config file. For each test case, it checks that the machine states after
+the execution of each command match the expected machine states.
+
+For more details take a look at the design documentation at http://goo.gl/Q0rGM6
+"""
+
+import json
+import optparse
+import os
+import subprocess
+
+import settings
+import verifier
+
+
+class Config:
+  """Describes the machine states, actions, and test cases.
+
+  A state is a dictionary where each key is a verifier's name and the
+  associated value is the input to that verifier. An action is a shorthand for
+  a command. A test is array of alternating state names and action names,
+  starting and ending with state names. An instance of this class stores a map
+  from state names to state objects, a map from action names to commands, and
+  an array of test objects.
+  """
+  def __init__(self):
+    self.states = {}
+    self.actions = {}
+    self.tests = []
+
+
+def MergeProperties(current_property, new_property):

[Mathieu, 2013/08/05 18:46:13]

Googling quickly for merging dicts, I found:
http://stackoverflow.com/questions/38987/how-can-i-merge-union-two-python-dic...

Would it work here?

[sukolsak, 2013/08/05 20:34:46]

No, it wouldn't work here. We are doing it differently from the one on
stackoverflow. The design doc explicitly specifies how to merge dictionaries.
For example, the result from merging {a: 0, b: {x: 1, y: 2}} and {b: {z: 3}}
should be {a: 0, b: {x: 1, y: 2, z: 3}}, not {a: 0, b: {z: 3}}. Anyway, I have
added a comment clarifying that.

[Mathieu, 2013/08/05 21:03:26]

Thanks for clarifying!

+  """Merges the new Property object into the current Property object

[Mathieu, 2013/08/05 18:46:13]

nit: period.

[Mathieu, 2013/08/05 18:46:13]

This makes it seem like Property is a class. How about "property dictionary"? If
you agree, change throughout the file.

[sukolsak, 2013/08/05 20:34:46]

Done.

[sukolsak, 2013/08/05 20:34:46]

Done.

[Mathieu, 2013/08/05 21:03:26]

Please also change the references to "Property object" throughout the file to be
property dictionary. 

[sukolsak, 2013/08/05 21:57:00]

Would changing references to "Property object" in the comments be enough, or
should I change variable names as well, e.g. current_property to
current_property_dictionary? (Personally, I don't think including data types in
variable names is a good idea.) In the spec, we use Property objects to refer to
dictionaries with certain keys for the sake of readability. This probably
doesn't make sense in Python where objects and dictionaries are not the same
thing.

[Mathieu, 2013/08/05 22:06:10]

Just the comments is fine. Generally, you want to keep the variable names short.

You can also call it "property object", although like I said my personal
preferences is dictionary. 

You can also ignore me if you're using this convention elsewhere.

+
+  Args:
+    current_property: The Property object to be modified.
+    new_property: The new Property object.
+  """
+  for key, value in new_property.iteritems():
+    if key not in current_property:
+      current_property[key] = value
+    else:
+      assert(isinstance(current_property[key], dict) and
+          isinstance(value, dict))
+      # This merges two dictionaries together. In case there are properties with
+      # the same name, the latter will override the former.
+      current_property[key] = dict(
+          current_property[key].items() + value.items())
+
+
+def ParseProperty(directory, property_filename):

[Mathieu, 2013/08/05 18:46:13]

I don't think you need this function. See below.

[sukolsak, 2013/08/05 20:34:46]

Done.

+  """Parses a .prop file.
+
+  Args:
+    property_filename: A Property filename.
+    directory: The directory where the Config file and all Property files
+        reside in.
+
+  Returns:
+    A Property object.
+  """
+  property_path = os.path.join(directory, property_filename)
+  with open(property_path, "r") as property_file:
+    return json.load(property_file)
+
+
+def ParseProperties(directory, property_filenames):

[Mathieu, 2013/08/05 18:46:13]

could you inline ParseProperty in ParseProperties?

also note my rename of the function. And see above for whether or not you need a
full MergeProperties function.

def ParsePropertyFiles(directory, filenames):
  """... usual comment..."""
  current_property = {}
  for filename in filenames:
    path = os.path.join(directory, filename)
    new_property = json.load(open(path))
    MergeProperties(current_property, new_property)
  return current_property

[sukolsak, 2013/08/05 20:34:46]

Done.

+  """Parses an array of .prop files.
+
+  Args:
+    property_filenames: An array of Property filenames.
+    directory: The directory where the Config file and all Property files
+        reside in.
+
+  Returns:
+    A Property object created by merging all Property objects specified in
+        the array.
+  """
+  current_property = {}
+  for property_filename in property_filenames:
+    new_property = ParseProperty(directory, property_filename)
+    MergeProperties(current_property, new_property)
+  return current_property
+
+
+def ParseConfig(config_filename):
+  """Parses a .config file.
+
+  Args:
+    config_filename: A Config filename.
+
+  Returns:
+    A config object.
+  """
+  config = Config()
+
+  with open(config_filename, "r") as config_file:
+    config_data = json.load(config_file)

[Mathieu, 2013/08/05 18:46:13]

as before, I would just have 
config_data = json.load(open(config_file))

Also, you're not catching the exception if the file is not found or the JSON
parsing fails (it happens a lot). 

[gab, 2013/08/05 19:06:20]

The idea was that the exception would bubble up the stack and make the test
fail. Or is it bad enough that this would make the test flaky?

[gab, 2013/08/06 14:12:56]

ping mathp

[Mathieu, 2013/08/06 14:16:07]

It's fine. I was just mentioning this to make sure you cover all the cases
gracefully. In the context of a test, you don't want to silently fail so your
thinking is perfect.

+  directory = os.path.dirname(os.path.abspath(config_filename))
+
+  for state_name, state_property_filenames in config_data["states"]:

[Mathieu, 2013/08/05 18:46:13]

single quotes, throughout all your files.

[sukolsak, 2013/08/05 20:34:46]

Done.

+    config.states[state_name] = ParseProperties(directory,
+                                                state_property_filenames)
+  for action_name, action_command in config_data["actions"]:
+    config.actions[action_name] = action_command
+  config.tests = config_data["tests"]

[Mathieu, 2013/08/05 18:46:13]

could you bring this at the top after the creation of |config|?

[sukolsak, 2013/08/05 20:34:46]

Done.

+  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):
+  """Tests the installer using the given Config object.
+
+  Args:
+    config: A Config object.
+  """
+  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)
+
+
+def main():
+  usage = "usage: %prog configfile"
+  parser = optparse.OptionParser(usage, description="Test the installer.")
+  (options, args) = parser.parse_args()
+  if len(args) != 1:
+    parser.error("incorrect number of arguments")
+
+  config = ParseConfig(args[0])
+  Test(config)
+
+
+if __name__ == "__main__":
+  main()