Merge lp:~javier.collado/utah/bug1082087 into lp:utah

[Javier Collado]

This branch adds a new function `master_runlist_argument` that includes
`url_argument` together with validation against the master runlist schema. This
way, the server can fail early before provisioning when the runlist doesn't
pass the validation step instead of going through all the provisioning and wait
for the client to fail.

[Javier Collado (javier.collado)]

What the bug asks for is a separate validation command that retrieves not only
the master runlists, but also the test suites and the test cases to validate
everything in one step. Hence, this merge doesn't really fix the bug, but is an
improvement over getting an error in the client after the provisioning that
could have been easily detected in the server at the very beginning.

[Andy Doan (doanac)]

+1 I think its a nice start. Should we start adding unit testing for this new function in a file like tests/test_run[_arg].py?

review: Approve

[Javier Collado (javier.collado)]

Yes, I think I should add some test case. Let me work on it and resubmit when I'm done.

[lp:~javier.collado/utah/bug1082087]

865. By Javier Collado on 2013-04-16

Updated documentation build to mock modules only when they aren't available

This makes easier to run doctests without causing any problems to the build in
readthedocs.org

866. By Javier Collado on 2013-04-16

Ignore documentation build files

867. By Javier Collado on 2013-04-16

Updated url_argument example to include a launchpad url

868. By Javier Collado on 2013-04-16

Added a usage example for the master_runlist_argument function

The example is also a doctests

869. By Javier Collado on 2013-04-16

Added master_runlist_argument unittests

[Javier Collado (javier.collado)]

I've added a doctest with an example of how to use `master_runlist_argument`
and a file with unit tests to make sure that either the filename is returned on
success or ArgumentTypeError is reased on a failure.

review: Needs Resubmitting

[lp:~javier.collado/utah/bug1082087]

870. By Javier Collado on 2013-04-16

Fixed ignore patterns

Star character was expanded by the shell by accident.

[Max Brustkern (nuclearbob)]

When I test things I get:
======================================================================
ERROR: Filename is returned on success.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/max/bzr/merges/bug1082087/tests/test_run.py", line 66, in test_filename_returned
    filename = master_runlist_argument('url')
  File "/home/max/bzr/merges/bug1082087/utah/run.py", line 111, in master_runlist_argument
    filename = url_argument(url)
  File "/usr/lib/python2.7/dist-packages/mock.py", line 341, in __call__
    ret_val = self.side_effect(*args, **kwargs)
TypeError: 'list' object is not callable

----------------------------------------------------------------------
Ran 57 tests in 19.359s

FAILED (errors=1)

I'm looking into whether that's something wrong with my setup.

[Javier Collado (javier.collado)]

The mock.py I'm looking at doesn't seem to have the same line as in your
backtrace. From what I see that file was installed in my quantal system from
main and the version is 0.8.0-2. Do you have the same version installed?

[Andy Doan (doanac)]

Not sure, but for the snippets like:

 with self.assertRaises(ArgumentTypeError):

could we use "assertRaisesRegexp" and be sure we are catching the right thing?

[Javier Collado (javier.collado)]

Yes, I can do that. Indeed, even if the exception is always of the same type,
the message is different depending on the error. Let me add that and resubmit.

[lp:~javier.collado/utah/bug1082087]

871. By Javier Collado on 2013-04-16

Added patterns to check error messages in exceptions

The idea is to make sure that exceptions of the same type were raised because
of different reasons as suggested by Andy.

[Javier Collado (javier.collado)]

Replaced assertRaises with assertRaisesRegexp to make sure about why the
ArgumentTypeError exceptions were raised.

review: Needs Resubmitting

[Javier Collado (javier.collado)]

Thanks for the suggestion, by the way.

[Andy Doan (doanac)]

looks nice

review: Approve

=== added file '.bzrignore'
--- .bzrignore	1970-01-01 00:00:00 +0000
+++ .bzrignore	2013-04-16 15:26:28 +0000
@@ -0,0 +1,2 @@
+docs/source/man/*.txt
+docs/build/*

=== modified file 'debian/changelog'
--- debian/changelog	2013-04-16 13:21:57 +0000
+++ debian/changelog	2013-04-16 15:26:28 +0000
@@ -17,6 +17,7 @@
   * Try to close SFTP client only if it's open (LP: #1163197)
   * Use sys.exit when SIGTERM is received (LP: #1160247)
   * Write syslog pattern matched and timeouts timestamps (LP: #1162862)
+  * Validate master runlist before provisioning (LP: #1082087)
 
   [ Max Brustkern ]
   * Cleaned up docstrings to be PEP257 compliant

=== modified file 'docs/source/conf.py'
--- docs/source/conf.py	2013-04-04 15:39:49 +0000
+++ docs/source/conf.py	2013-04-16 15:26:28 +0000
@@ -79,10 +79,15 @@
 
 
 # Mock all third party modules not available in the readthedocs.org environment
+# Note that in a development environment where the modules are available, they
+# are not mocked to make it possible to run the doctests correctly.
 for mod_name in ('psutil', 'yaml', 'paramiko', 'jsonschema', 'libvirt',
                  'bzrlib', 'bzrlib.builtins', 'bzrlib.plugin', 'bzrlib.errors',
                  'apt', 'apt.cache', 'netifaces'):
-    sys.modules[mod_name] = ModuleMock(mod_name)
+    try:
+        __import__(mod_name)
+    except ImportError:
+        sys.modules[mod_name] = ModuleMock(mod_name)
 
 
 def get_parser_strings(script_name, parser):

=== added file 'tests/test_run.py'
--- tests/test_run.py	1970-01-01 00:00:00 +0000
+++ tests/test_run.py	2013-04-16 15:26:28 +0000
@@ -0,0 +1,70 @@
+# Ubuntu Testing Automation Harness
+# Copyright 2013 Canonical Ltd.
+
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Test command line argument functions."""
+
+import unittest
+
+from argparse import ArgumentTypeError
+from jsonschema import ValidationError
+from mock import patch
+
+from utah.client.exceptions import (
+    YAMLEmptyFile,
+    YAMLParsingError,
+)
+from utah.run import master_runlist_argument
+
+
+class TestMasterRunlistArgument(unittest.TestCase):
+    """Test master_runlist_argument function."""
+
+    @patch('utah.run.parse_yaml_file')
+    @patch('utah.run.url_argument')
+    def test_empty_file(self, url_argument, parse_yaml_file):
+        """ArgumentTypeError is raised on empty file."""
+        parse_yaml_file.side_effect = YAMLEmptyFile()
+        pattern = r'Master runlist seems to be empty:'
+        with self.assertRaisesRegexp(ArgumentTypeError, pattern):
+            master_runlist_argument('url')
+
+    @patch('utah.run.parse_yaml_file')
+    @patch('utah.run.url_argument')
+    def test_parsing_error(self, url_argument, parse_yaml_file):
+        """ArgumentTypeError is raised on parsing error."""
+        parse_yaml_file.side_effect = YAMLParsingError()
+        pattern = r'Master runlist failed to parse as a yaml file:'
+        with self.assertRaisesRegexp(ArgumentTypeError, pattern):
+            master_runlist_argument('url')
+
+    @patch('utah.run.jsonschema.validate')
+    @patch('utah.run.parse_yaml_file')
+    @patch('utah.run.url_argument')
+    def test_validation_error(self, url_argument, parse_yaml_file, validate):
+        """ArgumentTypeError is raised on validation error."""
+        validate.side_effect = ValidationError('error description')
+        pattern = 'Master runlist failed to validate:'
+        with self.assertRaisesRegexp(ArgumentTypeError, pattern):
+            master_runlist_argument('url')
+
+    @patch('utah.run.jsonschema.validate')
+    @patch('utah.run.parse_yaml_file')
+    @patch('utah.run.url_argument')
+    def test_filename_returned(self, url_argument, parse_yaml_file, validate):
+        """Filename is returned on success."""
+        expected_filename = 'filename'
+        url_argument.side_effect = [expected_filename]
+        filename = master_runlist_argument('url')
+        self.assertEqual(filename, expected_filename)

=== modified file 'utah/run.py'
--- utah/run.py	2013-04-11 19:09:06 +0000
+++ utah/run.py	2013-04-16 15:26:28 +0000
@@ -16,6 +16,7 @@
 """Provide routines used to process command line arguments and run tests."""
 
 
+from argparse import ArgumentTypeError
 import logging
 import os
 import shutil
@@ -26,13 +27,23 @@
 import traceback
 import urllib
 
+import jsonschema
+
 from utah import config
+from utah.client.common import (
+    parse_yaml_file,
+    ReturnCodes as ClientReturnCodes,
+)
+from utah.client.runner import Runner
+from utah.client.exceptions import (
+    YAMLEmptyFile,
+    YAMLParsingError,
+)
 from utah.exceptions import UTAHException
 from utah.retry import retry
 from utah.timeout import timeout
 from utah.url import url_argument
 from utah.provisioning.ssh import ProvisionedMachine
-from utah.client.common import ReturnCodes as ClientReturnCodes
 
 
 # Return codes for the server
@@ -64,6 +75,63 @@
         return returncode + offset
 
 
+def master_runlist_argument(url):
+    """Get master runlist and validate it against its schema.
+
+    This function calls :func:`utah.url.url_argument` to download the runlist
+    file if needed and then uses the schema defined in
+    :class:`utah.client.runner.Runner` to validate it.
+
+    This functions is expected to be passed as the `type` argument of an
+    `argparse.ArgumentParser` object to make sure the master runlist is
+    reachable and valid before provisioning the system under test.
+
+    :param url: URL as passed to parser object.
+    :type url: `basestring`
+    :returns: URL or path to the local file
+    :rtype: `basestring`
+
+    .. seealso::
+        :func:`utah.url.url_argument`,
+        :class:`utah.client.runner.Runner`.
+
+    :Example:
+
+    >>> from utah.run import master_runlist_argument
+    >>> import argparse
+    >>> parser = argparse.ArgumentParser()
+    >>> parser.add_argument('url',
+    ...                     type=master_runlist_argument)  # doctest: +ELLIPSIS
+    _StoreAction(... dest='url', ...)
+    >>> runlist = 'lp:utah/utah/client/examples/pass.run'
+    >>> parser.parse_args([runlist])  # doctest: +ELLIPSIS
+    Namespace(url='/tmp/utah_...')
+
+    """
+    filename = url_argument(url)
+    try:
+        data = parse_yaml_file(filename)
+    except YAMLEmptyFile:
+        raise ArgumentTypeError(
+            'Master runlist seems to be empty: {!r}'
+            .format(filename))
+    except YAMLParsingError as exception:
+        raise ArgumentTypeError(
+            'Master runlist failed to parse as a yaml file: {!r}\n'
+            'Detailed information: {}'
+            .format(filename, exception))
+
+    try:
+        jsonschema.validate(data, Runner.MASTER_RUNLIST_SCHEMA)
+    except jsonschema.ValidationError as exception:
+        raise ArgumentTypeError(
+            'Master runlist failed to validate: {!r}\n'
+            'Detailed information: {}'
+            .format(filename, exception))
+
+    return filename
+
+
 def common_arguments(parser):
     """Centralize command line arguments for all run_ scripts.
 
@@ -72,7 +140,7 @@
 
     """
     parser.add_argument('runlist', metavar='runlist',
-                        type=url_argument,
+                        type=master_runlist_argument,
                         help='URLs of runlist files to run')
     parser.add_argument('-s', '--series', metavar='SERIES',
                         choices=config.serieschoices,

=== modified file 'utah/url.py'
--- utah/url.py	2013-04-03 21:17:26 +0000
+++ utah/url.py	2013-04-16 15:26:28 +0000
@@ -196,6 +196,8 @@
     _StoreAction(... dest='url', ...)
     >>> parser.parse_args(['http://www.ubuntu.com'])
     Namespace(url='http://www.ubuntu.com')
+    >>> parser.parse_args(['lp:utah/setup.py'])  # doctest: +ELLIPSIS
+    Namespace(url='/tmp/utah_...')
 
     .. seealso:: :class:`URLChecker`