LAVA Tool

Merge lp:~milo/lava-tool/lava-165 into lp:~linaro-validation/lava-tool/trunk

lava-165
Merge into trunk

Proposed by Milo Casagrande on 2013-07-16

Status:	Merged
Merged at revision:	188
Proposed branch:	lp:~milo/lava-tool/lava-165
Merge into:	lp:~linaro-validation/lava-tool/trunk
Prerequisite:	lp:~milo/lava-tool/device-parameters
Diff against target:	3405 lines (+2196/-459) 30 files modified .coveragerc (+1/-0) entry_points.ini (+8/-0) lava/commands.py (+240/-0) lava/config.py (+50/-17) lava/device/__init__.py (+3/-3) lava/device/commands.py (+8/-6) lava/device/tests/test_commands.py (+25/-18) lava/device/tests/test_device.py (+16/-11) lava/helper/command.py (+137/-60) lava/helper/template.py (+80/-0) lava/helper/tests/helper_test.py (+24/-4) lava/helper/tests/test_command.py (+0/-89) lava/helper/tests/test_template.py (+102/-0) lava/job/__init__.py (+40/-12) lava/job/commands.py (+21/-77) lava/job/templates.py (+84/-19) lava/job/tests/test_commands.py (+2/-9) lava/job/tests/test_job.py (+40/-29) lava/parameter.py (+199/-17) lava/testdef/__init__.py (+60/-0) lava/testdef/commands.py (+72/-0) lava/testdef/templates.py (+75/-0) lava/testdef/tests/test_commands.py (+153/-0) lava/tests/test_commands.py (+128/-0) lava/tests/test_config.py (+59/-65) lava/tests/test_parameter.py (+92/-11) lava_tool/tests/__init__.py (+13/-10) lava_tool/tests/test_utils.py (+177/-2) lava_tool/utils.py (+286/-0) setup.py (+1/-0)
To merge this branch:	bzr merge lp:~milo/lava-tool/lava-165
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Antonio Terceiro		2013-07-16	Pending
Review via email: mp+174942@code.launchpad.net

Description of the change

This adds final work towards completing the lava helper tools for card-15.
The merge adds the following commands:

lava init [DIR]
lava run [JOB|DIR]
lava submit [JOB|DIR]
lava update [JOB|DIR]

The "init" command initializes a directory with a job definition file, and a default subdirectory called "tests/" that will contain the test definition file(s), and a default shell script, called "mytest.sh". The test definition file steps, by default, will execute the shell script. The user is prompted to fill in the shell script.

The "run" command runs a job file on the local dispatcher, the "submit" one will send the job file to a remote LAVA server.

The "update" command can be used, in case the user modifies the content of the "tests/" directory, to update the job file.

The "run", "submit" and "update" commands can accept a job file (.json extension), a directory (it will pick up the first .json file) or nothing to use the current working directory.

New Parameter class have been introduced, not used at this stage, but where used with the first implementation. New attributes have also been added in order not to store the parameter on disk at program exit.

Tests have been added to cover most of the new features.

lp:~milo/lava-tool/lava-165 updated on 2013-07-22

389. By Milo Casagrande on 2013-07-16

Updated copyright info, fixed typo.

390. By Milo Casagrande on 2013-07-17

Reworked the default job tempalte.

    * Remove the UrlListParameter and introduced the TarRepoParameter.
    * Reworked the key of the job templates.
    * Fixed the tests.

391. By Milo Casagrande on 2013-07-17

Fixed testdef_repos parameter: it is an array.

392. By Milo Casagrande on 2013-07-17

Fixed template access.

393. By Milo Casagrande on 2013-07-17

New functions to get and set a key in a template.

* Added new tests too.

394. By Milo Casagrande on 2013-07-18

Used device_type, not target.

395. By Milo Casagrande on 2013-07-18

Make sure shell script is executable.

* Removed unused imports.

396. By Milo Casagrande on 2013-07-18

Fixed docstring.

397. By Milo Casagrande on 2013-07-18

Removed unused imports.

398. By Milo Casagrande on 2013-07-22

Added submit_results command.

    * Added new step in job template to submit the results.
      It will ask two parameters: the stream and the server.
    * Fixed timeout value in lava_test_shell command.

399. By Milo Casagrande on 2013-07-22

Added default parse parttern for testdef, updated tests.

Revision history for this message

Antonio Terceiro (terceiro) wrote on 2013-07-24:

Download full text (57.8 KiB)

On Tue, Jul 16, 2013 at 07:41:29AM -0000, Milo Casagrande wrote:
> Milo Casagrande has proposed merging lp:~milo/lava-tool/lava-165 into lp:lava-tool with lp:~milo/lava-tool/device-parameters as a prerequisite.
>
> Requested reviews:
> Antonio Terceiro (terceiro)
>
> For more details, see:
> https://code.launchpad.net/~milo/lava-tool/lava-165/+merge/174942
>
> This adds final work towards completing the lava helper tools for card-15.
> The merge adds the following commands:
>
> lava init [DIR]
> lava run [JOB|DIR]
> lava submit [JOB|DIR]
> lava update [JOB|DIR]
>
> The "init" command initializes a directory with a job definition file, and a default subdirectory called "tests/" that will contain the test definition file(s), and a default shell script, called "mytest.sh". The test definition file steps, by default, will execute the shell script. The user is prompted to fill in the shell script.
>
> The "run" command runs a job file on the local dispatcher, the "submit" one will send the job file to a remote LAVA server.
>
> The "update" command can be used, in case the user modifies the content of the "tests/" directory, to update the job file.
>
> The "run", "submit" and "update" commands can accept a job file (.json extension), a directory (it will pick up the first .json file) or nothing to use the current working directory.
>
> New Parameter class have been introduced, not used at this stage, but where used with the first implementation. New attributes have also been added in order not to store the parameter on disk at program exit.
>
> Tests have been added to cover most of the new features.

arf ... this one was big!

On Tue, Jul 16, 2013 at 07:41:29AM -0000, Milo Casagrande wrote:
> Milo Casagrande has proposed merging lp:~milo/lava-tool/lava-165 into lp:lava-tool with lp:~milo/lava-tool/device-parameters as a prerequisite.
> 
> Requested reviews:
>   Antonio Terceiro (terceiro)
> 
> For more details, see:
> https://code.launchpad.net/~milo/lava-tool/lava-165/+merge/174942
> 
> This adds final work towards completing the lava helper tools for card-15.
> The merge adds the following commands:
> 
> lava init [DIR]
> lava run [JOB|DIR]
> lava submit [JOB|DIR]
> lava update [JOB|DIR]
> 
> The "init" command initializes a directory with a job definition file, and a default subdirectory called "tests/" that will contain the test definition file(s), and a default shell script, called "mytest.sh". The test definition file steps, by default, will execute the shell script. The user is prompted to fill in the shell script.
> 
> The "run" command runs a job file on the local dispatcher, the "submit" one will send the job file to a remote LAVA server.
> 
> The "update" command can be used, in case the user modifies the content of the "tests/" directory, to update the job file.
> 
> The "run", "submit" and "update" commands can accept a job file (.json extension), a directory (it will pick up the first .json file) or nothing to use the current working directory.
> 
> New Parameter class have been introduced, not used at this stage, but where used with the first implementation. New attributes have also been added in order not to store the parameter on disk at program exit.
> 
> Tests have been added to cover most of the new features.

arf ... this one was big!

> === added file 'lava/commands.py'
> --- lava/commands.py	1970-01-01 00:00:00 +0000
> +++ lava/commands.py	2013-07-22 09:32:26 +0000
> @@ -0,0 +1,293 @@
> +# Copyright (C) 2013 Linaro Limited
> +#
> +# Author: Milo Casagrande <milo.casagrande@linaro.org>
> +#
> +# This file is part of lava-tool.
> +#
> +# lava-tool is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU Lesser General Public License version 3
> +# as published by the Free Software Foundation
> +#
> +# lava-tool is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public License
> +# along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
> +
> +"""
> +Lava init commands.
> +
> +When invoking:
> +
> + `lava init [DIR]`
> +
> +the command will create a default directory and files structure as follows:
> +
> +DIR/
> +   |
> +   +- JOB_FILE.json
> +   +- tests/
> +           |
> +           + mytest.sh
> +           + lavatest.yaml
> +
> +If DIR is not passed, it will use the current working directory.
> +JOB_FILE is a file name that will be asked to the user, along with
> +other necessary information to define the tests.
> +
> +If the user manually updates either the lavatest.yaml or mytest.sh file, it is
> +necessary to run the following command in order to update the job definition:
> +
> + `lava update [JOB|DIR]`
> +"""
> +
> +import copy
> +import json
> +import os
> +import sys
> +import stat
> +
> +from lava.helper.command import BaseCommand
> +from lava.helper.template import (
> +    expand_template,
> +    set_value
> +)
> +from lava.job.commands import JOB_FILE_EXTENSIONS
> +from lava.parameter import (
> +    Parameter,
> +    TarRepoParameter,
> +)
> +from lava.job.templates import (
> +    TESTDEF_REPOS_TAR_REPO,
> +)
> +from lava.testdef.templates import (
> +    DEFAULT_TESTDEF_FILE,
> +    DEFAULT_TESTDEF_SCRIPT,
> +    DEFAULT_TESTDEF_SCRIPT_CONTENT,
> +)
> +from lava.tool.errors import CommandError
> +
> +# Default directory structure name.
> +TESTS_DIR = "tests"
> +
> +# Internal parameter ids.
> +JOBFILE_ID = "jobfile"
> +
> +JOBFILE_PARAMETER = Parameter(JOBFILE_ID)
> +JOBFILE_PARAMETER.store = False
> +
> +INIT_TEMPLATE = {
> +    JOBFILE_ID: JOBFILE_PARAMETER,
> +}

Do we really need a dictionary with a single key?

> +
> +
> +class init(BaseCommand):
> +    """Set-ups the base directory structure."""
> +
> +    @classmethod
> +    def register_arguments(cls, parser):
> +        super(init, cls).register_arguments(parser)
> +        parser.add_argument("DIR",
> +                            help=("The name of the directory to initialize. "
> +                                  "Defaults to current working directory."),
> +                            nargs="?",
> +                            default=os.getcwd())
> +
> +    def invoke(self):
> +        full_path = os.path.abspath(self.args.DIR)
> +
> +        if os.path.isfile(full_path):
> +            raise CommandError("'{0}' already exists, and is a "
> +                               "file.".format(self.args.DIR))
> +
> +        if not os.path.isdir(full_path):
> +            try:
> +                os.makedirs(full_path)
> +            except OSError:
> +                raise CommandError("Cannot create directory "
> +                                   "'{0}'.".format(self.args.DIR))
> +
> +        test_path = os.path.join(full_path, TESTS_DIR)
> +        if not os.path.isdir(test_path):
> +            try:
> +                os.makedirs(test_path)
> +            except OSError:
> +                raise CommandError("Cannot create directory "
> +                                   "'{0}'.".format(self.args.DIR))
> +
> +        data = self._update_data()
> +        self._create_files(data, full_path, test_path)
> +
> +    def _update_data(self):
> +        """Updates the template and ask values to the user.
> +
> +        The template in this case is a layout of the directory structure as it
> +        would be written to disk.
> +
> +        :return A dictionary containing all the necessary file names to create.
> +        """
> +        data = copy.deepcopy(INIT_TEMPLATE)
> +        expand_template(data, self.config)
> +
> +        return data
> +
> +    def _create_files(self, data, full_path, test_path):
> +        # This is the default script file as defined in the testdef template.
> +        default_script = os.path.join(test_path, DEFAULT_TESTDEF_SCRIPT)
> +
> +        if not os.path.isfile(default_script):
> +            # We do not have the default testdef script. Create it, but
> +            # remind the user to update it.
> +            print >> sys.stdout, ("\nCreating default test script "
> +                                  "'{0}'.".format(DEFAULT_TESTDEF_SCRIPT))
> +
> +            with open(default_script, "w") as write_file:
> +                write_file.write(DEFAULT_TESTDEF_SCRIPT_CONTENT)

if this behaviour of "create a file with this content here" starts to repeat,
we probably want to encapsulatet it somewhere else (I noted it at least once in
other parts of this MP).

> +
> +            # Prompt the user to write the script file.
> +            self.edit_file(default_script)
> +
> +        # Make sure the script is executable.
> +        os.chmod(default_script,
> +                 stat.S_IRWXU | stat.S_IRWXG | stat.S_IROTH | stat.S_IXOTH)
> +
> +        print >> sys.stdout, ("\nCreating test definition "
> +                              "'{0}':".format(DEFAULT_TESTDEF_FILE))

print already writes to stdout by default. is it really necessary to be that explicit?

> +        self._create_test_file(os.path.join(test_path,
> +                                            DEFAULT_TESTDEF_FILE))
> +
> +        job = data[JOBFILE_ID]
> +        print >> sys.stdout, "\nCreating job file '{0}':".format(job)
> +        self._create_job_file(os.path.join(full_path, job), test_path)
> +
> +    def _create_job_file(self, job_file, test_path=None):
> +        """Creates the job file on the filesystem."""
> +
> +        # Invoke the command to create new job files: make a copy of the local
> +        # args and add what is necessary for the command.
> +        from lava.job.commands import new
> +
> +        args = copy.copy(self.args)
> +        args.FILE = job_file
> +
> +        job_cmd = new(self.parser, args)
> +        job_cmd.invoke(tests_dir=test_path)
> +
> +    def _create_test_file(self, test_file):
> +        """Creates the test definition file on the filesystem."""
> +
> +        # Invoke the command to create new testdef files: make a copy of the
> +        # local args and add what is necessary for the command.
> +        from lava.testdef.commands import new
> +
> +        args = copy.copy(self.args)
> +        args.FILE = test_file
> +
> +        testdef_cmd = new(self.parser, args)
> +        testdef_cmd.invoke()

I’m not sure about this approach of calling other commands. IMO the best
approach would be to concentrate as much of the logic as possible in the
model/backend classes, them call them from both commands.

> +class run(BaseCommand):
> +    """Runs a job on the local dispatcher."""
> +
> +    @classmethod
> +    def register_arguments(cls, parser):
> +        super(run, cls).register_arguments(parser)
> +        parser.add_argument("JOB",
> +                            help=("The job file to run, or a directory "
> +                                  "containing a job file. If nothing is "
> +                                  "passed, it uses the current working "
> +                                  "directory."),
> +                            nargs="?",
> +                            default=os.getcwd())
> +
> +    def invoke(self):
> +        full_path = os.path.abspath(self.args.JOB)
> +        job_file = self.retrieve_file(full_path, JOB_FILE_EXTENSIONS)
> +
> +        self._run_job(job_file)
> +
> +    def _run_job(self, job_file):
> +        """Runs a job on the dispatcher."""
> +        from lava.job.commands import run
> +
> +        args = copy.copy(self.args)
> +        args.FILE = job_file
> +
> +        run_cmd = run(self.parser, args)
> +        run_cmd.invoke()

ditto

> +class submit(BaseCommand):
> +    """Submits a job to LAVA."""
> +
> +    @classmethod
> +    def register_arguments(cls, parser):
> +        super(submit, cls).register_arguments(parser)
> +        parser.add_argument("JOB",
> +                            help=("The job file to send, or a directory "
> +                                  "containing a job file. If nothing is "
> +                                  "passed, it uses the current working "
> +                                  "directory."),
> +                            nargs="?",
> +                            default=os.getcwd())
> +
> +    def invoke(self):
> +        full_path = os.path.abspath(self.args.JOB)
> +        job_file = self.retrieve_file(full_path, JOB_FILE_EXTENSIONS)
> +
> +
> +    def _submit_job(self, job_file):
> +        """Submits a job file to LAVA."""
> +        from lava.job.commands import submit
> +
> +        args = copy.copy(self.args)
> +        args.FILE = job_file
> +
> +        submit_cmd = submit(self.parser, args)
> +        submit_cmd.invoke()

ditto

> +class update(BaseCommand):
> +    """Updates a job file with the correct data."""
> +
> +    @classmethod
> +    def register_arguments(cls, parser):
> +        super(update, cls).register_arguments(parser)
> +        parser.add_argument("JOB",
> +                            help=("Automatically updates a job file "
> +                                  "definition. If nothing is passed, it uses"
> +                                  "the current working directory."),
> +                            nargs="?",
> +                            default=os.getcwd())
> +
> +    def invoke(self):
> +        full_path = os.path.abspath(self.args.JOB)
> +        job_file = self.retrieve_file(full_path, JOB_FILE_EXTENSIONS)
> +        job_dir = os.path.dirname(job_file)
> +        tests_dir = os.path.join(job_dir, TESTS_DIR)
> +
> +        if os.path.isdir(tests_dir):
> +            encoded_tests = TarRepoParameter.get_encoded_tar(tests_dir)
> +
> +            json_data = None
> +            with open(job_file, "r") as json_file:
> +                try:
> +                    json_data = json.load(json_file)
> +                    set_value(json_data, TESTDEF_REPOS_TAR_REPO, encoded_tests)
> +                except Exception:
> +                    raise CommandError("Cannot read job file '{0}'.".format(
> +                        job_file))
> +
> +            with open(job_file, "w") as write_file:
> +                try:
> +                    write_file.write(json.dumps(json_data, indent=4))
> +                except Exception:
> +                    raise CommandError("Cannot update job file "
> +                                       "'{0}'.".format(json_file))
> +            print >> sys.stdout, "Job definition updated."
> +        else:
> +            raise CommandError("Cannot find tests directory.")

IIUC you are assuming that you will be always storing the tarball in the job
file? My initial thinking was to always generate a tarball dinamically, so that
you don’t have to explicitly update a job file. It would go like this:

$ lava job submit FOO.json

submits FOO.json as ir

$ lava testdef submit FOO.yaml

creates the tarball and a job file dinamically; submit those

$ lava script submit FOO.sh

creates testdef, tarball and job file dynamically; submit them

> === modified file 'lava/config.py'
> --- lava/config.py	2013-07-22 09:32:25 +0000
> +++ lava/config.py	2013-07-22 09:32:26 +0000
> @@ -30,6 +30,7 @@
>      NoSectionError,
>  )
>  
> +from lava.parameter import Parameter
>  from lava.tool.errors import CommandError
>  
>  __all__ = ['Config', 'InteractiveConfig']
> @@ -54,14 +55,45 @@
>  atexit.register(_run_at_exit)
>  
>  
> +class Singleton(type):
> +    """A singleton implementation.
> +
> +    Configuration should be shared among the various call. The other approach
> +    would be to turn this into a module or a class with only static/class
> +    methods.
> +    """
> +    _instances = {}
> +
> +    def __call__(cls, *args, **kwargs):
> +        if cls not in cls._instances:
> +            cls._instances[cls] = super(Singleton, cls).__call__(*args,
> +                                                                 **kwargs)
> +        return cls._instances[cls]
> +
> +    @classmethod
> +    def _drop(cls):
> +        "Drop the instance (for testing purposes)."
> +        Singleton._instances = {}

what is the difference between using this Singleton implementation and just
instantiating a single object during initialization and exposing that object to
be used by other modules?

>  class Config(object):
>      """A generic config object."""
> -    def __init__(self):
> +    # Between all the various singleton Python patterns, the metaclass one
> +    # looks like is the one that works better with inheritance.
> +    # In Python3 this would go into the class declaration as follows:
> +    # class Config(object, metaclass=Singleton)
> +    __metaclass__ = Singleton
> +
> +    def __init__(self, config_file=None):
>          # The cache where to store parameters.
>          self._cache = {}
> -        self._config_file = (os.environ.get('LAVACONFIG') or
> -                             os.path.join(os.path.expanduser('~'),
> -                                          '.lavaconfig'))
> +        # Mostly needed for testing purposes.
> +        if config_file is not None:
> +            self._config_file = config_file
> +        else:
> +            self._config_file = (os.environ.get('LAVACONFIG') or
> +                                 os.path.join(os.path.expanduser('~'),
> +                                              '.lavaconfig'))

you could instead turn config_file into a property with lazy initialization,
then for testing you could mock that method to make it return the config data
you want.

>          self._config_backend = ConfigParser()
>          self._config_backend.read([self._config_file])
>          AT_EXIT_CALLS.add(self.save)
> @@ -92,7 +124,7 @@
>          if not section:
>              section = self._calculate_config_section(parameter)
>          # Try to get the parameter value first if it has one.
> -        if parameter.value:
> +        if parameter.value is not None:
>              value = parameter.value
>          else:
>              value = self._get_from_cache(parameter, section)
> @@ -150,6 +182,13 @@
>          if (not self._config_backend.has_section(section) and
>                  section != DEFAULT_SECTION):
>              self._config_backend.add_section(section)
> +
> +        # This is done to serialize a list when ConfigParser is written to
> +        # file. Since there is no real support for list in ConfigParser, we
> +        # serialized it in a common way that can get easily deserialized.
> +        if isinstance(value, list):
> +            value = Parameter.serialize(value)
> +
>          self._config_backend.set(section, key, value)
>          # Store in the cache too.
>          self._put_in_cache(key, value, section)
> @@ -158,6 +197,7 @@
>          """Adds a Parameter to the config file and cache.
>  
>          :param Parameter: The parameter to add.
> +        :type Parameter
>          :param value: The value of the parameter. Defaults to None.
>          :param section: The section where this parameter should be stored.
>                          Defaults to None.
> @@ -184,8 +224,8 @@
>      If a value is not found in the config file, it will ask it and then stores
>      it.
>      """
> -    def __init__(self, force_interactive=True):
> -        super(InteractiveConfig, self).__init__()
> +    def __init__(self, config_file=None, force_interactive=True):
> +        super(InteractiveConfig, self).__init__(config_file=config_file)
>          self._force_interactive = force_interactive
>  
>      def get(self, parameter, section=None):
> @@ -198,10 +238,9 @@
>              section = self._calculate_config_section(parameter)
>          value = super(InteractiveConfig, self).get(parameter, section)
>  
> -        if not (value is not None and parameter.asked):
> -            if not value or self._force_interactive:
> -                value = parameter.prompt(old_value=value)
> +        if value is None or self._force_interactive:
> +            value = parameter.prompt(old_value=value)
>  
> -        if value is not None:
> +        if value is not None and parameter.store:
>              self.put(parameter.id, value, section)
>          return value
> 
> === modified file 'lava/helper/command.py'
> --- lava/helper/command.py	2013-07-22 09:32:25 +0000
> +++ lava/helper/command.py	2013-07-22 09:32:26 +0000
> @@ -21,7 +21,7 @@
>  import os
>  import subprocess
>  import sys
> -
> +import types
>  
>  from lava.config import InteractiveConfig
>  from lava.tool.command import Command
> @@ -30,7 +30,9 @@
>  
>  
>  class BaseCommand(Command):
> +
>      """Base command class for all lava commands."""
> +
>      def __init__(self, parser, args):
>          super(BaseCommand, self).__init__(parser, args)
>          self.config = InteractiveConfig(
> @@ -59,10 +61,10 @@
>          return can_edit
>  
>      @classmethod
> -    def edit_file(cls, config_file):
> +    def edit_file(cls, file_to_edit):
>          """Opens the specified file with the default file editor.
>  
> -        :param config_file: The file to edit.
> +        :param file_to_edit: The file to edit.
>          """
>          editor = os.environ.get("EDITOR", None)
>          if editor is None:
> @@ -73,27 +75,71 @@
>              else:
>                  # We really do not know how to open a file.
>                  print >> sys.stdout, ("Cannot find an editor to open the "
> -                                      "file '{0}'.".format(config_file))
> +                                      "file '{0}'.".format(file_to_edit))
>                  print >> sys.stdout, ("Either set the 'EDITOR' environment "
>                                        "variable, or install 'sensible-editor' "
>                                        "or 'xdg-open'.")
>                  sys.exit(-1)
>          try:
> -            subprocess.Popen([editor, config_file]).wait()
> +            subprocess.Popen([editor, file_to_edit]).wait()
>          except Exception:
>              raise CommandError("Error opening the file '{0}' with the "
> -                               "following editor: {1}.".format(config_file,
> +                               "following editor: {1}.".format(file_to_edit,
>                                                                 editor))
>  
>      @classmethod
> +    def retrieve_file(cls, path, extensions):
> +        """Searches for a job file in the provided path.
> +
> +        :return The job file full path.
> +        """

it look to me that this method could be generic. If that doesn’t make sense, it
could probably be called `find_job_file` or something like that.

> +        if os.path.isfile(path):
> +            job_file = path
> +        else:
> +            for element in os.listdir(path):
> +                element_path = os.path.join(path, element)
> +                if os.path.isdir(element_path):
> +                    continue
> +                elif os.path.isfile(element_path):
> +                    job_file = os.path.split(element)[1]
> +                    # Extension here contains also the leading dot.
> +                    full_extension = os.path.splitext(job_file)[1]
> +
> +                    if full_extension:
> +                        # Make sure that we have an extension and remove the
> +                        # dot.
> +                        extension = full_extension[1:].strip().lower()
> +                        if extension in extensions:
> +                            job_file = element_path
> +                            break
> +            else:
> +                raise CommandError("No job file found in: '{0}'".format(path))
> +
> +        return job_file
> +
> +    @classmethod
> +    def verify_file_extension(cls, path, default, supported):
> +        # Checks that the file we pass has an extension or a correct one.
> +        # If not, it adds the provided one.

this comment is rather confusing

> +        full_path, file_name = os.path.split(path)
> +        name, extension = os.path.splitext(file_name)
> +        if not extension:
> +            path = ".".join([path, default])
> +        elif extension[1:] not in supported:
> +            path = os.path.join(full_path, ".".join([name, default]))
> +        return path
> +
> +    @classmethod
>      def run(cls, cmd_args):
>          """Runs the supplied command args.
>  
>          :param cmd_args: The command, and its optional arguments, to run.
>          :return The command execution return code.
>          """
> -        if not isinstance(cmd_args, list):
> +        if isinstance(cmd_args, types.StringTypes):
>              cmd_args = [cmd_args]
> +        else:
> +            cmd_args = list(cmd_args)
>          try:
>              return subprocess.check_call(cmd_args)
>          except subprocess.CalledProcessError:
> 
> === modified file 'lava/helper/template.py'
> --- lava/helper/template.py	2013-07-22 09:32:25 +0000
> +++ lava/helper/template.py	2013-07-22 09:32:26 +0000
> @@ -16,6 +16,8 @@
>  # You should have received a copy of the GNU Lesser General Public License
>  # along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
>  
> +"""Helper functions for a template."""
> +
>  from lava.parameter import Parameter
>  
>  
> @@ -42,3 +44,76 @@
>                  update(entry)
>  
>      update(template)
> +
> +
> +def get_key(data, search_key):
> +    """Goes through a template looking for a key.
> +
> +    :param data: The template to traverse.
> +    :param search_key: The key to look for.
> +    :return The key value.
> +    """
> +    return_value = None
> +
> +    if isinstance(data, dict):
> +        bucket = []
> +
> +        for key, value in data.iteritems():
> +            if key == search_key:
> +                return_value = value
> +                break
> +            else:
> +                bucket.append(value)
> +
> +        if bucket and return_value is None:
> +            for element in bucket:
> +                if isinstance(element, list):
> +                    for element in element:
> +                        bucket.append(element)
> +                elif isinstance(element, dict):
> +                    for key, value in element.iteritems():
> +                        if key == search_key:
> +                            return_value = value
> +                            break
> +                        else:
> +                            bucket.append(value)
> +
> +    return return_value
> +
> +
> +def set_value(data, search_key, new_value):
> +    """Sets a new value for a template key.
> +
> +    :param data: The data structure to update.
> +    :type dict
> +    :param search_key: The key to search and update.
> +    :param new_value: The new value to set.
> +    """
> +    is_set = False
> +
> +    if isinstance(data, dict):
> +        bucket = []
> +
> +        for key, value in data.iteritems():
> +            if key == search_key:
> +                data[key] = new_value
> +                is_set = True
> +                break
> +            else:
> +                bucket.append(value)
> +
> +        if bucket and not is_set:
> +            for element in bucket:
> +                if isinstance(element, list):
> +                    for element in element:
> +                        bucket.append(element)
> +                elif isinstance(element, dict):
> +                    for key, value in element.iteritems():
> +                        if key == search_key:
> +                            element[key] = new_value
> +                            is_set = True
> +                            break
> +                        else:
> +                            bucket.append(value)
> +                if is_set:
> +                    break
> 
> === modified file 'lava/helper/tests/helper_test.py'
> --- lava/helper/tests/helper_test.py	2013-07-22 09:32:25 +0000
> +++ lava/helper/tests/helper_test.py	2013-07-22 09:32:26 +0000
> @@ -29,12 +29,20 @@
>  import tempfile
>  
>  from unittest import TestCase
> -from mock import MagicMock
> +from mock import (
> +    MagicMock,
> +    patch
> +)
>  
>  
>  class HelperTest(TestCase):
>      """Helper test class that all tests under the lava package can inherit."""
> +
>      def setUp(self):
> +        # Need to patch it here, not as a decorator, or running the tests
> +        # via `./setup.py test` will fail.
> +        self.at_exit_patcher = patch("lava.config.AT_EXIT_CALLS", spec=set)
> +        self.at_exit_patcher.start()
>          self.original_stdout = sys.stdout
>          sys.stdout = open("/dev/null", "w")
>          self.original_stderr = sys.stderr
> @@ -50,12 +58,18 @@
>          self.args.interactive = MagicMock(return_value=False)
>          self.args.DEVICE = self.device
>  
> -        self.config = MagicMock()
> -        self.config.get = MagicMock(return_value=self.temp_dir)
> -
>      def tearDown(self):
> +        self.at_exit_patcher.stop()
>          sys.stdin = self.original_stdin
>          sys.stdout = self.original_stdout
>          sys.stderr = self.original_stderr
>          shutil.rmtree(self.temp_dir)
>          os.unlink(self.temp_file.name)
> +
> +    def tmp(self, name):
> +        """Returns a path to a non existent file/directory.

A method called `tmp` that returns a non existing file is rather confusing. Can
we come up with a better name? Maybe the method name is correct but the
docstring needs to say something less confusing.

What about

"""
Returns the full path to a file called `name` in a temporary directory. This
method does not create the file, it only gives a full filename where you can
actually write some data.
"""

> +
> +        :param name: The name the file/directory should have.
> +        :return A path.
> +        """
> +        return os.path.join(tempfile.gettempdir(), name)
> 
> === modified file 'lava/job/commands.py'
> --- lava/job/commands.py	2013-07-22 09:32:25 +0000
> +++ lava/job/commands.py	2013-07-22 09:32:26 +0000
> @@ -26,18 +26,36 @@
>  
>  from lava.helper.command import BaseCommand
>  from lava.helper.dispatcher import get_devices
> -
> +from lava.helper.template import get_key
>  from lava.job import Job
>  from lava.job.templates import (
> -    BOOT_TEST,
> -)
> -from lava.parameter import Parameter
> +    LAVA_TEST_SHELL,
> +    TESTDEF_REPOS_TAR_REPO,
> +)
> +from lava.parameter import (
> +    ListParameter,
> +    Parameter,
> +    SingleChoiceParameter,
> +)
>  from lava.tool.command import CommandGroup
>  from lava.tool.errors import CommandError
>  from lava_tool.authtoken import AuthenticatingServerProxy, KeyringAuthBackend
>  from lava_tool.utils import has_command
>  
>  
> +# Name of the config value to store the job ids.
> +JOBS_ID = "jobs_id"
> +# Name of the config value to store the LAVA server URL.
> +SERVER = "server"
> +# Name of the config value to store the LAVA rpc_endpoint.
> +RPC_ENDPOINT = "rpc_endpoint"
> +
> +# Default job file extension.
> +DEFAULT_EXTENSION = "json"
> +# Possible extension for a job file.
> +JOB_FILE_EXTENSIONS = [DEFAULT_EXTENSION]
> +
> +
>  class job(CommandGroup):
>      """LAVA job file handling."""
>      namespace = 'lava.job.commands'
> @@ -51,19 +69,31 @@
>          super(new, cls).register_arguments(parser)
>          parser.add_argument("FILE", help=("Job file to be created."))
>  
> -    def invoke(self):
> -        if os.path.exists(self.args.FILE):
> -            raise CommandError('{0} already exists.'.format(self.args.FILE))
> -
> -        with open(self.args.FILE, 'w') as job_file:
> -            job_instance = Job(BOOT_TEST)
> +    def invoke(self, tests_dir=None):
> +        full_path = os.path.abspath(self.args.FILE)
> +        job_file = self.verify_file_extension(full_path, DEFAULT_EXTENSION,
> +                                              JOB_FILE_EXTENSIONS)
> +
> +        if os.path.exists(job_file):
> +            raise CommandError('{0} already exists.'.format(job_file))
> +
> +        job_instance = Job(LAVA_TEST_SHELL)
> +        if tests_dir:
> +            # TODO: find a better way to retrieve a key.
> +            testdef_tar_repo = get_key(TESTDEF_REPOS_TAR_REPO)
> +            testdef_tar_repo.set(tests_dir)
> +            testdef_tar_repo.asked = True
> +
> +        with open(job_file, 'w') as write_file:
>              job_instance.fill_in(self.config)
> -            job_instance.write(job_file)
> +            job_instance.write(write_file)
>  
>  
>  class submit(BaseCommand):
>      """Submits the specified job file."""
>  
> +    JOBS_ID = "jobs_id"
> +
>      @classmethod
>      def register_arguments(cls, parser):
>          super(submit, cls).register_arguments(parser)
> @@ -73,16 +103,31 @@
>          jobfile = self.args.FILE
>          jobdata = open(jobfile, 'rb').read()
>  
> -        server_name = Parameter('server')
> -        rpc_endpoint = Parameter('rpc_endpoint', depends=server_name)
> -        self.config.get(server_name)
> -        endpoint = self.config.get(rpc_endpoint)
> +        server_name_parameter = Parameter(SERVER)
> +        rpc_endpoint_parameter = Parameter(RPC_ENDPOINT,
> +                                           depends=server_name_parameter)
> +        self.config.get(server_name_parameter)
> +        endpoint = self.config.get(rpc_endpoint_parameter)
>  
>          server = AuthenticatingServerProxy(endpoint,
>                                             auth_backend=KeyringAuthBackend())
>          try:
>              job_id = server.scheduler.submit_job(jobdata)
>              print >> sys.stdout, "Job submitted with job ID {0}".format(job_id)
> +
> +            # Store the job_id into the config file.
> +            if job_id:
> +                # We need first to take out the old values, and then store the
> +                # new one.
> +                job_id_parameter = ListParameter(JOBS_ID)
> +                job_id_parameter.asked = True
> +
> +                value = self.config.get(job_id_parameter)
> +                if value:
> +                    job_id_parameter.set(value)
> +
> +                job_id_parameter.add(job_id)
> +                self.config.put_parameter(job_id_parameter)
>          except xmlrpclib.Fault, exc:
>              raise CommandError(str(exc))
>

I don't think we should keep the list of submitted jobs in the config file.
Think of someone that wants to keep their lava config file under version
control (I do that for most of all my config files); that user won't want
lava-too to change the config file for every job submission.

Instead, I think the list of job ids (and eventually some extra data about each
job) is runtime data, and as such should be kept under ~/.cache/lava/jobs or
something like that

> @@ -95,42 +140,16 @@
>          super(run, cls).register_arguments(parser)
>          parser.add_argument("FILE", help=("The job file to submit."))
>  
> -    @classmethod
> -    def _choose_device(cls, devices):
> -        """Let the user choose the device to use.
> -
> -        :param devices: The list of available devices.
> -        :return The selected device.
> -        """
> -        devices_len = len(devices)
> -        output_list = []
> -        for device, number in zip(devices, range(1, devices_len + 1)):
> -            output_list.append("\t{0}. {1}\n".format(number, device.hostname))
> -
> -        print >> sys.stdout, ("More than one local device found. "
> -                              "Please choose one:\n")
> -        print >> sys.stdout, "".join(output_list)
> -
> -        while True:
> -            try:
> -                user_input = raw_input("Device number to use: ").strip()
> -
> -                if user_input in [str(x) for x in range(1, devices_len + 1)]:
> -                    return devices[int(user_input) - 1].hostname
> -                else:
> -                    continue
> -            except EOFError:
> -                user_input = None
> -            except KeyboardInterrupt:
> -                sys.exit(-1)
> -
>      def invoke(self):
>          if os.path.isfile(self.args.FILE):
>              if has_command("lava-dispatch"):
>                  devices = get_devices()
>                  if devices:
>                      if len(devices) > 1:
> -                        device = self._choose_device(devices)
> +                        device_names = [device.hostname for device in devices]
> +                        device_param = SingleChoiceParameter("device",
> +                                                             device_names)
> +                        device = device_param.prompt("Device to use: ")
>                      else:
>                          device = devices[0].hostname
>                      self.run(["lava-dispatch", "--target", device,
> @@ -138,5 +157,5 @@
>              else:
>                  raise CommandError("Cannot find lava-dispatcher installation.")
>          else:
> -            raise CommandError("The file '{0}' does not exists. or is not "
> +            raise CommandError("The file '{0}' does not exists, or it is not "
>                                 "a file.".format(self.args.FILE))
> 
> === modified file 'lava/job/templates.py'
> --- lava/job/templates.py	2013-07-22 09:32:25 +0000
> +++ lava/job/templates.py	2013-07-22 09:32:26 +0000
> @@ -16,44 +16,78 @@
>  # You should have received a copy of the GNU Lesser General Public License
>  # along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
>  
> -from lava.parameter import Parameter
> -
> -device_type = Parameter("device_type")
> -prebuilt_image = Parameter("prebuilt_image", depends=device_type)
> +from lava.parameter import (
> +    Parameter,
> +    TarRepoParameter,
> +)
> +
> +ACTIONS_ID = "actions"
> +COMMAND_ID = "command"
> +IMAGE_ID = "image"
> +PARAMETERS_ID = "parameters"
> +DEVICE_TYPE_ID = "device_type"
> +TESTDEF_REPOS_ID = "testdef_repos"
> +TESTDEF_REPOS_TAR_REPO = "tar-repo"
> +STREAM_ID = "stream"
> +SERVER_ID = "server"

I don’t think this many constants are usefull. I would rather expect the actual
template data to be as close to the actual data as possible and believe that
will make their maintaince easier.

> +
> +DEVICE_TYPE_PARAMETER = Parameter(DEVICE_TYPE_ID)
> +PREBUILT_IMAGE_PARAMETER = Parameter(IMAGE_ID, depends=DEVICE_TYPE_PARAMETER)
> +
> +# Never store the testdef_urls parameter in the config file.
> +TESTDEF_URL_PARAMETER = TarRepoParameter(TESTDEF_REPOS_TAR_REPO)
> +TESTDEF_URL_PARAMETER.store = False
> +
> +# Use another ID for the server parameter, might be different.
> +SERVER_PARAMETER = Parameter("stream_server")
> +STREAM_PARAMETER = Parameter(STREAM_ID)
>  
>  BOOT_TEST = {
> +    "timeout": 18000,
>      "job_name": "Boot test",
> -    "device_type": device_type,
> -    "actions": [
> +    DEVICE_TYPE_ID: DEVICE_TYPE_PARAMETER,
> +    ACTIONS_ID: [
>          {
> -            "command": "deploy_linaro_image",
> -            "parameters": {
> -                "image": prebuilt_image
> +            COMMAND_ID: "deploy_linaro_image",
> +            PARAMETERS_ID: {
> +                "image": PREBUILT_IMAGE_PARAMETER
>              }
>          },
>          {
> -            "command": "boot_linaro_image"
> +            COMMAND_ID: "boot_linaro_image"
>          }
>      ]
>  }
>  
>  LAVA_TEST_SHELL = {
>      "job_name": "LAVA Test Shell",
> -    "device_type": device_type,
> -    "actions": [
> +    "timeout": 18000,
> +    DEVICE_TYPE_ID: DEVICE_TYPE_PARAMETER,
> +    ACTIONS_ID: [
>          {
> -            "command": "deploy_linaro_image",
> -            "parameters": {
> -                "image": prebuilt_image,
> +            COMMAND_ID: "deploy_linaro_image",
> +            PARAMETERS_ID: {
> +                IMAGE_ID: PREBUILT_IMAGE_PARAMETER,
>              }
>          },
>          {
> -            "command": "lava_test_shell",
> -            "parameters": {
> -                "testdef_urls": [
> -                    Parameter("testdef_url")
> +            COMMAND_ID: "lava_test_shell",
> +            PARAMETERS_ID: {
> +                "timeout": 1800,
> +                TESTDEF_REPOS_ID: [
> +                        {
> +                            "testdef": "lavatest.yaml",
> +                            TESTDEF_REPOS_TAR_REPO: TESTDEF_URL_PARAMETER
> +                        }
>                  ]
>              }
> +        },
> +        {
> +            COMMAND_ID: "submit_results",
> +            PARAMETERS_ID : {
> +                STREAM_ID: STREAM_PARAMETER,
> +                SERVER_ID: SERVER_PARAMETER
> +            }
>          }
>      ]
>  }
> 
> === modified file 'lava/parameter.py'
> --- lava/parameter.py	2013-07-22 09:32:25 +0000
> +++ lava/parameter.py	2013-07-22 09:32:26 +0000
> @@ -20,7 +20,19 @@
>  Parameter class and its accessory methods/functions.
>  """
>  
> +import StringIO
> +import base64
> +import os
>  import sys
> +import tarfile
> +import tempfile
> +import types
> +import urlparse
> +
> +from lava.tool.errors import CommandError
> +
> +# Character used to join serialized list parameters.
> +LIST_SERIALIZE_DELIMITER = ","
>  
>  
>  class Parameter(object):
> @@ -38,6 +50,15 @@
>          self.value = value
>          self.depends = depends
>          self.asked = False
> +        # Whether to store or not the parameter in the user config file.
> +        self.store = True
> +
> +    def set(self, value):
> +        """Sets the value of the parameter.
> +
> +        :param value: The value to set.
> +        """
> +        self.value = value
>  
>      def prompt(self, old_value=None):
>          """Gets the parameter value from the user.
> @@ -50,26 +71,287 @@
>          :param old_value: The old parameter value.
>          :return The input as typed by the user, or the old value.
>          """
> -        if old_value is not None:
> -            prompt = "{0} [{1}]: ".format(self.id, old_value)
> -        else:
> -            prompt = "{0}: ".format(self.id)
> -
> -        user_input = None
> +        if not self.asked:
> +            if old_value is not None:
> +                prompt = "{0} [{1}]: ".format(self.id, old_value)
> +            else:
> +                prompt = "{0}: ".format(self.id)
> +
> +            user_input = self.get_user_input(prompt)
> +
> +            if user_input is not None:
> +                if len(user_input) == 0 and old_value:
> +                    # Keep the old value when user press enter or another
> +                    # whitespace char.
> +                    self.value = old_value
> +                else:
> +                    self.value = user_input
> +
> +            self.asked = True
> +
> +        return self.value
> +
> +    @classmethod
> +    def get_user_input(cls, prompt=""):
> +        """Asks the user for input data.
> +
> +        :param prompt: The prompt that should be given to the user.
> +        :return A string with what the user typed.
> +        """
> +        data = None
>          try:
> -            user_input = raw_input(prompt).strip()
> +            data = raw_input(prompt).strip()
>          except EOFError:
> -            pass
> +            # Force to return None.
> +            data = None
>          except KeyboardInterrupt:
>              sys.exit(-1)
> -
> -        if user_input is not None:
> +        return data
> +
> +    @classmethod
> +    def serialize(cls, value):
> +        """Serializes the passed value to be friendly written to file.
> +
> +        Lists are serialized as a comma separated string of values.

I already saw something about serializing lists above. Can we keep all this
list serialization/deserialization just here?

> +
> +        :param value: The value to serialize.
> +        :return The serialized value as string.
> +        """
> +        serialized = ""
> +        if isinstance(value, list):
> +            serialized = LIST_SERIALIZE_DELIMITER.join(
> +                str(x) for x in value if x)
> +        else:
> +            serialized = str(value)
> +        return serialized

I wonder if we can we use a existing serialization instead of using a arbitrary
delimiter ...  e.g. json + base64, this way we don’t have to limit ourselves to
values that do not contain a comma. But OTOH we want something that is readable
(and editable!) in the config file ...

> +    @classmethod
> +    def deserialize(cls, value):
> +        """Deserialize a value into a list.
> +
> +        The value must have been serialized with the class instance serialize()
> +        method.
> +
> +        :param value: The string value to be deserialized.
> +        :type str
> +        :return A list of values.
> +        """
> +        deserialized = []
> +        if isinstance(value, types.StringTypes):
> +            deserialized = filter(None, (x.strip() for x in value.split(
> +                LIST_SERIALIZE_DELIMITER)))
> +        else:
> +            deserialized = list(value)
> +        return deserialized
> +
> +    @classmethod
> +    def to_list(cls, value):
> +        """Return a list from the passed value.
> +
> +        :param value: The parameter to turn into a list.
> +        """
> +        return_value = []
> +        if isinstance(value, types.StringType):
> +            return_value = [value]
> +        else:
> +            return_value = list(value)
> +        return return_value
> +
> +
> +class SingleChoiceParameter(Parameter):
> +    """A parameter implemeting a single choice between multiple choices."""
> +    def __init__(self, id, choices):
> +        super(SingleChoiceParameter, self).__init__(id)
> +        self.choices = self.to_list(choices)
> +
> +    def prompt(self, prompt, old_value=None):
> +        """Asks the user for their choice."""
> +        # Sliglty different than the other parameters: here we first present
> +        # the user with what the choices are about.
> +        print >> sys.stdout, prompt
> +
> +        index = 1
> +        for choice in self.choices:
> +            print >> sys.stdout, "\t{0:d}. {1}".format(index, choice)
> +            index += 1
> +
> +        choices_len = len(self.choices)
> +        while True:
> +            user_input = self.get_user_input("Choice: ")
> +
>              if len(user_input) == 0 and old_value:
> -                # Keep the old value when user press enter or another
> -                # whitespace char.
> -                self.value = old_value
> -            else:
> -                self.value = user_input
> -
> -        self.asked = True
> -        return self.value
> +                choice = old_value
> +                break
> +            elif user_input in [str(x) for x in range(1, choices_len + 1)]:
> +                choice = self.choices[int(user_input) - 1]
> +                break
> +
> +        return choice
> +
> +
> +class UrlSchemeParameter(SingleChoiceParameter):
> +    """A parameter to choose the URL scheme to use."""
> +    # The supported URL schemes:
> +    #   file: normal file URL.
> +    #   data: base64 encoded string of the file pointed to and its path.
> +    FILE_SCHEME = "file"
> +    DATA_SCHEME = "data"
> +
> +    SCHEME_CHOICES = [
> +        FILE_SCHEME,
> +        DATA_SCHEME,
> +    ]
> +
> +    def __init__(self, value=None, id="url_scheme"):
> +        super(UrlSchemeParameter, self).__init__(id, self.SCHEME_CHOICES)
> +        self.value = value
> +
> +    def prompt(self, prompt="", old_value=None):
> +        if not self.asked:
> +            if old_value is not None:
> +                prompt = "\nURL scheme [was: {0}]:".format(old_value)
> +            else:
> +                prompt = "\nURL scheme:"
> +            self.value = super(UrlSchemeParameter, self).prompt(
> +                prompt=prompt, old_value=old_value)
> +            self.asked = True
> +
> +        return self.value

As far as this MP is concerned this class is not used (apart from its own unit
tests). This is the one you mentioned in the introduction right?

I can't see the point of a URL scheme parameter, I think it's easier to just
ask for the URL itself and then deduce the scheme from there. See also my
comment above about the tarball (i.e. you should never need to inform the
tarball data manually)

> +class ListParameter(Parameter):
> +    """A specialized Parameter to handle list values."""
> +
> +    # This is used as a deletion character. When we have an old value and the
> +    # user enters this char, it sort of deletes the value.
> +    DELETE_CHAR = "-"
> +
> +    def __init__(self, id, value=None, depends=None):
> +        super(ListParameter, self).__init__(id, depends=depends)
> +        self.value = []
> +        if value:
> +            self.set(value)
> +
> +    def set(self, value):
> +        """Sets the value of the parameter.
> +
> +        :param value: The value to set.
> +        """
> +        self.value = self.to_list(value)
> +
> +    def add(self, value):
> +        """Adds a new value to the list of values of this parameter.
> +
> +        :param value: The value to add.
> +        """
> +        if isinstance(value, list):
> +            self.value.extend(value)
> +        else:
> +            self.value.append(value)
> +
> +    def prompt(self, old_value=None):
> +        """Gets the parameter in a list form.
> +
> +        To exit the input procedure it is necessary to insert an empty line.
> +
> +        :return The list of values.
> +        """
> +
> +        if not self.asked:
> +            if old_value is not None:
> +                # We might get the old value read from file via ConfigParser,
> +                # and usually it comes in string format.
> +                old_value = self.deserialize(old_value)
> +
> +            print >> sys.stdout, "Values for '{0}': ".format(self.id)
> +
> +            index = 1
> +            while True:
> +                user_input = None
> +                if old_value is not None and (0 < len(old_value) >= index):
> +                    prompt = "{0:>3d}.\n\told: {1}\n\tnew: ".format(
> +                        index, old_value[index-1])
> +                    user_input = self.get_user_input(prompt)
> +                else:
> +                    prompt = "{0:>3d}. ".format(index)
> +                    user_input = self.get_user_input(prompt)
> +
> +                if user_input is not None:
> +                    # The user has pressed Enter.
> +                    if len(user_input) == 0:
> +                        if old_value is not None and \
> +                                (0 < len(old_value) >= index):
> +                            user_input = old_value[index-1]
> +                        else:
> +                            break
> +
> +                    if len(user_input) == 1 and user_input == \
> +                            self.DELETE_CHAR and (0 < len(old_value) >= index):
> +                        # We have an old value, user presses the DELETE_CHAR
> +                        # and we do not store anything. This is done to delete
> +                        # an old entry.
> +                        pass
> +                    else:
> +                        self.value.append(user_input)
> +                    index += 1
> +
> +            self.asked = True
> +
> +        return self.value
> +
> +
> +class TarRepoParameter(Parameter):
> +    def __init__(self, id="tar_repo_dir", value=None, depends=None):
> +        super(TarRepoParameter, self).__init__(id, value=value,
> +                                               depends=depends)
> +
> +    @classmethod
> +    def get_encoded_tar(cls, paths):
> +        """Encodes in base64 the provided path.
> +
> +        The econding will create a temporary tar file, read the content of
> +        the tarball and encode it in base64. At the end the temporary file
> +        will be deleted.
> +
> +        :param paths: Paths that will be added to the temporary tar file.
> +        :return The encoded content of the tar file.
> +        """
> +        paths = cls.to_list(paths)
> +        try:
> +            temp_tar_file = tempfile.NamedTemporaryFile(suffix=".tar",
> +                                                        delete=False)
> +            with tarfile.open(temp_tar_file.name, "w") as tar_file:
> +                for path in paths:
> +                    full_path = os.path.abspath(path)
> +                    if os.path.isfile(full_path):
> +                        arcname = os.path.basename(full_path)
> +                        tar_file.add(full_path, arcname=arcname)
> +                    elif os.path.isdir(full_path):
> +                        # If we pass a directory, flatten it out.
> +                        # List its content, and add them as they are.
> +                        for element in os.listdir(full_path):
> +                            arcname = element
> +                            tar_file.add(os.path.join(full_path, element),
> +                                         arcname=arcname)
> +
> +            encoded_content = StringIO.StringIO()
> +
> +            if os.path.isfile(temp_tar_file.name):
> +                try:
> +                    with open(temp_tar_file.name) as read_file:
> +                        base64.encode(read_file, encoded_content)
> +
> +                    return encoded_content.getvalue().strip()
> +                except IOError:
> +                    raise CommandError("Cannot read file "
> +                                       "'{0}'.".format(temp_tar_file.name))
> +            else:
> +                raise CommandError("Provided path does not exists: "
> +                                   "{0}.".format(temp_tar_file.name))
> +        finally:
> +            if os.path.isfile(temp_tar_file.name):
> +                os.unlink(temp_tar_file.name)
> +
> +    def prompt(self, old_value=None):
> +        super(TarRepoParameter, self).prompt(old_value=old_value)
> +        return self.get_encoded_tar(self.value)

As I commented above, this TarRepoParameter class should not be needed (i.e.
users should not need to deal with tarballs themselves)

> === added file 'lava/testdef/commands.py'
> --- lava/testdef/commands.py	1970-01-01 00:00:00 +0000
> +++ lava/testdef/commands.py	2013-07-22 09:32:26 +0000
> @@ -0,0 +1,93 @@
> +# Copyright (C) 2013 Linaro Limited
> +#
> +# Author: Milo Casagrande <milo.casagrande@linaro.org>
> +#
> +# This file is part of lava-tool.
> +#
> +# lava-tool is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU Lesser General Public License version 3
> +# as published by the Free Software Foundation
> +#
> +# lava-tool is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public License
> +# along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
> +
> +"""
> +Test definition commands class.
> +"""
> +
> +import os
> +
> +from lava.helper.command import BaseCommand
> +from lava.testdef import TestDefinition
> +from lava.testdef.templates import TESTDEF_TEMPLATE
> +from lava.tool.command import CommandGroup
> +from lava.tool.errors import CommandError
> +
> +
> +# Default test def file extension.
> +DEFAULT_TEST_EXTENSION = "yaml"
> +# Possible extensions for a test def file.
> +TEST_FILE_EXTENSIONS = [DEFAULT_TEST_EXTENSION]
> +
> +
> +class testdef(CommandGroup):
> +
> +    """LAVA test definitions handling."""
> +
> +    namespace = "lava.testdef.commands"
> +
> +
> +class new(BaseCommand):
> +
> +    """Creates a new test definition file."""
> +
> +    @classmethod
> +    def register_arguments(cls, parser):
> +        super(new, cls).register_arguments(parser)
> +        parser.add_argument("FILE", help="Test definition file to create.")
> +
> +    def invoke(self):
> +        full_path = os.path.abspath(self.args.FILE)
> +        testdef_file = self.verify_file_extension(full_path,
> +                                                  DEFAULT_TEST_EXTENSION,
> +                                                  TEST_FILE_EXTENSIONS)
> +        if os.path.exists(testdef_file):
> +            raise CommandError("Test definition file '{0}' already "
> +                               "exists.".format(self.args.FILE))
> +        try:
> +            testdef = TestDefinition(testdef_file, TESTDEF_TEMPLATE)
> +            testdef.update(self.config)
> +            testdef.write()
> +        except IOError:
> +            raise CommandError("Cannot write file '{0}': permission "
> +                               "denied".format(self.args.FILE))

I don't think that insuficient permissions are the only possible cause of an
IOError here ...

> +
> +
> +class run(BaseCommand):
> +
> +    """Runs the specified test definition on a local device."""
> +
> +    @classmethod
> +    def register_arguments(cls, parser):
> +        super(run, cls).register_arguments(parser)
> +        parser.add_argument("FILE", help="Test definition file to run.")
> +
> +    def invoke(self):
> +        pass

missing a FIXME/TODO comment? (I realize that this is probably coming in the
                               next MP though :))

> +def submit(BaseCommand):
> +    """Submits the specified test definition to a remove LAVA server."""
> +
> +    @classmethod
> +    def register_arguments(cls, parser):
> +        super(submit, cls).register_arguments(parser)
> +        parser.add_argument("FILE", help="Test definition file to send.")
> +
> +    def invoke(self):
> +        pass

ditto

> === added file 'lava/testdef/templates.py'
> --- lava/testdef/templates.py	1970-01-01 00:00:00 +0000
> +++ lava/testdef/templates.py	2013-07-22 09:32:26 +0000
> @@ -0,0 +1,75 @@
> +# Copyright (C) 2013 Linaro Limited
> +#
> +# Author: Milo Casagrande <milo.casagrande@linaro.org>
> +#
> +# This file is part of lava-tool.
> +#
> +# lava-tool is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU Lesser General Public License version 3
> +# as published by the Free Software Foundation
> +#
> +# lava-tool is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public License
> +# along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
> +
> +"""Test definition templates."""
> +
> +from lava.parameter import (
> +    ListParameter,
> +    Parameter,
> +)
> +
> +DEFAULT_TESTDEF_FILE = "lavatest.yaml"
> +
> +DEFAULT_TESTDEF_VERSION = "1.0"
> +DEFAULT_TESTDEF_FORMAT = "Lava-Test Test Definition 1.0"
> +
> +# This is what will be called by default by the test definition yaml file.
> +DEFAULT_TESTDEF_SCRIPT = "mytest.sh"
> +DEFAULT_TESTDEF_SCRIPT_CONTENT = """#!/bin/sh
> +# Automatic generated content by lava-tool.
> +# Please add your own instructions.
> +"""

It would be nice to include some commented examples of the lava-test-shell API
usage ...

> +DEFAULT_TESTDEF_STEP = "./mytest.sh"
> +
> +DEFAULT_ENVIRONMET_VALUE = "lava_test_shell"
> +
> +# All these parameters will not be stored on the local config file.
> +NAME_PARAMETER = Parameter("name")
> +NAME_PARAMETER.store = False
> +
> +DESCRIPTION_PARAMETER = Parameter("description", depends=NAME_PARAMETER)
> +DESCRIPTION_PARAMETER.store = False
> +
> +ENVIRONMENT_PARAMETER = ListParameter("environment",
> +                                      depends=NAME_PARAMETER)
> +ENVIRONMENT_PARAMETER.add(DEFAULT_ENVIRONMET_VALUE)
> +ENVIRONMENT_PARAMETER.asked = True
> +ENVIRONMENT_PARAMETER.store = False
> +
> +# Steps parameter. Default to a local shell script that the user defines.
> +# We do not ask this parameter, and we do not store it either.
> +STEPS_PARAMETER = ListParameter("steps", depends=NAME_PARAMETER)
> +STEPS_PARAMETER.add(DEFAULT_TESTDEF_STEP)
> +STEPS_PARAMETER.asked = True
> +STEPS_PARAMETER.store = False
> +
> +TESTDEF_TEMPLATE = {
> +    "metadata": {
> +        "name": NAME_PARAMETER,
> +        "format": DEFAULT_TESTDEF_FORMAT,
> +        "version": DEFAULT_TESTDEF_VERSION,
> +        "description": DESCRIPTION_PARAMETER,
> +        "environment": ENVIRONMENT_PARAMETER,
> +    },
> +    "run": {
> +        "steps": STEPS_PARAMETER,
> +    },
> +    "parse": {
> +        "pattern": "^\\s*(?P<test_case_id>\\w+)=(?P<result>\\w+)\\s*$"

use single quotes to avoid escaping hell?

maybe you also want to include some examples of patterns commented-out ...
(yay for YAML actually supporting comments)

> +    }
> +}
>

Revision history for this message

Milo Casagrande (milo) wrote on 2013-07-24:

Download full text (12.0 KiB)

On Wed, Jul 24, 2013 at 2:49 AM, Antonio Terceiro
<email address hidden> wrote:
>> +
>> +INIT_TEMPLATE = {
>> + JOBFILE_ID: JOBFILE_PARAMETER,
>> +}
>
> Do we really need a dictionary with a single key?

If we want to maintain the same pattern as for the other commands,
yes: go through the template, look up the parameter in the config, and
ask for the values there.
Or we need to change the "expand_template" function signature and also
the name to accept single Parameter objects.

>> + def _create_files(self, data, full_path, test_path):
>> + # This is the default script file as defined in the testdef template.
>> + default_script = os.path.join(test_path, DEFAULT_TESTDEF_SCRIPT)
>> +
>> + if not os.path.isfile(default_script):
>> + # We do not have the default testdef script. Create it, but
>> + # remind the user to update it.
>> + print >> sys.stdout, ("\nCreating default test script "
>> + "'{0}'.".format(DEFAULT_TESTDEF_SCRIPT))
>> +
>> + with open(default_script, "w") as write_file:
>> + write_file.write(DEFAULT_TESTDEF_SCRIPT_CONTENT)
>
> if this behaviour of "create a file with this content here" starts to repeat,
> we probably want to encapsulatet it somewhere else (I noted it at least once in
> other parts of this MP).

Sounds good, even if at the moment should be only used here.
It was used also in the tests to fill up files with fake content.

>> + # Prompt the user to write the script file.
>> + self.edit_file(default_script)
>> +
>> + # Make sure the script is executable.
>> + os.chmod(default_script,
>> + stat.S_IRWXU | stat.S_IRWXG | stat.S_IROTH | stat.S_IXOTH)
>> +
>> + print >> sys.stdout, ("\nCreating test definition "
>> + "'{0}':".format(DEFAULT_TESTDEF_FILE))
>
> print already writes to stdout by default. is it really necessary to be that explicit?

I used the same behavior found in most of lava-tool (in particular in
lava_dashboard_tool) where it is using the explicit redirect.
Albeit, there is a mixed use of it even in the old code.

> I’m not sure about this approach of calling other commands. IMO the best
> approach would be to concentrate as much of the logic as possible in the
> model/backend classes, them call them from both commands.

Some refactoring will be needed, not sure if in this merge proposal though.

> IIUC you are assuming that you will be always storing the tarball in the job
> file?

True only if you go through the "lava init" step.
If you want to use the other commands, this does not happen. It might
need some refactoring since "testdef|script submit" is still missing
though.

> My initial thinking was to always generate a tarball dinamically, so that
> you don’t have to explicitly update a job file. It would go like this:
>
> $ lava job submit FOO.json
>
> submits FOO.json as ir

This works.

> $ lava testdef submit FOO.yaml
>
> creates the tarball and a job file dinamically; submit those
>
> $ lava script submit FOO.sh
>
> creates testdef, tarball and job file dynamically; submit th...

On Wed, Jul 24, 2013 at 2:49 AM, Antonio Terceiro
<antonio.terceiro@linaro.org> wrote:
>> +
>> +INIT_TEMPLATE = {
>> +    JOBFILE_ID: JOBFILE_PARAMETER,
>> +}
>
> Do we really need a dictionary with a single key?

>> +    def _create_files(self, data, full_path, test_path):
>> +        # This is the default script file as defined in the testdef template.
>> +        default_script = os.path.join(test_path, DEFAULT_TESTDEF_SCRIPT)
>> +
>> +        if not os.path.isfile(default_script):
>> +            # We do not have the default testdef script. Create it, but
>> +            # remind the user to update it.
>> +            print >> sys.stdout, ("\nCreating default test script "
>> +                                  "'{0}'.".format(DEFAULT_TESTDEF_SCRIPT))
>> +
>> +            with open(default_script, "w") as write_file:
>> +                write_file.write(DEFAULT_TESTDEF_SCRIPT_CONTENT)
>
> if this behaviour of "create a file with this content here" starts to repeat,
> we probably want to encapsulatet it somewhere else (I noted it at least once in
> other parts of this MP).

Sounds good, even if at the moment should be only used here.
It was used also in the tests to fill up files with fake content.

>> +            # Prompt the user to write the script file.
>> +            self.edit_file(default_script)
>> +
>> +        # Make sure the script is executable.
>> +        os.chmod(default_script,
>> +                 stat.S_IRWXU | stat.S_IRWXG | stat.S_IROTH | stat.S_IXOTH)
>> +
>> +        print >> sys.stdout, ("\nCreating test definition "
>> +                              "'{0}':".format(DEFAULT_TESTDEF_FILE))
>
> print already writes to stdout by default. is it really necessary to be that explicit?

I used the same behavior found in most of lava-tool (in particular in
lava_dashboard_tool) where it is using the explicit redirect.
Albeit, there is a mixed use of it even in the old code.

Some refactoring will be needed, not sure if in this merge proposal though.

> IIUC you are assuming that you will be always storing the tarball in the job
> file?

True only if you go through the "lava init" step.
If you want to use the other commands, this does not happen. It might
need some refactoring since "testdef|script submit" is still missing
though.

This works.

> $ lava testdef submit FOO.yaml
>
>   creates the tarball and a job file dinamically; submit those
>
> $ lava script submit FOO.sh
>
>   creates testdef, tarball and job file dynamically; submit them

These two are still missing.

After showing this at Connect though, the common patterns were:
- I will not use this (mostly from QA or people that knows how to write testdef)
- There are too many commands (from a couple of Linaro employees and
ARM ones too, hence the "lava init" stuff that will do everything
almost automatically).

> what is the difference between using this Singleton implementation and just
> instantiating a single object during initialization and exposing that object to
> be used by other modules?

The Config object is instantiated in the BaseCommand class, class that
all the other commands inherits. If I turn that into a class
attribute, or even a module one:

class BaseCommand():

CONFIG = InteractiveConfig( ... )
 __init__():
  self.config = self.CONFIG

I need to change how the interactiviness of the Config class is set,
since I cannot control it directly anymore. I can add a
"set_force_interactive" method for that and call it from the
BaseCommand class.
It was mostly used when calling other commands from another one, since
I needed the same Config object shared between them. Changing that
behavior, this will hopefully not be needed anymore.

>> +        # Mostly needed for testing purposes.
>> +        if config_file is not None:
>> +            self._config_file = config_file
>> +        else:
>> +            self._config_file = (os.environ.get('LAVACONFIG') or
>> +                                 os.path.join(os.path.expanduser('~'),
>> +                                              '.lavaconfig'))
>
> you could instead turn config_file into a property with lazy initialization,
> then for testing you could mock that method to make it return the config data
> you want.

Sounds good.

>>      @classmethod
>> +    def retrieve_file(cls, path, extensions):
>> +        """Searches for a job file in the provided path.
>> +
>> +        :return The job file full path.
>> +        """
>
> it look to me that this method could be generic. If that doesn’t make sense, it
> could probably be called `find_job_file` or something like that.

It is generic, it's the doc string that is misleading.

>> +    @classmethod
>> +    def verify_file_extension(cls, path, default, supported):
>> +        # Checks that the file we pass has an extension or a correct one.
>> +        # If not, it adds the provided one.
>
> this comment is rather confusing

Fixed now, turned it into a doc string also.

>> +
>> +    def tmp(self, name):
>> +        """Returns a path to a non existent file/directory.
>
> A method called `tmp` that returns a non existing file is rather confusing. Can
> we come up with a better name? Maybe the method name is correct but the
> docstring needs to say something less confusing.
>
> What about
>
> """
> Returns the full path to a file called `name` in a temporary directory. This
> method does not create the file, it only gives a full filename where you can
> actually write some data.
> """

> I don't think we should keep the list of submitted jobs in the config file.
> Think of someone that wants to keep their lava config file under version
> control (I do that for most of all my config files); that user won't want
> lava-too to change the config file for every job submission.
>
> Instead, I think the list of job ids (and eventually some extra data about each
> job) is runtime data, and as such should be kept under ~/.cache/lava/jobs or
> something like that

Sounds good.
But I'm not fixing it with this merge proposal. I will do it in
another one, since I guess it is better if we use pyxdg for this. Not
sure if we should also move the ".lavaconfig" file into
$XDG_CONFIG_HOME.

>> +ACTIONS_ID = "actions"
>> +COMMAND_ID = "command"
>> +IMAGE_ID = "image"
>> +PARAMETERS_ID = "parameters"
>> +DEVICE_TYPE_ID = "device_type"
>> +TESTDEF_REPOS_ID = "testdef_repos"
>> +TESTDEF_REPOS_TAR_REPO = "tar-repo"
>> +STREAM_ID = "stream"
>> +SERVER_ID = "server"
>
> I don’t think this many constants are usefull. I would rather expect the actual
> template data to be as close to the actual data as possible and believe that
> will make their maintaince easier.

Will clean them out.

>> -            user_input = raw_input(prompt).strip()
>> +            data = raw_input(prompt).strip()
>>          except EOFError:
>> -            pass
>> +            # Force to return None.
>> +            data = None
>>          except KeyboardInterrupt:
>>              sys.exit(-1)
>> -
>> -        if user_input is not None:
>> +        return data
>> +
>> +    @classmethod
>> +    def serialize(cls, value):
>> +        """Serializes the passed value to be friendly written to file.
>> +
>> +        Lists are serialized as a comma separated string of values.
>
> I already saw something about serializing lists above. Can we keep all this
> list serialization/deserialization just here?

What do you mean? Having a single method to serialize and deserialize
instead of the two we have right now?
"serialize" and "deserialize" are in the same class (Parameter), the
LIST_SERIALIZE_DELIMITER is a module constant.

>> +        :param value: The value to serialize.
>> +        :return The serialized value as string.
>> +        """
>> +        serialized = ""
>> +        if isinstance(value, list):
>> +            serialized = LIST_SERIALIZE_DELIMITER.join(
>> +                str(x) for x in value if x)
>> +        else:
>> +            serialized = str(value)
>> +        return serialized
>
> I wonder if we can we use a existing serialization instead of using a arbitrary
> delimiter ...  e.g. json + base64, this way we don’t have to limit ourselves to
> values that do not contain a comma. But OTOH we want something that is readable
> (and editable!) in the config file ...

I wanted to use something that could have been human readable
(avoiding also all hashing mechanism), and easily to modify.
I considered also pickle, but then I would have preferred to pass
everything through it rather than just a small set of values.
We can probably use just json.dumps() and json.loads(), even if I'm
not so sure about that... why not using just json or YAML in the
config that have better marshaling support?

> As far as this MP is concerned this class is not used (apart from its own unit
> tests). This is the one you mentioned in the introduction right?
>
> I can't see the point of a URL scheme parameter, I think it's easier to just
> ask for the URL itself and then deduce the scheme from there.

Yes. Right now it is not needed anymore, I thought I had removed it in
the other MP, but
This was done to infer the scheme type when at the beginning we wanted
to use "data:" to send base64 encoded content.

> As I commented above, this TarRepoParameter class should not be needed (i.e.
> users should not need to deal with tarballs themselves)

Will refactor this out.

> I don't think that insuficient permissions are the only possible cause of an
> IOError here ...

Yep, fixed.

>> === added file 'lava/testdef/templates.py'
>> +# This is what will be called by default by the test definition yaml file.
>> +DEFAULT_TESTDEF_SCRIPT = "mytest.sh"
>> +DEFAULT_TESTDEF_SCRIPT_CONTENT = """#!/bin/sh
>> +# Automatic generated content by lava-tool.
>> +# Please add your own instructions.
>> +"""
>
> It would be nice to include some commented examples of the lava-test-shell API
> usage ...

Will add them, sure.

>> +DEFAULT_TESTDEF_STEP = "./mytest.sh"
>> +
>> +DEFAULT_ENVIRONMET_VALUE = "lava_test_shell"
>> +
>> +# All these parameters will not be stored on the local config file.
>> +NAME_PARAMETER = Parameter("name")
>> +NAME_PARAMETER.store = False
>> +
>> +DESCRIPTION_PARAMETER = Parameter("description", depends=NAME_PARAMETER)
>> +DESCRIPTION_PARAMETER.store = False
>> +
>> +ENVIRONMENT_PARAMETER = ListParameter("environment",
>> +                                      depends=NAME_PARAMETER)
>> +ENVIRONMENT_PARAMETER.add(DEFAULT_ENVIRONMET_VALUE)
>> +ENVIRONMENT_PARAMETER.asked = True
>> +ENVIRONMENT_PARAMETER.store = False
>> +
>> +# Steps parameter. Default to a local shell script that the user defines.
>> +# We do not ask this parameter, and we do not store it either.
>> +STEPS_PARAMETER = ListParameter("steps", depends=NAME_PARAMETER)
>> +STEPS_PARAMETER.add(DEFAULT_TESTDEF_STEP)
>> +STEPS_PARAMETER.asked = True
>> +STEPS_PARAMETER.store = False
>> +
>> +TESTDEF_TEMPLATE = {
>> +    "metadata": {
>> +        "name": NAME_PARAMETER,
>> +        "format": DEFAULT_TESTDEF_FORMAT,
>> +        "version": DEFAULT_TESTDEF_VERSION,
>> +        "description": DESCRIPTION_PARAMETER,
>> +        "environment": ENVIRONMENT_PARAMETER,
>> +    },
>> +    "run": {
>> +        "steps": STEPS_PARAMETER,
>> +    },
>> +    "parse": {
>> +        "pattern": "^\\s*(?P<test_case_id>\\w+)=(?P<result>\\w+)\\s*$"
>
> use single quotes to avoid escaping hell?
> maybe you also want to include some examples of patterns commented-out ...
> (yay for YAML actually supporting comments)

Sounds good.

-- 
Milo Casagrande | Automation Engineer
Linaro.org <www.linaro.org> │ Open source software for ARM SoCs

Revision history for this message

Antonio Terceiro (terceiro) wrote on 2013-07-24:

Download full text (3.6 KiB)

> > My initial thinking was to always generate a tarball dinamically, so that
> > you don’t have to explicitly update a job file. It would go like this:
> >
> > $ lava job submit FOO.json
> >
> > submits FOO.json as ir
>
> This works.
>
> > $ lava testdef submit FOO.yaml
> >
> > creates the tarball and a job file dinamically; submit those
> >
> > $ lava script submit FOO.sh
> >
> > creates testdef, tarball and job file dynamically; submit them
>
> These two are still missing.
>
> After showing this at Connect though, the common patterns were:
> - I will not use this (mostly from QA or people that knows how to write testdef)
> - There are too many commands (from a couple of Linaro employees and
> ARM ones too, hence the "lava init" stuff that will do everything
> almost automatically).

OK, that's good to know. I think we can reduce the number of commands,
but still keep the functionality - because craeting testdefs
automatically is *not* intended at people who already know how to create
them by themselves. :-)

> >> - user_input = raw_input(prompt).strip()
> >> + data = raw_input(prompt).strip()
> >> except EOFError:
> >> - pass
> >> + # Force to return None.
> >> + data = None
> >> except KeyboardInterrupt:
> >> sys.exit(-1)
> >> -
> >> - if user_input is not None:
> >> + return data
> >> +
> >> + @classmethod
> >> + def serialize(cls, value):
> >> + """Serializes the passed value to be friendly written to file.
> >> +
> >> + Lists are serialized as a comma separated string of values.
> >
> > I already saw something about serializing lists above. Can we keep all this
> > list serialization/deserialization just here?
>
> What do you mean? Having a single method to serialize and deserialize
> instead of the two we have right now?
> "serialize" and "deserialize" are in the same class (Parameter), the
> LIST_SERIALIZE_DELIMITER is a module constant.

Above there was a call to serialize, and maybe this should be handled
transparently inside this class. The factoring in serialize/deserialize
is good already.

> >> + :param value: The value to serialize.
> >> + :return The serialized value as string.
> >> + """
> >> + serialized = ""
> >> + if isinstance(value, list):
> >> + serialized = LIST_SERIALIZE_DELIMITER.join(
> >> + str(x) for x in value if x)
> >> + else:
> >> + serialized = str(value)
> >> + return serialized
> >
> > I wonder if we can we use a existing serialization instead of using a arbitrary
> > delimiter ... e.g. json + base64, this way we don’t have to limit ourselves to
> > values that do not contain a comma. But OTOH we want something that is readable
> > (and editable!) in the config file ...
>
> I wanted to use something that could have been human readable
> (avoiding also all hashing mechanism), and easily to modify.
> I considered also pickle, but then I would have preferred to pass
> everything through it rather than just a small set of values.
> We can probably use just json.dumps() and json.loads(), even if ...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Linaro Validation Team

Michael Hudson-Doyle

Milo Casagrande

Yongqin Liu

Zygmunt Krynicki

 === modified file '.coveragerc'
 --- .coveragerc	2013-07-25 15:43:25 +0000
 +++ .coveragerc	2013-07-25 15:43:26 +0000
@@ -3,6 +3,7 @@
  source = .
  omit =
      setup*
++    */tests/*
  [report]
  precision = 2
 === modified file 'entry_points.ini'
 --- entry_points.ini	2013-07-25 15:43:25 +0000
 +++ entry_points.ini	2013-07-25 15:43:26 +0000
@@ -9,6 +9,11 @@
  dashboard = lava_dashboard_tool.commands:dashboard
  job = lava.job.commands:job
  device = lava.device.commands:device
++testdef = lava.testdef.commands:testdef
++init = lava.commands:init
++submit = lava.commands:submit
++run = lava.commands:run
++update = lava.commands:update
  [lava_tool.commands]
  help = lava.tool.commands.help:help
@@ -78,3 +83,6 @@
  add = lava.device.commands:add
  remove = lava.device.commands:remove
  config = lava.device.commands:config
++
++[lava.testdef.commands]
++new = lava.testdef.commands:new
 === added file 'lava/commands.py'
 --- lava/commands.py	1970-01-01 00:00:00 +0000
 +++ lava/commands.py	2013-07-25 15:43:26 +0000
@@ -0,0 +1,240 @@
++# Copyright (C) 2013 Linaro Limited
++#
++# Author: Milo Casagrande <milo.casagrande@linaro.org>
++#
++# This file is part of lava-tool.
++#
++# lava-tool is free software: you can redistribute it and/or modify
++# it under the terms of the GNU Lesser General Public License version 3
++# as published by the Free Software Foundation
++#
++# lava-tool is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++# GNU General Public License for more details.
++#
++# You should have received a copy of the GNU Lesser General Public License
++# along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
++
++"""
++Lava init commands.
++
++When invoking:
++
++ `lava init [DIR]`
++
++the command will create a default directory and files structure as follows:
++
++DIR/
++   |
++   +- JOB_FILE.json
++   +- tests/
++           |
++           + mytest.sh
++           + lavatest.yaml
++
++If DIR is not passed, it will use the current working directory.
++JOB_FILE is a file name that will be asked to the user, along with
++other necessary information to define the tests.
++
++If the user manually updates either the lavatest.yaml or mytest.sh file, it is
++necessary to run the following command in order to update the job definition:
++
++ `lava update [JOB|DIR]`
++"""
++
++import copy
++import json
++import os
++import stat
++import sys
++
++from lava.helper.command import BaseCommand
++from lava.helper.template import (
++    expand_template,
++    set_value
++)
++from lava.job import (
++    JOB_FILE_EXTENSIONS,
++)
++from lava.job.templates import (
++    LAVA_TEST_SHELL_TAR_REPO_KEY,
++)
++from lava.parameter import (
++    Parameter,
++)
++from lava.testdef.templates import (
++    DEFAULT_TESTDEF_FILE,
++    DEFAULT_TESTDEF_SCRIPT,
++    DEFAULT_TESTDEF_SCRIPT_CONTENT,
++)
++from lava.tool.errors import CommandError
++from lava_tool.utils import (
++    edit_file,
++    retrieve_file,
++    create_dir,
++    write_file,
++)
++
++# Default directory structure name.
++TESTS_DIR = "tests"
++
++# Internal parameter ids.
++JOBFILE_ID = "jobfile"
++
++JOBFILE_PARAMETER = Parameter(JOBFILE_ID)
++JOBFILE_PARAMETER.store = False
++
++INIT_TEMPLATE = {
++    JOBFILE_ID: JOBFILE_PARAMETER,
++}
++
++
++class init(BaseCommand):
++    """Set-ups the base directory structure."""
++
++    @classmethod
++    def register_arguments(cls, parser):
++        super(init, cls).register_arguments(parser)
++        parser.add_argument("DIR",
++                            help=("The name of the directory to initialize. "
++                                  "Defaults to current working directory."),
++                            nargs="?",
++                            default=os.getcwd())
++
++    def invoke(self):
++        full_path = os.path.abspath(self.args.DIR)
++
++        if os.path.isfile(full_path):
++            raise CommandError("'{0}' already exists, and is a "
++                               "file.".format(self.args.DIR))
++
++        create_dir(full_path)
++
++        data = self._update_data()
++
++        test_path = create_dir(full_path, TESTS_DIR)
++        # TODO
++        self._create_script(test_path)
++
++        testdef_file = self.create_test_definition(
++            os.path.join(test_path, DEFAULT_TESTDEF_FILE))
++
++        job = data[JOBFILE_ID]
++        self.create_tar_repo_job(
++            os.path.join(full_path, job), testdef_file, test_path)
++
++    def _update_data(self):
++        """Updates the template and ask values to the user.
++
++        The template in this case is a layout of the directory structure as it
++        would be written to disk.
++
++        :return A dictionary containing all the necessary file names to create.
++        """
++        data = copy.deepcopy(INIT_TEMPLATE)
++        expand_template(data, self.config)
++
++        return data
++
++    def _create_script(self, test_path):
++        # This is the default script file as defined in the testdef template.
++        default_script = os.path.join(test_path, DEFAULT_TESTDEF_SCRIPT)
++
++        if not os.path.isfile(default_script):
++            # We do not have the default testdef script. Create it, but
++            # remind the user to update it.
++            print >> sys.stdout, ("\nCreating default test script "
++                                  "'{0}'.".format(DEFAULT_TESTDEF_SCRIPT))
++
++            write_file(default_script, DEFAULT_TESTDEF_SCRIPT_CONTENT)
++
++        # Prompt the user to write the script file.
++        edit_file(default_script)
++
++        # Make sure the script is executable.
++        os.chmod(default_script,
++                 stat.S_IRWXU | stat.S_IRWXG | stat.S_IROTH | stat.S_IXOTH)
++
++
++class run(BaseCommand):
++    """Runs a job on the local dispatcher."""
++
++    @classmethod
++    def register_arguments(cls, parser):
++        super(run, cls).register_arguments(parser)
++        parser.add_argument("JOB",
++                            help=("The job file to run, or a directory "
++                                  "containing a job file. If nothing is "
++                                  "passed, it uses the current working "
++                                  "directory."),
++                            nargs="?",
++                            default=os.getcwd())
++
++    def invoke(self):
++        full_path = os.path.abspath(self.args.JOB)
++        job_file = retrieve_file(full_path, JOB_FILE_EXTENSIONS)
++
++        super(run, self).run(job_file)
++
++
++class submit(BaseCommand):
++    """Submits a job to LAVA."""
++
++    @classmethod
++    def register_arguments(cls, parser):
++        super(submit, cls).register_arguments(parser)
++        parser.add_argument("JOB",
++                            help=("The job file to send, or a directory "
++                                  "containing a job file. If nothing is "
++                                  "passed, it uses the current working "
++                                  "directory."),
++                            nargs="?",
++                            default=os.getcwd())
++
++    def invoke(self):
++        full_path = os.path.abspath(self.args.JOB)
++        job_file = self.retrieve_file(full_path, JOB_FILE_EXTENSIONS)
++
++        super(submit, self).submit(job_file)
++
++
++class update(BaseCommand):
++    """Updates a job file with the correct data."""
++
++    @classmethod
++    def register_arguments(cls, parser):
++        super(update, cls).register_arguments(parser)
++        parser.add_argument("JOB",
++                            help=("Automatically updates a job file "
++                                  "definition. If nothing is passed, it uses"
++                                  "the current working directory."),
++                            nargs="?",
++                            default=os.getcwd())
++
++    def invoke(self):
++        full_path = os.path.abspath(self.args.JOB)
++        job_file = self.retrieve_file(full_path, JOB_FILE_EXTENSIONS)
++        job_dir = os.path.dirname(job_file)
++        tests_dir = os.path.join(job_dir, TESTS_DIR)
++
++        if os.path.isdir(tests_dir):
++            # TODO
++            encoded_tests = None
++
++            json_data = None
++            with open(job_file, "r") as json_file:
++                try:
++                    json_data = json.load(json_file)
++                    set_value(
++                        json_data, LAVA_TEST_SHELL_TAR_REPO_KEY, encoded_tests)
++                except Exception:
++                    raise CommandError("Cannot read job file '{0}'.".format(
++                        job_file))
++
++            content = json.dumps(json_data, indent=4)
++            write_file(job_file, content)
++
++            print >> sys.stdout, "Job definition updated."
++        else:
++            raise CommandError("Cannot find tests directory.")
 === modified file 'lava/config.py'
 --- lava/config.py	2013-07-25 15:43:25 +0000
 +++ lava/config.py	2013-07-25 15:43:26 +0000
@@ -30,8 +30,10 @@
      NoSectionError,
+ )
++from lava.parameter import Parameter
  from lava.tool.errors import CommandError
++
  __all__ = ['Config', 'InteractiveConfig']
  # Store for function calls to be made at exit time.
@@ -53,19 +55,35 @@
          call()
  atexit.register(_run_at_exit)
--
  class Config(object):
      """A generic config object."""
++
      def __init__(self):
          # The cache where to store parameters.
          self._cache = {}
--        self._config_file = (os.environ.get('LAVACONFIG') or
--                             os.path.join(os.path.expanduser('~'),
--                                          '.lavaconfig'))
--        self._config_backend = ConfigParser()
--        self._config_backend.read([self._config_file])
++        self._config_file = None
++        self._config_backend = None
          AT_EXIT_CALLS.add(self.save)
++    @property
++    def config_file(self):
++        if self._config_file is None:
++            self._config_file = (os.environ.get('LAVACONFIG') or
++                                 os.path.join(os.path.expanduser('~'),
++                                              '.lavaconfig'))
++        return self._config_file
++
++    @config_file.setter
++    def config_file(self, value):
++        self._config_file = value
++
++    @property
++    def config_backend(self):
++        if self._config_backend is None:
++            self._config_backend = ConfigParser()
++            self._config_backend.read([self.config_file])
++        return self._config_backend
++
      def _calculate_config_section(self, parameter):
          """Calculates the config section of the specified parameter.
@@ -92,7 +110,7 @@
          if not section:
              section = self._calculate_config_section(parameter)
          # Try to get the parameter value first if it has one.
--        if parameter.value:
++        if parameter.value is not None:
              value = parameter.value
          else:
              value = self._get_from_cache(parameter, section)
@@ -109,7 +127,7 @@
          """
          value = None
          try:
--            value = self._config_backend.get(section, parameter.id)
++            value = self.config_backend.get(section, parameter.id)
          except (NoOptionError, NoSectionError):
              # Ignore, we return None.
              pass
@@ -147,10 +165,17 @@
          :param value: The value to add.
          :param section: The name of the section as in the config file.
          """
--        if (not self._config_backend.has_section(section) and
++        if (not self.config_backend.has_section(section) and
                  section != DEFAULT_SECTION):
--            self._config_backend.add_section(section)
--        self._config_backend.set(section, key, value)
++            self.config_backend.add_section(section)
++
++        # This is done to serialize a list when ConfigParser is written to
++        # file. Since there is no real support for list in ConfigParser, we
++        # serialized it in a common way that can get easily deserialized.
++        if isinstance(value, list):
++            value = Parameter.serialize(value)
++
++        self.config_backend.set(section, key, value)
          # Store in the cache too.
          self._put_in_cache(key, value, section)
@@ -158,6 +183,7 @@
          """Adds a Parameter to the config file and cache.
          :param Parameter: The parameter to add.
++        :type Parameter
          :param value: The value of the parameter. Defaults to None.
          :param section: The section where this parameter should be stored.
                          Defaults to None.
@@ -174,8 +200,8 @@
      def save(self):
          """Saves the config to file."""
--        with open(self._config_file, "w") as write_file:
--            self._config_backend.write(write_file)
++        with open(self.config_file, "w") as write_file:
++            self.config_backend.write(write_file)
  class InteractiveConfig(Config):
@@ -188,6 +214,14 @@
          super(InteractiveConfig, self).__init__()
          self._force_interactive = force_interactive
++    @property
++    def force_interactive(self):
++        return self._force_interactive
++
++    @force_interactive.setter
++    def force_interactive(self, value):
++        self._force_interactive = value
++
      def get(self, parameter, section=None):
          """Overrides the parent one.
@@ -198,10 +232,9 @@
              section = self._calculate_config_section(parameter)
          value = super(InteractiveConfig, self).get(parameter, section)
--        if not (value is not None and parameter.asked):
--            if not value or self._force_interactive:
--                value = parameter.prompt(old_value=value)
++        if value is None or self.force_interactive:
++            value = parameter.prompt(old_value=value)
--        if value is not None:
++        if value is not None and parameter.store:
              self.put(parameter.id, value, section)
          return value
 === modified file 'lava/device/__init__.py'
 --- lava/device/__init__.py	2013-07-25 15:43:25 +0000
 +++ lava/device/__init__.py	2013-07-25 15:43:26 +0000
@@ -67,8 +67,8 @@
          # given on the command line for the config file.
          if self.hostname is not None:
              # We do not ask the user again this parameter.
++            self.data[HOSTNAME_PARAMETER.id].value = self.hostname
              self.data[HOSTNAME_PARAMETER.id].asked = True
--            config.put(HOSTNAME_PARAMETER.id, self.hostname)
          expand_template(self.data, config)
@@ -85,9 +85,9 @@
      :param name: The name of the device we want matched to a real device.
      :return A Device instance.
          """
--    instance = Device(DEFAULT_TEMPLATE, name)
++    instance = Device(DEFAULT_TEMPLATE, hostname=name)
      for known_dev, (matcher, dev_template) in KNOWN_DEVICES.iteritems():
          if matcher.match(name):
--            instance = Device(dev_template, name)
++            instance = Device(dev_template, hostname=name)
              break
      return instance
 === modified file 'lava/device/commands.py'
 --- lava/device/commands.py	2013-07-25 15:43:25 +0000
 +++ lava/device/commands.py	2013-07-25 15:43:26 +0000
@@ -23,18 +23,20 @@
  import os
  import sys
++from lava.device import get_known_device
  from lava.helper.command import (
      BaseCommand,
+ )
--
  from lava.helper.dispatcher import (
      get_device_file,
      get_devices_path,
+ )
--
--from lava.device import get_known_device
  from lava.tool.command import CommandGroup
  from lava.tool.errors import CommandError
++from lava_tool.utils import (
++    can_edit_file,
++    edit_file,
++)
  DEVICE_FILE_SUFFIX = "conf"
@@ -73,7 +75,7 @@
          print >> sys.stdout, ("Created device file '{0}' in: {1}".format(
              real_file_name, devices_path))
--        self.edit_file(device_conf_file)
++        edit_file(device_conf_file)
  class remove(BaseCommand):
@@ -114,7 +116,7 @@
          real_file_name = ".".join([self.args.DEVICE, DEVICE_FILE_SUFFIX])
          device_conf = get_device_file(real_file_name)
--        if device_conf and self.can_edit_file(device_conf):
--            self.edit_file(device_conf)
++        if device_conf and can_edit_file(device_conf):
++            edit_file(device_conf)
          else:
              raise CommandError("Cannot edit file '{0}'".format(real_file_name))
 === modified file 'lava/device/tests/test_commands.py'
 --- lava/device/tests/test_commands.py	2013-07-25 15:43:25 +0000
 +++ lava/device/tests/test_commands.py	2013-07-25 15:43:26 +0000
@@ -50,35 +50,39 @@
          name, args, kwargs = self.parser.method_calls[1]
          self.assertIn("DEVICE", args)
--    @patch("lava.device.Device.__str__", new=MagicMock(return_value=""))
--    @patch("lava.device.Device.update", new=MagicMock())
--    @patch("lava.device.commands.get_device_file",
--           new=MagicMock(return_value=None))
++    @patch("lava.device.commands.edit_file", create=True)
++    @patch("lava.device.Device.__str__")
++    @patch("lava.device.Device.update")
++    @patch("lava.device.commands.get_device_file")
      @patch("lava.device.commands.get_devices_path")
--    def test_add_invoke_0(self, get_devices_path_mock):
++    def test_add_invoke_0(self, mocked_get_devices_path,
++                          mocked_get_device_file, mocked_update, mocked_str,
++                          mocked_edit_file):
          # Tests invocation of the add command. Verifies that the conf file is
          # written to disk.
--        get_devices_path_mock.return_value = self.temp_dir
++        mocked_get_devices_path.return_value = self.temp_dir
++        mocked_get_device_file.return_value = None
++        mocked_str.return_value = ""
          add_command = add(self.parser, self.args)
--        add_command.edit_file = MagicMock()
          add_command.invoke()
          expected_path = os.path.join(self.temp_dir,
                                       ".".join([self.device, "conf"]))
          self.assertTrue(os.path.isfile(expected_path))
++    @patch("lava.device.commands.edit_file", create=True)
      @patch("lava.device.commands.get_known_device")
      @patch("lava.device.commands.get_devices_path")
      @patch("lava.device.commands.sys.exit")
      @patch("lava.device.commands.get_device_file")
      def test_add_invoke_1(self, mocked_get_device_file, mocked_sys_exit,
--                          mocked_get_devices_path, mocked_get_known_device):
++                          mocked_get_devices_path, mocked_get_known_device,
++                          mocked_edit_file):
          mocked_get_devices_path.return_value = self.temp_dir
          mocked_get_device_file.return_value = self.temp_file.name
          add_command = add(self.parser, self.args)
--        add_command.edit_file = MagicMock()
          add_command.invoke()
          self.assertTrue(mocked_sys_exit.called)
@@ -97,11 +101,13 @@
          name, args, kwargs = self.parser.method_calls[1]
          self.assertIn("DEVICE", args)
--    @patch("lava.device.Device.__str__", new=MagicMock(return_value=""))
--    @patch("lava.device.Device.update", new=MagicMock())
++    @patch("lava.device.commands.edit_file", create=True)
++    @patch("lava.device.Device.__str__", return_value="")
++    @patch("lava.device.Device.update")
      @patch("lava.device.commands.get_device_file")
      @patch("lava.device.commands.get_devices_path")
--    def test_remove_invoke(self, get_devices_path_mock, get_device_file_mock):
++    def test_remove_invoke(self, get_devices_path_mock, get_device_file_mock,
++                           mocked_update, mocked_str, mocked_edit_file):
          # Tests invocation of the remove command. Verifies that the conf file
          # has been correctly removed.
          # First we add a new conf file, then we remove it.
@@ -109,7 +115,6 @@
          get_devices_path_mock.return_value = self.temp_dir
          add_command = add(self.parser, self.args)
--        add_command.edit_file = MagicMock()
          add_command.invoke()
          expected_path = os.path.join(self.temp_dir,
@@ -145,18 +150,20 @@
          name, args, kwargs = self.parser.method_calls[1]
          self.assertIn("DEVICE", args)
++    @patch("lava.device.commands.can_edit_file", create=True)
++    @patch("lava.device.commands.edit_file", create=True)
      @patch("lava.device.commands.get_device_file")
--    def test_config_invoke_0(self, mocked_get_device_file):
++    def test_config_invoke_0(self, mocked_get_device_file, mocked_edit_file,
++                             mocked_can_edit_file):
          command = config(self.parser, self.args)
++        mocked_can_edit_file.return_value = True
          mocked_get_device_file.return_value = self.temp_file.name
--        command.can_edit_file = MagicMock(return_value=True)
--        command.edit_file = MagicMock()
          command.invoke()
--        self.assertTrue(command.edit_file.called)
++        self.assertTrue(mocked_edit_file.called)
          self.assertEqual([call(self.temp_file.name)],
--                         command.edit_file.call_args_list)
++                         mocked_edit_file.call_args_list)
      @patch("lava.device.commands.get_device_file",
             new=MagicMock(return_value=None))
 === modified file 'lava/device/tests/test_device.py'
 --- lava/device/tests/test_device.py	2013-07-25 15:43:25 +0000
 +++ lava/device/tests/test_device.py	2013-07-25 15:43:26 +0000
@@ -20,19 +20,20 @@
  Device class unit tests.
  """
--from lava.parameter import Parameter
++from mock import patch
++
++from lava.config import Config
++from lava.device import (
++    Device,
++    get_known_device,
++)
  from lava.device.templates import (
      HOSTNAME_PARAMETER,
      PANDA_DEVICE_TYPE,
      PANDA_CONNECTION_COMMAND,
+ )
--from lava.tests.test_config import MockedConfig
--from lava.device import (
--    Device,
--    get_known_device,
--)
--from lava.tool.errors import CommandError
  from lava.helper.tests.helper_test import HelperTest
++from lava.parameter import Parameter
  class DeviceTest(HelperTest):
@@ -64,12 +65,14 @@
          self.assertIsInstance(instance.data['device_type'], Parameter)
          self.assertEqual(instance.data['device_type'].value, 'vexpress')
--    def test_device_update_1(self):
++    @patch("lava.config.Config.save")
++    def test_device_update_1(self, patched_save):
          # Tests that when calling update() on a Device, the template gets
          # updated with the correct values from a Config instance.
          hostname = "panda_device"
--        config = MockedConfig(self.temp_file.name)
++        config = Config()
++        config._config_file = self.temp_file.name
          config.put_parameter(HOSTNAME_PARAMETER, hostname)
          config.put_parameter(PANDA_DEVICE_TYPE, "panda")
          config.put_parameter(PANDA_CONNECTION_COMMAND, "test")
@@ -85,12 +88,14 @@
          self.assertEqual(expected, instance.data)
--    def test_device_write(self):
++    @patch("lava.config.Config.save")
++    def test_device_write(self, mocked_save):
          # User tries to create a new panda device. The conf file is written
          # and contains the expected results.
          hostname = "panda_device"
--        config = MockedConfig(self.temp_file.name)
++        config = Config()
++        config._config_file = self.temp_file.name
          config.put_parameter(HOSTNAME_PARAMETER, hostname)
          config.put_parameter(PANDA_DEVICE_TYPE, "panda")
          config.put_parameter(PANDA_CONNECTION_COMMAND, "test")
 === modified file 'lava/helper/command.py'
 --- lava/helper/command.py	2013-07-25 15:43:25 +0000
 +++ lava/helper/command.py	2013-07-25 15:43:26 +0000
@@ -19,22 +19,49 @@
  """Base command class common to lava commands series."""
  import os
--import subprocess
  import sys
--
++import xmlrpclib
  from lava.config import InteractiveConfig
++from lava.helper.dispatcher import get_devices
++from lava.parameter import (
++    Parameter,
++    SingleChoiceParameter,
++)
  from lava.tool.command import Command
  from lava.tool.errors import CommandError
--from lava_tool.utils import has_command
++from lava_tool.authtoken import (
++    AuthenticatingServerProxy,
++    KeyringAuthBackend
++)
++from lava_tool.utils import (
++    has_command,
++    verify_and_create_url,
++    create_tar,
++    base64_encode,
++)
++from lava.job import Job
++from lava.job.templates import (
++    LAVA_TEST_SHELL_TAR_REPO,
++    LAVA_TEST_SHELL_TAR_REPO_KEY,
++    LAVA_TEST_SHELL_TESDEF_KEY,
++)
++
++from lava.testdef import TestDefinition
++from lava.testdef.templates import (
++    TESTDEF_TEMPLATE,
++)
++CONFIG = InteractiveConfig()
  class BaseCommand(Command):
++
      """Base command class for all lava commands."""
++
      def __init__(self, parser, args):
          super(BaseCommand, self).__init__(parser, args)
--        self.config = InteractiveConfig(
--            force_interactive=self.args.non_interactive)
++        self.config = CONFIG
++        self.config.force_interactive = self.args.non_interactive
      @classmethod
      def register_arguments(cls, parser):
@@ -43,59 +70,109 @@
                              action='store_false',
                              help=("Do not ask for input parameters."))
--    @classmethod
--    def can_edit_file(cls, conf_file):
--        """Checks if a file can be opend in write mode.
--
--        :param conf_file: The path to the file.
--        :return True if it is possible to write on the file, False otherwise.
--        """
--        can_edit = True
--        try:
--            fp = open(conf_file, "a")
--            fp.close()
--        except IOError:
--            can_edit = False
--        return can_edit
--
--    @classmethod
--    def edit_file(cls, config_file):
--        """Opens the specified file with the default file editor.
--
--        :param config_file: The file to edit.
--        """
--        editor = os.environ.get("EDITOR", None)
--        if editor is None:
--            if has_command("sensible-editor"):
--                editor = "sensible-editor"
--            elif has_command("xdg-open"):
--                editor = "xdg-open"
++    def authenticated_server(self):
++        """Returns a connection to a LAVA server.
++
++        It will ask the user the necessary parameters to establish the
++        connection.
++        """
++        server_name_parameter = Parameter("server")
++        rpc_endpoint_parameter = Parameter("rpc_endpoint",
++                                           depends=server_name_parameter)
++
++        server_url = self.config.get(server_name_parameter)
++        endpoint = self.config.get(rpc_endpoint_parameter)
++
++        rpc_url = verify_and_create_url(server_url, endpoint)
++        server = AuthenticatingServerProxy(rpc_url,
++                                           auth_backend=KeyringAuthBackend())
++        return server
++
++    def submit(self, job_file):
++        """Submits a job file to a LAVA server.
++
++        :param job_file: The job file to submit.
++        :return The job ID on success.
++        """
++        if os.path.isfile(job_file):
++            try:
++                jobdata = open(job_file, 'rb').read()
++                server = self.authenticated_server()
++
++                job_id = server.scheduler.submit_job(jobdata)
++                print >> sys.stdout, ("Job submitted with job "
++                                      "ID {0}.".format(job_id))
++
++                return job_id
++            except xmlrpclib.Fault, exc:
++                raise CommandError(str(exc))
++        else:
++            raise CommandError("Job file '{0}' does not exists, or is not "
++                               "a file.".format(job_file))
++
++    def run(self, job_file):
++        """Runs a job file on the local LAVA dispatcher.
++
++        :param job_file: The job file to run.
++        """
++        if os.path.isfile(job_file):
++            if has_command("lava-dispatch"):
++                devices = get_devices()
++                if devices:
++                    if len(devices) > 1:
++                        device_names = [device.hostname for device in devices]
++                        device_param = SingleChoiceParameter("device",
++                                                             device_names)
++                        device = device_param.prompt("Device to use: ")
++                    else:
++                        device = devices[0].hostname
++                    self.execute(
++                        ["lava-dispatch", "--target", device, job_file])
              else:
--                # We really do not know how to open a file.
--                print >> sys.stdout, ("Cannot find an editor to open the "
--                                      "file '{0}'.".format(config_file))
--                print >> sys.stdout, ("Either set the 'EDITOR' environment "
--                                      "variable, or install 'sensible-editor' "
--                                      "or 'xdg-open'.")
--                sys.exit(-1)
--        try:
--            subprocess.Popen([editor, config_file]).wait()
--        except Exception:
--            raise CommandError("Error opening the file '{0}' with the "
--                               "following editor: {1}.".format(config_file,
--                                                               editor))
--
--    @classmethod
--    def run(cls, cmd_args):
--        """Runs the supplied command args.
--
--        :param cmd_args: The command, and its optional arguments, to run.
--        :return The command execution return code.
--        """
--        if not isinstance(cmd_args, list):
--            cmd_args = [cmd_args]
--        try:
--            return subprocess.check_call(cmd_args)
--        except subprocess.CalledProcessError:
--            raise CommandError("Error running the following command: "
--                               "{0}".format(" ".join(cmd_args)))
++                raise CommandError("Cannot find lava-dispatcher installation.")
++        else:
++            raise CommandError("Job file '{0}' does not exists, or it is not "
++                               "a file.".format(job_file))
++
++    def create_tar_repo_job(self, job_file, testdef_file, tar_content):
++        """Creates a job file based on the tar-repo template.
++
++        The tar repo is not kept on the file system.
++
++        :param job_file: The path of the job file to create.
++        :param testdef_file: The path of the test definition file.
++        :param tar_content: What should go into the tarball repository.
++        :return The path of the job file created.
++        """
++        try:
++            tar_repo = create_tar(tar_content)
++
++            job_instance = Job(LAVA_TEST_SHELL_TAR_REPO, job_file)
++            job_instance.update(self.config)
++
++            job_instance.set(LAVA_TEST_SHELL_TAR_REPO_KEY,
++                             base64_encode(tar_repo))
++            job_instance.set(LAVA_TEST_SHELL_TESDEF_KEY,
++                             os.path.basename(testdef_file))
++
++            job_instance.write()
++
++            return job_instance.file_name
++        finally:
++            if os.path.isfile(tar_repo):
++                os.unlink(tar_repo)
++
++    def create_test_definition(self, testdef_file, template=TESTDEF_TEMPLATE):
++        """Creates a test definition YAML file.
++
++        :param testdef_file: The file to create.
++        :return The path of the file created.
++        """
++        testdef = TestDefinition(template, testdef_file)
++        testdef.update(self.config)
++        testdef.write()
++
++        print >> sys.stdout, ("Create test definition "
++                              "'{0}'.".format(testdef.file_name))
++
++        return testdef.file_name
 === modified file 'lava/helper/template.py'
 --- lava/helper/template.py	2013-07-25 15:43:25 +0000
 +++ lava/helper/template.py	2013-07-25 15:43:26 +0000
@@ -16,6 +16,8 @@
  # You should have received a copy of the GNU Lesser General Public License
  # along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
++"""Helper functions for a template."""
++
  from lava.parameter import Parameter
@@ -42,3 +44,81 @@
                  update(entry)
      update(template)
++
++
++def get_key(data, search_key):
++    """Goes through a template looking for a key.
++
++    :param data: The template to traverse.
++    :param search_key: The key to look for.
++    :return The key value.
++    """
++    return_value = None
++    found = False
++
++    if isinstance(data, dict):
++        bucket = []
++
++        for key, value in data.iteritems():
++            if key == search_key:
++                return_value = value
++                found = True
++                break
++            else:
++                bucket.append(value)
++
++        if bucket and not found:
++            for element in bucket:
++                if isinstance(element, list):
++                    for element in element:
++                        bucket.append(element)
++                elif isinstance(element, dict):
++                    for key, value in element.iteritems():
++                        if key == search_key:
++                            return_value = value
++                            found = True
++                            break
++                        else:
++                            bucket.append(value)
++                if found:
++                    break
++
++    return return_value
++
++
++def set_value(data, search_key, new_value):
++    """Sets a new value for a template key.
++
++    :param data: The data structure to update.
++    :type dict
++    :param search_key: The key to search and update.
++    :param new_value: The new value to set.
++    """
++    is_set = False
++
++    if isinstance(data, dict):
++        bucket = []
++
++        for key, value in data.iteritems():
++            if key == search_key:
++                data[key] = new_value
++                is_set = True
++                break
++            else:
++                bucket.append(value)
++
++        if bucket and not is_set:
++            for element in bucket:
++                if isinstance(element, list):
++                    for element in element:
++                        bucket.append(element)
++                elif isinstance(element, dict):
++                    for key, value in element.iteritems():
++                        if key == search_key:
++                            element[key] = new_value
++                            is_set = True
++                            break
++                        else:
++                            bucket.append(value)
++                if is_set:
++                    break
 === modified file 'lava/helper/tests/helper_test.py'
 --- lava/helper/tests/helper_test.py	2013-07-25 15:43:25 +0000
 +++ lava/helper/tests/helper_test.py	2013-07-25 15:43:26 +0000
@@ -29,12 +29,20 @@
  import tempfile
  from unittest import TestCase
--from mock import MagicMock
++from mock import (
++    MagicMock,
++    patch
++)
  class HelperTest(TestCase):
      """Helper test class that all tests under the lava package can inherit."""
++
      def setUp(self):
++        # Need to patch it here, not as a decorator, or running the tests
++        # via `./setup.py test` will fail.
++        self.at_exit_patcher = patch("lava.config.AT_EXIT_CALLS", spec=set)
++        self.at_exit_patcher.start()
          self.original_stdout = sys.stdout
          sys.stdout = open("/dev/null", "w")
          self.original_stderr = sys.stderr
@@ -50,12 +58,24 @@
          self.args.interactive = MagicMock(return_value=False)
          self.args.DEVICE = self.device
--        self.config = MagicMock()
--        self.config.get = MagicMock(return_value=self.temp_dir)
--
      def tearDown(self):
++        self.at_exit_patcher.stop()
          sys.stdin = self.original_stdin
          sys.stdout = self.original_stdout
          sys.stderr = self.original_stderr
          shutil.rmtree(self.temp_dir)
          os.unlink(self.temp_file.name)
++
++    def tmp(self, name):
++        """
++        Returns the full path to a file, or directory, called `name` in a
++        temporary directory.
++
++        This method does not create the file, it only gives a full filename
++        where you can actually write some data. The file will not be removed
++        by this method.
++
++        :param name: The name the file/directory should have.
++        :return A path.
++        """
++        return os.path.join(tempfile.gettempdir(), name)
 === modified file 'lava/helper/tests/test_command.py'
 --- lava/helper/tests/test_command.py	2013-07-25 15:43:25 +0000
 +++ lava/helper/tests/test_command.py	2013-07-25 15:43:26 +0000
@@ -18,14 +18,7 @@
  """lava.herlp.command module tests."""
--import subprocess
--from mock import (
--    MagicMock,
--    call,
--    patch,
--)
--from lava.tool.errors import CommandError
  from lava.helper.command import BaseCommand
  from lava.helper.tests.helper_test import HelperTest
@@ -39,85 +32,3 @@
          command.register_arguments(self.parser)
          name, args, kwargs = self.parser.method_calls[0]
          self.assertIn("--non-interactive", args)
--
--    def test_can_edit_file(self):
--        # Tests the can_edit_file method of the config command.
--        # This is to make sure the device config file is not erased when
--        # checking if it is possible to open it.
--        expected = ("hostname = a_fake_panda02\nconnection_command = \n"
--                    "device_type = panda\n")
--
--        command = BaseCommand(self.parser, self.args)
--        conf_file = self.temp_file
--
--        with open(conf_file.name, "w") as f:
--            f.write(expected)
--
--        self.assertTrue(command.can_edit_file(conf_file.name))
--        obtained = ""
--        with open(conf_file.name) as f:
--            obtained = f.read()
--
--        self.assertEqual(expected, obtained)
--
--    @patch("lava.helper.command.subprocess")
--    def test_run_0(self, mocked_subprocess):
--        mocked_subprocess.check_call = MagicMock()
--        BaseCommand.run("foo")
--        self.assertEqual(mocked_subprocess.check_call.call_args_list,
--                         [call(["foo"])])
--        self.assertTrue(mocked_subprocess.check_call.called)
--
--    @patch("lava.helper.command.subprocess.check_call")
--    def test_run_1(self, mocked_check_call):
--        mocked_check_call.side_effect = subprocess.CalledProcessError(1, "foo")
--        self.assertRaises(CommandError, BaseCommand.run, ["foo"])
--
--    @patch("lava.helper.command.subprocess")
--    @patch("lava.helper.command.has_command", return_value=False)
--    @patch("lava.helper.command.os.environ.get", return_value=None)
--    @patch("lava.helper.command.sys.exit")
--    def test_edit_file_0(self, mocked_sys_exit, mocked_env_get,
--                         mocked_has_command, mocked_subprocess):
--        BaseCommand.edit_file(self.temp_file.name)
--        self.assertTrue(mocked_sys_exit.called)
--
--    @patch("lava.helper.command.subprocess")
--    @patch("lava.helper.command.has_command", side_effect=[True, False])
--    @patch("lava.helper.command.os.environ.get", return_value=None)
--    def test_edit_file_1(self, mocked_env_get, mocked_has_command,
--                         mocked_subprocess):
--        mocked_subprocess.Popen = MagicMock()
--        BaseCommand.edit_file(self.temp_file.name)
--        expected = [call(["sensible-editor", self.temp_file.name])]
--        self.assertEqual(expected, mocked_subprocess.Popen.call_args_list)
--
--    @patch("lava.helper.command.subprocess")
--    @patch("lava.helper.command.has_command", side_effect=[False, True])
--    @patch("lava.helper.command.os.environ.get", return_value=None)
--    def test_edit_file_2(self, mocked_env_get, mocked_has_command,
--                         mocked_subprocess):
--        mocked_subprocess.Popen = MagicMock()
--        BaseCommand.edit_file(self.temp_file.name)
--        expected = [call(["xdg-open", self.temp_file.name])]
--        self.assertEqual(expected, mocked_subprocess.Popen.call_args_list)
--
--    @patch("lava.helper.command.subprocess")
--    @patch("lava.helper.command.has_command", return_value=False)
--    @patch("lava.helper.command.os.environ.get", return_value="vim")
--    def test_edit_file_3(self, mocked_env_get, mocked_has_command,
--                         mocked_subprocess):
--        mocked_subprocess.Popen = MagicMock()
--        BaseCommand.edit_file(self.temp_file.name)
--        expected = [call(["vim", self.temp_file.name])]
--        self.assertEqual(expected, mocked_subprocess.Popen.call_args_list)
--
--    @patch("lava.helper.command.subprocess")
--    @patch("lava.helper.command.has_command", return_value=False)
--    @patch("lava.helper.command.os.environ.get", return_value="vim")
--    def test_edit_file_4(self, mocked_env_get, mocked_has_command,
--                         mocked_subprocess):
--        mocked_subprocess.Popen = MagicMock()
--        mocked_subprocess.Popen.side_effect = Exception()
--        self.assertRaises(CommandError, BaseCommand.edit_file,
--                          self.temp_file.name)
 === added file 'lava/helper/tests/test_template.py'
 --- lava/helper/tests/test_template.py	1970-01-01 00:00:00 +0000
 +++ lava/helper/tests/test_template.py	2013-07-25 15:43:26 +0000
@@ -0,0 +1,102 @@
++# Copyright (C) 2013 Linaro Limited
++#
++# Author: Milo Casagrande <milo.casagrande@linaro.org>
++#
++# This file is part of lava-tool.
++#
++# lava-tool is free software: you can redistribute it and/or modify
++# it under the terms of the GNU Lesser General Public License version 3
++# as published by the Free Software Foundation
++#
++# lava-tool is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++# GNU General Public License for more details.
++#
++# You should have received a copy of the GNU Lesser General Public License
++# along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
++
++""" """
++
++import copy
++from unittest import TestCase
++
++from lava.helper.template import (
++    get_key,
++    set_value
++)
++
++
++TEST_TEMPLATE = {
++    "key1": "value1",
++    "key2": [
++        "value2", "value3"
++    ],
++    "key3": [
++        {
++            "key4": "value4",
++            "key5": "value5"
++        },
++        {
++            "key6": "value6",
++            "key7": "value7"
++        },
++        [
++            {
++                "key8": "value8"
++            }
++        ]
++    ],
++    "key10": {
++        "key11": "value11"
++    }
++}
++
++
++class TestParameter(TestCase):
++
++    def test_get_key_simple_key(self):
++        expected = "value1"
++        obtained = get_key(TEST_TEMPLATE, "key1")
++        self.assertEquals(expected, obtained)
++
++    def test_get_key_nested_key(self):
++        expected = "value4"
++        obtained = get_key(TEST_TEMPLATE, "key4")
++        self.assertEquals(expected, obtained)
++
++    def test_get_key_nested_key_1(self):
++        expected = "value7"
++        obtained = get_key(TEST_TEMPLATE, "key7")
++        self.assertEquals(expected, obtained)
++
++    def test_get_key_nested_key_2(self):
++        expected = "value8"
++        obtained = get_key(TEST_TEMPLATE, "key8")
++        self.assertEquals(expected, obtained)
++
++    def test_get_key_nested_key_3(self):
++        expected = "value11"
++        obtained = get_key(TEST_TEMPLATE, "key11")
++        self.assertEquals(expected, obtained)
++
++    def test_set_value_0(self):
++        data = copy.deepcopy(TEST_TEMPLATE)
++        expected = "foo"
++        set_value(data, "key1", expected)
++        obtained = get_key(data, "key1")
++        self.assertEquals(expected, obtained)
++
++    def test_set_value_1(self):
++        data = copy.deepcopy(TEST_TEMPLATE)
++        expected = "foo"
++        set_value(data, "key6", expected)
++        obtained = get_key(data, "key6")
++        self.assertEquals(expected, obtained)
++
++    def test_set_value_2(self):
++        data = copy.deepcopy(TEST_TEMPLATE)
++        expected = "foo"
++        set_value(data, "key11", expected)
++        obtained = get_key(data, "key11")
++        self.assertEquals(expected, obtained)
 === modified file 'lava/job/__init__.py'
 --- lava/job/__init__.py	2013-07-25 15:43:25 +0000
 +++ lava/job/__init__.py	2013-07-25 15:43:26 +0000
@@ -16,18 +16,46 @@
  # You should have received a copy of the GNU Lesser General Public License
  # along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
++import json
++
  from copy import deepcopy
--import json
--
--from lava.helper.template import expand_template
--
--
--class Job:
--    def __init__(self, template):
--        self.data = deepcopy(template)
--
--    def fill_in(self, config):
++
++from lava.helper.template import (
++    expand_template,
++    set_value,
++)
++from lava_tool.utils import (
++    verify_file_extension,
++    verify_path_existance,
++    write_file
++)
++
++# Default job file extension.
++DEFAULT_JOB_EXTENSION = "json"
++# Possible extension for a job file.
++JOB_FILE_EXTENSIONS = [DEFAULT_JOB_EXTENSION]
++
++
++class Job(object):
++    def __init__(self, data, file_name):
++        self.file_name = verify_file_extension(file_name,
++                                               DEFAULT_JOB_EXTENSION,
++                                               JOB_FILE_EXTENSIONS)
++        verify_path_existance(self.file_name)
++        self.data = deepcopy(data)
++
++    def set(self, key, value):
++        """Set key to the specified value.
++
++        :param key: The key to look in the object data.
++        :param value: The value to set.
++        """
++        set_value(self.data, key, value)
++
++    def update(self, config):
++        """Updates the Job object based on the provided config."""
          expand_template(self.data, config)
--    def write(self, stream):
--        stream.write(json.dumps(self.data, indent=4))
++    def write(self):
++        """Writes the Job object to file."""
++        write_file(self.file_name, json.dumps(self.data, indent=4))
 === modified file 'lava/job/commands.py'
 --- lava/job/commands.py	2013-07-25 15:43:25 +0000
 +++ lava/job/commands.py	2013-07-25 15:43:26 +0000
@@ -21,21 +21,14 @@
  """
  import os
--import sys
--import xmlrpclib
  from lava.helper.command import BaseCommand
--from lava.helper.dispatcher import get_devices
--
  from lava.job import Job
  from lava.job.templates import (
--    BOOT_TEST,
++    BOOT_TEST_KEY,
++    JOB_TYPES,
+ )
--from lava.parameter import Parameter
  from lava.tool.command import CommandGroup
--from lava.tool.errors import CommandError
--from lava_tool.authtoken import AuthenticatingServerProxy, KeyringAuthBackend
--from lava_tool.utils import has_command
  class job(CommandGroup):
@@ -50,18 +43,25 @@
      def register_arguments(cls, parser):
          super(new, cls).register_arguments(parser)
          parser.add_argument("FILE", help=("Job file to be created."))
--
--    def invoke(self):
--        if os.path.exists(self.args.FILE):
--            raise CommandError('{0} already exists.'.format(self.args.FILE))
--
--        with open(self.args.FILE, 'w') as job_file:
--            job_instance = Job(BOOT_TEST)
--            job_instance.fill_in(self.config)
--            job_instance.write(job_file)
++        parser.add_argument("--type",
++                            help=("The type of job to create. Defaults to "
++                                  "'{0}'.".format(BOOT_TEST_KEY)),
++                            choices=JOB_TYPES.keys(),
++                            default=BOOT_TEST_KEY)
++
++    def invoke(self, job_template=None):
++        if not job_template:
++            job_template = JOB_TYPES.get(self.args.type)
++
++        full_path = os.path.abspath(self.args.FILE)
++
++        job_instance = Job(job_template, full_path)
++        job_instance.update(self.config)
++        job_instance.write()
  class submit(BaseCommand):
++
      """Submits the specified job file."""
      @classmethod
@@ -70,24 +70,11 @@
          parser.add_argument("FILE", help=("The job file to submit."))
      def invoke(self):
--        jobfile = self.args.FILE
--        jobdata = open(jobfile, 'rb').read()
--
--        server_name = Parameter('server')
--        rpc_endpoint = Parameter('rpc_endpoint', depends=server_name)
--        self.config.get(server_name)
--        endpoint = self.config.get(rpc_endpoint)
--
--        server = AuthenticatingServerProxy(endpoint,
--                                           auth_backend=KeyringAuthBackend())
--        try:
--            job_id = server.scheduler.submit_job(jobdata)
--            print >> sys.stdout, "Job submitted with job ID {0}".format(job_id)
--        except xmlrpclib.Fault, exc:
--            raise CommandError(str(exc))
++        super(submit, self).submit(self.args.FILE)
  class run(BaseCommand):
++
      """Runs the specified job file on the local dispatcher."""
      @classmethod
@@ -95,48 +82,5 @@
          super(run, cls).register_arguments(parser)
          parser.add_argument("FILE", help=("The job file to submit."))
--    @classmethod
--    def _choose_device(cls, devices):
--        """Let the user choose the device to use.
--
--        :param devices: The list of available devices.
--        :return The selected device.
--        """
--        devices_len = len(devices)
--        output_list = []
--        for device, number in zip(devices, range(1, devices_len + 1)):
--            output_list.append("\t{0}. {1}\n".format(number, device.hostname))
--
--        print >> sys.stdout, ("More than one local device found. "
--                              "Please choose one:\n")
--        print >> sys.stdout, "".join(output_list)
--
--        while True:
--            try:
--                user_input = raw_input("Device number to use: ").strip()
--
--                if user_input in [str(x) for x in range(1, devices_len + 1)]:
--                    return devices[int(user_input) - 1].hostname
--                else:
--                    continue
--            except EOFError:
--                user_input = None
--            except KeyboardInterrupt:
--                sys.exit(-1)
--
      def invoke(self):
--        if os.path.isfile(self.args.FILE):
--            if has_command("lava-dispatch"):
--                devices = get_devices()
--                if devices:
--                    if len(devices) > 1:
--                        device = self._choose_device(devices)
--                    else:
--                        device = devices[0].hostname
--                    self.run(["lava-dispatch", "--target", device,
--                              self.args.FILE])
--            else:
--                raise CommandError("Cannot find lava-dispatcher installation.")
--        else:
--            raise CommandError("The file '{0}' does not exists. or is not "
--                               "a file.".format(self.args.FILE))
++        super(run, self).run(self.args.FILE)
 === modified file 'lava/job/templates.py'
 --- lava/job/templates.py	2013-07-25 15:43:25 +0000
 +++ lava/job/templates.py	2013-07-25 15:43:26 +0000
@@ -16,19 +16,33 @@
  # You should have received a copy of the GNU Lesser General Public License
  # along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
--from lava.parameter import Parameter
--
--device_type = Parameter("device_type")
--prebuilt_image = Parameter("prebuilt_image", depends=device_type)
++from lava.parameter import (
++    ListParameter,
++    Parameter,
++)
++
++LAVA_TEST_SHELL_TAR_REPO_KEY = "tar-repo"
++LAVA_TEST_SHELL_TESDEF_KEY = "testdef"
++
++DEVICE_TYPE_PARAMETER = Parameter("device_type")
++PREBUILT_IMAGE_PARAMETER = Parameter("image", depends=DEVICE_TYPE_PARAMETER)
++
++TESTDEF_URLS_PARAMETER = ListParameter("testdef_urls")
++TESTDEF_URLS_PARAMETER.store = False
++
++# Use another ID for the server parameter, might be different.
++SERVER_PARAMETER = Parameter("stream_server")
++STREAM_PARAMETER = Parameter("stream")
  BOOT_TEST = {
++    "timeout": 18000,
      "job_name": "Boot test",
--    "device_type": device_type,
++    "device_type": DEVICE_TYPE_PARAMETER,
      "actions": [
+         {
              "command": "deploy_linaro_image",
              "parameters": {
--                "image": prebuilt_image
++                "image": PREBUILT_IMAGE_PARAMETER
+             }
          },
+         {
@@ -39,21 +53,72 @@
  LAVA_TEST_SHELL = {
      "job_name": "LAVA Test Shell",
--    "device_type": device_type,
--    "actions": [
--        {
--            "command": "deploy_linaro_image",
--            "parameters": {
--                "image": prebuilt_image,
--            }
--        },
--        {
--            "command": "lava_test_shell",
--            "parameters": {
--                "testdef_urls": [
--                    Parameter("testdef_url")
++    "timeout": 18000,
++    "device_type": DEVICE_TYPE_PARAMETER,
++    "actions": [
++        {
++            "command": "deploy_linaro_image",
++            "parameters": {
++                "image": PREBUILT_IMAGE_PARAMETER,
++            }
++        },
++        {
++            "command": "lava_test_shell",
++            "parameters": {
++                "timeout": 1800,
++                "testdef_urls": TESTDEF_URLS_PARAMETER,
++            }
++        },
++        {
++            "command": "submit_results",
++            "parameters" : {
++                "stream": STREAM_PARAMETER,
++                "server": SERVER_PARAMETER
++            }
++        }
++    ]
++}
++
++# This is a special case template, only use when automatically create job files
++# starting from a testdef or a script. Never to be used directly by the user.
++LAVA_TEST_SHELL_TAR_REPO = {
++    "job_name": "LAVA Test Shell",
++    "timeout": 18000,
++    "device_type": DEVICE_TYPE_PARAMETER,
++    "actions": [
++        {
++            "command": "deploy_linaro_image",
++            "parameters": {
++                "image": PREBUILT_IMAGE_PARAMETER,
++            }
++        },
++        {
++            "command": "lava_test_shell",
++            "parameters": {
++                "timeout": 1800,
++                "testdef_repos": [
++                        {
++                            LAVA_TEST_SHELL_TESDEF_KEY: None,
++                            LAVA_TEST_SHELL_TAR_REPO_KEY: None,
++                        }
+                 ]
+             }
++        },
++        {
++            "command": "submit_results",
++            "parameters" : {
++                "stream": STREAM_PARAMETER,
++                "server": SERVER_PARAMETER
++            }
+         }
+     ]
+ }
++
++BOOT_TEST_KEY = "boot-test"
++LAVA_TEST_SHELL_KEY = "lava-test-shell"
++
++# Dict with all the user available job templates.
++JOB_TYPES = {
++    BOOT_TEST_KEY: BOOT_TEST,
++    LAVA_TEST_SHELL_KEY: LAVA_TEST_SHELL,
++}
 === modified file 'lava/job/tests/test_commands.py'
 --- lava/job/tests/test_commands.py	2013-07-25 15:43:25 +0000
 +++ lava/job/tests/test_commands.py	2013-07-25 15:43:26 +0000
@@ -41,6 +41,7 @@
      def setUp(self):
          super(CommandTest, self).setUp()
          self.args.FILE = self.temp_file.name
++        self.args.type = "boot-test"
          self.device_type = Parameter('device_type')
          self.prebuilt_image = Parameter('prebuilt_image',
@@ -49,14 +50,6 @@
          self.config.put_parameter(self.device_type, 'foo')
          self.config.put_parameter(self.prebuilt_image, 'bar')
--    def tmp(self, filename):
--        """Returns a path to a non existent file.
--
--        :param filename: The name the file should have.
--        :return A path.
--        """
--        return os.path.join(self.temp_dir, filename)
--
  class JobNewTest(CommandTest):
@@ -106,7 +99,7 @@
          command = run(self.parser, self.args)
          self.assertRaises(CommandError, command.invoke)
--    @patch("lava.job.commands.has_command", new=MagicMock(return_value=False))
++    @patch("lava_tool.utils.has_command", new=MagicMock(return_value=False))
      def test_invoke_raises_1(self):
          # Users passes a valid file to the run command, but she does not have
          # the dispatcher installed.
 === modified file 'lava/job/tests/test_job.py'
 --- lava/job/tests/test_job.py	2013-07-25 15:43:25 +0000
 +++ lava/job/tests/test_job.py	2013-07-25 15:43:26 +0000
@@ -20,62 +20,73 @@
  Unit tests for the Job class
  """
++import os
  import json
--import os
  import tempfile
--from StringIO import StringIO
--from unittest import TestCase
++from mock import patch
  from lava.config import Config
++from lava.helper.tests.helper_test import HelperTest
  from lava.job import Job
  from lava.job.templates import BOOT_TEST
  from lava.parameter import Parameter
--class JobTest(TestCase):
++class JobTest(HelperTest):
--    def setUp(self):
--        self.config_file = tempfile.NamedTemporaryFile(delete=False)
++    @patch("lava.config.Config.save")
++    def setUp(self, mocked_config):
++        super(JobTest, self).setUp()
          self.config = Config()
--        self.config._config_file = self.config_file.name
--
--    def tearDown(self):
--        if os.path.isfile(self.config_file.name):
--            os.unlink(self.config_file.name)
++        self.config.config_file = self.temp_file.name
      def test_from_template(self):
          template = {}
--        job = Job(template)
++        job = Job(template, self.temp_file.name)
          self.assertEqual(job.data, template)
          self.assertIsNot(job.data, template)
--    def test_fill_in_data(self):
++    def test_update_data(self):
          image = "/path/to/panda.img"
          param1 = Parameter("device_type")
--        param2 = Parameter("prebuilt_image", depends=param1)
++        param2 = Parameter("image", depends=param1)
          self.config.put_parameter(param1, "panda")
          self.config.put_parameter(param2, image)
--        job = Job(BOOT_TEST)
--        job.fill_in(self.config)
++        job = Job(BOOT_TEST, self.temp_file.name)
++        job.update(self.config)
          self.assertEqual(job.data['device_type'], "panda")
          self.assertEqual(job.data['actions'][0]["parameters"]["image"], image)
      def test_write(self):
--        orig_data = {"foo": "bar"}
--        job = Job(orig_data)
--        output = StringIO()
--        job.write(output)
--
--        data = json.loads(output.getvalue())
--        self.assertEqual(data, orig_data)
++        try:
++            orig_data = {"foo": "bar"}
++            job_file = os.path.join(tempfile.gettempdir(), "a_json_file.json")
++            job = Job(orig_data, job_file)
++            job.write()
++
++            output = ""
++            with open(job_file) as read_file:
++                output = read_file.read()
++
++            data = json.loads(output)
++            self.assertEqual(data, orig_data)
++        finally:
++            os.unlink(job_file)
      def test_writes_nicely_formatted_json(self):
--        orig_data = {"foo": "bar"}
--        job = Job(orig_data)
--        output = StringIO()
--        job.write(output)
--
--        self.assertTrue(output.getvalue().startswith("{\n"))
++        try:
++            orig_data = {"foo": "bar"}
++            job_file = os.path.join(tempfile.gettempdir(), "b_json_file.json")
++            job = Job(orig_data, job_file)
++            job.write()
++
++            output = ""
++            with open(job_file) as read_file:
++                output = read_file.read()
++
++            self.assertTrue(output.startswith("{\n"))
++        finally:
++            os.unlink(job_file)
 === modified file 'lava/parameter.py'
 --- lava/parameter.py	2013-07-25 15:43:25 +0000
 +++ lava/parameter.py	2013-07-25 15:43:26 +0000
@@ -20,7 +20,19 @@
  Parameter class and its accessory methods/functions.
  """
++import StringIO
++import base64
++import os
  import sys
++import tarfile
++import tempfile
++import types
++
++from lava.tool.errors import CommandError
++from lava_tool.utils import to_list
++
++# Character used to join serialized list parameters.
++LIST_SERIALIZE_DELIMITER = ","
  class Parameter(object):
@@ -38,6 +50,15 @@
          self.value = value
          self.depends = depends
          self.asked = False
++        # Whether to store or not the parameter in the user config file.
++        self.store = True
++
++    def set(self, value):
++        """Sets the value of the parameter.
++
++        :param value: The value to set.
++        """
++        self.value = value
      def prompt(self, old_value=None):
          """Gets the parameter value from the user.
@@ -50,26 +71,187 @@
          :param old_value: The old parameter value.
          :return The input as typed by the user, or the old value.
          """
--        if old_value is not None:
--            prompt = "{0} [{1}]: ".format(self.id, old_value)
--        else:
--            prompt = "{0}: ".format(self.id)
--
--        user_input = None
++        if not self.asked:
++            if old_value is not None:
++                prompt = "{0} [{1}]: ".format(self.id, old_value)
++            else:
++                prompt = "{0}: ".format(self.id)
++
++            user_input = self.get_user_input(prompt)
++
++            if user_input is not None:
++                if len(user_input) == 0 and old_value:
++                    # Keep the old value when user press enter or another
++                    # whitespace char.
++                    self.value = old_value
++                else:
++                    self.value = user_input
++
++            self.asked = True
++
++        return self.value
++
++    @classmethod
++    def get_user_input(cls, prompt=""):
++        """Asks the user for input data.
++
++        :param prompt: The prompt that should be given to the user.
++        :return A string with what the user typed.
++        """
++        data = None
          try:
--            user_input = raw_input(prompt).strip()
++            data = raw_input(prompt).strip()
          except EOFError:
--            pass
++            # Force to return None.
++            data = None
          except KeyboardInterrupt:
              sys.exit(-1)
--
--        if user_input is not None:
++        return data
++
++    @classmethod
++    def serialize(cls, value):
++        """Serializes the passed value to be friendly written to file.
++
++        Lists are serialized as a comma separated string of values.
++
++        :param value: The value to serialize.
++        :return The serialized value as string.
++        """
++        serialized = ""
++        if isinstance(value, list):
++            serialized = LIST_SERIALIZE_DELIMITER.join(
++                str(x) for x in value if x)
++        else:
++            serialized = str(value)
++        return serialized
++
++    @classmethod
++    def deserialize(cls, value):
++        """Deserialize a value into a list.
++
++        The value must have been serialized with the class instance serialize()
++        method.
++
++        :param value: The string value to be deserialized.
++        :type str
++        :return A list of values.
++        """
++        deserialized = []
++        if isinstance(value, types.StringTypes):
++            deserialized = filter(None, (x.strip() for x in value.split(
++                LIST_SERIALIZE_DELIMITER)))
++        else:
++            deserialized = list(value)
++        return deserialized
++
++
++class SingleChoiceParameter(Parameter):
++    """A parameter implemeting a single choice between multiple choices."""
++    def __init__(self, id, choices):
++        super(SingleChoiceParameter, self).__init__(id)
++        self.choices = to_list(choices)
++
++    def prompt(self, prompt, old_value=None):
++        """Asks the user for their choice."""
++        # Sliglty different than the other parameters: here we first present
++        # the user with what the choices are about.
++        print >> sys.stdout, prompt
++
++        index = 1
++        for choice in self.choices:
++            print >> sys.stdout, "\t{0:d}. {1}".format(index, choice)
++            index += 1
++
++        choices_len = len(self.choices)
++        while True:
++            user_input = self.get_user_input("Choice: ")
++
              if len(user_input) == 0 and old_value:
--                # Keep the old value when user press enter or another
--                # whitespace char.
--                self.value = old_value
--            else:
--                self.value = user_input
--
--        self.asked = True
++                choice = old_value
++                break
++            elif user_input in [str(x) for x in range(1, choices_len + 1)]:
++                choice = self.choices[int(user_input) - 1]
++                break
++
++        return choice
++
++
++class ListParameter(Parameter):
++    """A specialized Parameter to handle list values."""
++
++    # This is used as a deletion character. When we have an old value and the
++    # user enters this char, it sort of deletes the value.
++    DELETE_CHAR = "-"
++
++    def __init__(self, id, value=None, depends=None):
++        super(ListParameter, self).__init__(id, depends=depends)
++        self.value = []
++        if value:
++            self.set(value)
++
++    def set(self, value):
++        """Sets the value of the parameter.
++
++        :param value: The value to set.
++        """
++        self.value = to_list(value)
++
++    def add(self, value):
++        """Adds a new value to the list of values of this parameter.
++
++        :param value: The value to add.
++        """
++        if isinstance(value, list):
++            self.value.extend(value)
++        else:
++            self.value.append(value)
++
++    def prompt(self, old_value=None):
++        """Gets the parameter in a list form.
++
++        To exit the input procedure it is necessary to insert an empty line.
++
++        :return The list of values.
++        """
++
++        if not self.asked:
++            if old_value is not None:
++                # We might get the old value read from file via ConfigParser,
++                # and usually it comes in string format.
++                old_value = self.deserialize(old_value)
++
++            print >> sys.stdout, "Values for '{0}': ".format(self.id)
++
++            index = 1
++            while True:
++                user_input = None
++                if old_value is not None and (0 < len(old_value) >= index):
++                    prompt = "{0:>3d}.\n\told: {1}\n\tnew: ".format(
++                        index, old_value[index-1])
++                    user_input = self.get_user_input(prompt)
++                else:
++                    prompt = "{0:>3d}. ".format(index)
++                    user_input = self.get_user_input(prompt)
++
++                if user_input is not None:
++                    # The user has pressed Enter.
++                    if len(user_input) == 0:
++                        if old_value is not None and \
++                                (0 < len(old_value) >= index):
++                            user_input = old_value[index-1]
++                        else:
++                            break
++
++                    if len(user_input) == 1 and user_input == \
++                            self.DELETE_CHAR and (0 < len(old_value) >= index):
++                        # We have an old value, user presses the DELETE_CHAR
++                        # and we do not store anything. This is done to delete
++                        # an old entry.
++                        pass
++                    else:
++                        self.value.append(user_input)
++                    index += 1
++
++            self.asked = True
++
          return self.value
 === added directory 'lava/testdef'
 === added file 'lava/testdef/__init__.py'
 --- lava/testdef/__init__.py	1970-01-01 00:00:00 +0000
 +++ lava/testdef/__init__.py	2013-07-25 15:43:26 +0000
@@ -0,0 +1,60 @@
++# Copyright (C) 2013 Linaro Limited
++#
++# Author: Milo Casagrande <milo.casagrande@linaro.org>
++#
++# This file is part of lava-tool.
++#
++# lava-tool is free software: you can redistribute it and/or modify
++# it under the terms of the GNU Lesser General Public License version 3
++# as published by the Free Software Foundation
++#
++# lava-tool is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++# GNU General Public License for more details.
++#
++# You should have received a copy of the GNU Lesser General Public License
++# along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
++
++import yaml
++
++from copy import deepcopy
++
++from lava.helper.template import expand_template
++from lava_tool.utils import (
++    write_file,
++    verify_path_existance,
++    verify_file_extension
++)
++
++# Default test def file extension.
++DEFAULT_TESTDEF_EXTENSION = "yaml"
++# Possible extensions for a test def file.
++TESTDEF_FILE_EXTENSIONS = [DEFAULT_TESTDEF_EXTENSION]
++
++
++class TestDefinition(object):
++
++    def __init__(self, data, file_name):
++        """Initialize the object.
++
++        :param data: The serializable data to be used, usually a template.
++        :type dict
++        :param file_name: Where the test definition will be written.
++        :type str
++        """
++        self.file_name = verify_file_extension(file_name,
++                                               DEFAULT_TESTDEF_EXTENSION,
++                                               TESTDEF_FILE_EXTENSIONS)
++        verify_path_existance(self.file_name)
++
++        self.data = deepcopy(data)
++
++    def write(self):
++        """Writes the test definition to file."""
++        content = yaml.dump(self.data, default_flow_style=False, indent=4)
++        write_file(self.file_name, content)
++
++    def update(self, config):
++        """Updates the TestDefinition object based on the provided config."""
++        expand_template(self.data, config)
 === added file 'lava/testdef/commands.py'
 --- lava/testdef/commands.py	1970-01-01 00:00:00 +0000
 +++ lava/testdef/commands.py	2013-07-25 15:43:26 +0000
@@ -0,0 +1,72 @@
++# Copyright (C) 2013 Linaro Limited
++#
++# Author: Milo Casagrande <milo.casagrande@linaro.org>
++#
++# This file is part of lava-tool.
++#
++# lava-tool is free software: you can redistribute it and/or modify
++# it under the terms of the GNU Lesser General Public License version 3
++# as published by the Free Software Foundation
++#
++# lava-tool is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++# GNU General Public License for more details.
++#
++# You should have received a copy of the GNU Lesser General Public License
++# along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
++
++"""
++Test definition commands class.
++"""
++
++import os
++
++from lava.helper.command import BaseCommand
++from lava.tool.command import CommandGroup
++
++
++class testdef(CommandGroup):
++
++    """LAVA test definitions handling."""
++
++    namespace = "lava.testdef.commands"
++
++
++class new(BaseCommand):
++
++    """Creates a new test definition file."""
++
++    @classmethod
++    def register_arguments(cls, parser):
++        super(new, cls).register_arguments(parser)
++        parser.add_argument("FILE", help="Test definition file to create.")
++
++    def invoke(self):
++        full_path = os.path.abspath(self.args.FILE)
++        self.create_test_definition(full_path)
++
++
++class run(BaseCommand):
++
++    """Runs the specified test definition on a local device."""
++
++    @classmethod
++    def register_arguments(cls, parser):
++        super(run, cls).register_arguments(parser)
++        parser.add_argument("FILE", help="Test definition file to run.")
++
++    def invoke(self):
++        pass
++
++
++def submit(BaseCommand):
++    """Submits the specified test definition to a remove LAVA server."""
++
++    @classmethod
++    def register_arguments(cls, parser):
++        super(submit, cls).register_arguments(parser)
++        parser.add_argument("FILE", help="Test definition file to send.")
++
++    def invoke(self):
++        pass
 === added file 'lava/testdef/templates.py'
 --- lava/testdef/templates.py	1970-01-01 00:00:00 +0000
 +++ lava/testdef/templates.py	2013-07-25 15:43:26 +0000
@@ -0,0 +1,75 @@
++# Copyright (C) 2013 Linaro Limited
++#
++# Author: Milo Casagrande <milo.casagrande@linaro.org>
++#
++# This file is part of lava-tool.
++#
++# lava-tool is free software: you can redistribute it and/or modify
++# it under the terms of the GNU Lesser General Public License version 3
++# as published by the Free Software Foundation
++#
++# lava-tool is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++# GNU General Public License for more details.
++#
++# You should have received a copy of the GNU Lesser General Public License
++# along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
++
++"""Test definition templates."""
++
++from lava.parameter import (
++    ListParameter,
++    Parameter,
++)
++
++DEFAULT_TESTDEF_FILE = "lavatest.yaml"
++
++DEFAULT_TESTDEF_VERSION = "1.0"
++DEFAULT_TESTDEF_FORMAT = "Lava-Test Test Definition 1.0"
++
++# This is what will be called by default by the test definition yaml file.
++DEFAULT_TESTDEF_SCRIPT = "mytest.sh"
++DEFAULT_TESTDEF_SCRIPT_CONTENT = """#!/bin/sh
++# Automatic generated content by lava-tool.
++# Please add your own instructions.
++"""
++DEFAULT_TESTDEF_STEP = "./mytest.sh"
++
++DEFAULT_ENVIRONMET_VALUE = "lava_test_shell"
++
++# All these parameters will not be stored on the local config file.
++NAME_PARAMETER = Parameter("name")
++NAME_PARAMETER.store = False
++
++DESCRIPTION_PARAMETER = Parameter("description", depends=NAME_PARAMETER)
++DESCRIPTION_PARAMETER.store = False
++
++ENVIRONMENT_PARAMETER = ListParameter("environment",
++                                      depends=NAME_PARAMETER)
++ENVIRONMENT_PARAMETER.add(DEFAULT_ENVIRONMET_VALUE)
++ENVIRONMENT_PARAMETER.asked = True
++ENVIRONMENT_PARAMETER.store = False
++
++# Steps parameter. Default to a local shell script that the user defines.
++# We do not ask this parameter, and we do not store it either.
++STEPS_PARAMETER = ListParameter("steps", depends=NAME_PARAMETER)
++STEPS_PARAMETER.add(DEFAULT_TESTDEF_STEP)
++STEPS_PARAMETER.asked = True
++STEPS_PARAMETER.store = False
++
++TESTDEF_TEMPLATE = {
++    "metadata": {
++        "name": NAME_PARAMETER,
++        "format": DEFAULT_TESTDEF_FORMAT,
++        "version": DEFAULT_TESTDEF_VERSION,
++        "description": DESCRIPTION_PARAMETER,
++        "environment": ENVIRONMENT_PARAMETER,
++    },
++    "run": {
++        "steps": STEPS_PARAMETER,
++    },
++    "parse": {
++        "pattern": '^\s*(?P<test_case_id>\w+)=(?P<result>\w+)\s*$'
++    }
++}
 === added directory 'lava/testdef/tests'
 === added file 'lava/testdef/tests/__init__.py'
 === added file 'lava/testdef/tests/test_commands.py'
 --- lava/testdef/tests/test_commands.py	1970-01-01 00:00:00 +0000
 +++ lava/testdef/tests/test_commands.py	2013-07-25 15:43:26 +0000
@@ -0,0 +1,153 @@
++# Copyright (C) 2013 Linaro Limited
++#
++# Author: Milo Casagrande <milo.casagrande@linaro.org>
++#
++# This file is part of lava-tool.
++#
++# lava-tool is free software: you can redistribute it and/or modify
++# it under the terms of the GNU Lesser General Public License version 3
++# as published by the Free Software Foundation
++#
++# lava-tool is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++# GNU General Public License for more details.
++#
++# You should have received a copy of the GNU Lesser General Public License
++# along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
++
++"""
++Tests for lava.testdef.commands.
++"""
++
++import os
++import tempfile
++import yaml
++
++from mock import (
++    patch,
++)
++
++from lava.config import InteractiveConfig
++from lava.helper.tests.helper_test import HelperTest
++from lava.testdef.commands import (
++    new,
++)
++from lava.tool.errors import CommandError
++
++
++class NewCommandTest(HelperTest):
++    """Class for the lava.testdef new command tests."""
++
++    @patch("lava.config.Config.save")
++    def setUp(self, mocked_save):
++        super(NewCommandTest, self).setUp()
++        self.file_name = "fake_testdef.yaml"
++        self.file_path = os.path.join(tempfile.gettempdir(), self.file_name)
++        self.args.FILE = self.file_path
++
++        self.temp_yaml = tempfile.NamedTemporaryFile(suffix=".yaml",
++                                                    delete=False)
++
++        self.config_file = tempfile.NamedTemporaryFile(delete=False)
++        self.config = InteractiveConfig()
++        self.config.config_file = self.config_file.name
++        # Patch class raw_input, start it, and stop it on tearDown.
++        self.patcher1 = patch("lava.parameter.raw_input", create=True)
++        self.mocked_raw_input = self.patcher1.start()
++
++    def tearDown(self):
++        super(NewCommandTest, self).tearDown()
++        if os.path.isfile(self.file_path):
++            os.unlink(self.file_path)
++        os.unlink(self.config_file.name)
++        os.unlink(self.temp_yaml.name)
++        self.patcher1.stop()
++
++    def test_register_arguments(self):
++        # Make sure that the parser add_argument is called and we have the
++        # correct argument.
++        new_command = new(self.parser, self.args)
++        new_command.register_arguments(self.parser)
++
++        # Make sure we do not forget about this test.
++        self.assertEqual(2, len(self.parser.method_calls))
++
++        _, args, _ = self.parser.method_calls[0]
++        self.assertIn("--non-interactive", args)
++
++        _, args, _ = self.parser.method_calls[1]
++        self.assertIn("FILE", args)
++
++    def test_invoke_0(self):
++        # Test that passing a file on the command line, it is created on the
++        # file system.
++        self.mocked_raw_input.return_value = "\n"
++        new_command = new(self.parser, self.args)
++        new_command.invoke()
++        self.assertTrue(os.path.exists(self.file_path))
++
++    def test_invoke_1(self):
++        # Test that when passing an already existing file, an exception is
++        # thrown.
++        self.args.FILE = self.temp_yaml.name
++        new_command = new(self.parser, self.args)
++        self.assertRaises(CommandError, new_command.invoke)
++
++    def test_invoke_2(self):
++        # Tests that when adding a new test definition and writing it to file
++        # a correct YAML structure is created.
++        self.mocked_raw_input.return_value = "\n"
++        new_command = new(self.parser, self.args)
++        new_command.config = self.config
++        new_command.invoke()
++        expected = {'run': {'steps': ["./mytest.sh"]},
++                    'metadata': {
++                        'environment': ['lava_test_shell'],
++                        'format': 'Lava-Test Test Definition 1.0',
++                        'version': '1.0',
++                        'description': '',
++                        'name': ''},
++                    'parse': {
++                        'pattern':
++                            '^\\s*(?P<test_case_id>\\w+)=(?P<result>\\w+)\\s*$'
++                        },
++                    }
++        obtained = None
++        with open(self.file_path, 'r') as read_file:
++            obtained = yaml.load(read_file)
++        self.assertEqual(expected, obtained)
++
++    def test_invoke_3(self):
++        # Tests that when adding a new test definition and writing it to a file
++        # in a directory withour permissions, exception is raised.
++        self.args.FILE = "/test_file.yaml"
++        self.mocked_raw_input.return_value = "\n"
++        new_command = new(self.parser, self.args)
++        self.assertRaises(CommandError, new_command.invoke)
++        self.assertFalse(os.path.exists(self.args.FILE))
++
++    def test_invoke_4(self):
++        # Tests that when passing values for the "steps" ListParameter, we get
++        # back the correct data structure.
++        self.mocked_raw_input.side_effect = ["foo", "\n", "\n", "\n", "\n",
++                                             "\n"]
++        new_command = new(self.parser, self.args)
++        new_command.invoke()
++        expected = {'run': {'steps': ["./mytest.sh"]},
++                    'metadata': {
++                        'environment': ['lava_test_shell'],
++                        'format': 'Lava-Test Test Definition 1.0',
++                        'version': '1.0',
++                        'description': '',
++                        'name': 'foo'
++                    },
++                    'parse': {
++                        'pattern':
++                            '^\\s*(?P<test_case_id>\\w+)=(?P<result>\\w+)\\s*$'
++                        },
++                    }
++        obtained = None
++        with open(self.file_path, 'r') as read_file:
++            obtained = yaml.load(read_file)
++        self.assertEqual(expected, obtained)
 === added file 'lava/tests/test_commands.py'
 --- lava/tests/test_commands.py	1970-01-01 00:00:00 +0000
 +++ lava/tests/test_commands.py	2013-07-25 15:43:26 +0000
@@ -0,0 +1,128 @@
++# Copyright (C) 2013 Linaro Limited
++#
++# Author: Milo Casagrande <milo.casagrande@linaro.org>
++#
++# This file is part of lava-tool.
++#
++# lava-tool is free software: you can redistribute it and/or modify
++# it under the terms of the GNU Lesser General Public License version 3
++# as published by the Free Software Foundation
++#
++# lava-tool is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++# GNU General Public License for more details.
++#
++# You should have received a copy of the GNU Lesser General Public License
++# along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
++
++"""
++Tests for lava.commands.
++"""
++
++import os
++import tempfile
++
++from mock import (
++    MagicMock,
++    patch
++)
++
++from lava.commands import (
++    init,
++    submit,
++)
++from lava.config import Config
++from lava.helper.tests.helper_test import HelperTest
++from lava.tool.errors import CommandError
++
++
++class InitCommandTests(HelperTest):
++
++    def setUp(self):
++        super(InitCommandTests, self).setUp()
++        self.config_file = self.tmp("init_command_tests")
++        self.config = Config()
++        self.config.config_file = self.config_file
++
++    def tearDown(self):
++        super(InitCommandTests, self).tearDown()
++        if os.path.isfile(self.config_file):
++            os.unlink(self.config_file)
++
++    def test_register_arguments(self):
++        self.args.DIR = os.path.join(tempfile.gettempdir(), "a_fake_dir")
++        init_command = init(self.parser, self.args)
++        init_command.register_arguments(self.parser)
++
++        # Make sure we do not forget about this test.
++        self.assertEqual(2, len(self.parser.method_calls))
++
++        _, args, _ = self.parser.method_calls[0]
++        self.assertIn("--non-interactive", args)
++
++        _, args, _ = self.parser.method_calls[1]
++        self.assertIn("DIR", args)
++
++
++    @patch("lava.commands.edit_file", create=True)
++    def test_command_invoke_0(self, mocked_edit_file):
++        # Invoke the init command passing a path to a file. Should raise an
++        # exception.
++        self.args.DIR = self.temp_file.name
++        init_command = init(self.parser, self.args)
++        self.assertRaises(CommandError, init_command.invoke)
++
++    def test_command_invoke_2(self):
++        # Invoke the init command passing a path where the user cannot write.
++        try:
++            self.args.DIR = "/root/a_temp_dir"
++            init_command = init(self.parser, self.args)
++            self.assertRaises(CommandError, init_command.invoke)
++        finally:
++            if os.path.exists(self.args.DIR):
++                os.removedirs(self.args.DIR)
++
++    def test_update_data(self):
++        # Make sure the template is updated accordingly with the provided data.
++        self.args.DIR = self.temp_file.name
++
++        init_command = init(self.parser, self.args)
++        init_command.config.get = MagicMock()
++        init_command.config.save = MagicMock()
++        init_command.config.get.side_effect = ["a_job.json"]
++
++        expected = {
++            "jobfile": "a_job.json",
++        }
++
++        obtained = init_command._update_data()
++        self.assertEqual(expected, obtained)
++
++
++class SubmitCommandTests(HelperTest):
++    def setUp(self):
++        super(SubmitCommandTests, self).setUp()
++        self.config_file = self.tmp("submit_command_tests")
++        self.config = Config()
++        self.config.config_file = self.config_file
++        self.config.save = MagicMock()
++
++    def tearDown(self):
++        super(SubmitCommandTests, self).tearDown()
++        if os.path.isfile(self.config_file):
++            os.unlink(self.config_file)
++
++    def test_register_arguments(self):
++        self.args.JOB = os.path.join(tempfile.gettempdir(), "a_fake_file")
++        submit_command = submit(self.parser, self.args)
++        submit_command.register_arguments(self.parser)
++
++        # Make sure we do not forget about this test.
++        self.assertEqual(2, len(self.parser.method_calls))
++
++        _, args, _ = self.parser.method_calls[0]
++        self.assertIn("--non-interactive", args)
++
++        _, args, _ = self.parser.method_calls[1]
++        self.assertIn("JOB", args)
 === modified file 'lava/tests/test_config.py'
 --- lava/tests/test_config.py	2013-07-25 15:43:25 +0000
 +++ lava/tests/test_config.py	2013-07-25 15:43:26 +0000
@@ -20,69 +20,46 @@
  lava.config unit tests.
  """
--import os
  import sys
--import tempfile
  from StringIO import StringIO
--from mock import MagicMock, patch, call
++from mock import (
++    MagicMock,
++    call,
++    patch,
++)
  from lava.config import (
      Config,
      InteractiveConfig,
--    ConfigParser,
+ )
  from lava.helper.tests.helper_test import HelperTest
--from lava.parameter import Parameter
++from lava.parameter import (
++    Parameter,
++    ListParameter,
++)
  from lava.tool.errors import CommandError
--class MockedConfig(Config):
--    """A subclass of the original Config class.
--
--    Used to test the Config class, but to not have the same constructor in
--    order to use temporary files for the configuration.
--    """
--    def __init__(self, config_file):
--        self._cache = {}
--        self._config_file = config_file
--        self._config_backend = ConfigParser()
--        self._config_backend.read([self._config_file])
--
--
--class MockedInteractiveConfig(InteractiveConfig):
--    def __init__(self, config_file, force_interactive=False):
--        self._cache = {}
--        self._config_file = config_file
--        self._config_backend = ConfigParser()
--        self._config_backend.read([self._config_file])
--        self._force_interactive = force_interactive
--
--
  class ConfigTestCase(HelperTest):
      """General test case class for the different Config classes."""
      def setUp(self):
          super(ConfigTestCase, self).setUp()
--        self.config_file = tempfile.NamedTemporaryFile(delete=False)
--
          self.param1 = Parameter("foo")
          self.param2 = Parameter("bar", depends=self.param1)
--    def tearDown(self):
--        super(ConfigTestCase, self).tearDown()
--        if os.path.isfile(self.config_file.name):
--            os.unlink(self.config_file.name)
--
  class ConfigTest(ConfigTestCase):
--    def setUp(self):
++    @patch("lava.config.Config.save")
++    def setUp(self, mocked_save):
          super(ConfigTest, self).setUp()
--        self.config = MockedConfig(self.config_file.name)
++        self.config = Config()
++        self.config.config_file = self.temp_file.name
      def test_assert_temp_config_file(self):
          # Dummy test to make sure we are overriding correctly the Config class.
--        self.assertEqual(self.config._config_file, self.config_file.name)
++        self.assertEqual(self.config.config_file, self.temp_file.name)
      def test_config_put_in_cache_0(self):
          self.config._put_in_cache("key", "value", "section")
@@ -167,46 +144,38 @@
          expected = "[DEFAULT]\nfoo = foo\n\n"
          obtained = ""
--        with open(self.config_file.name) as tmp_file:
++        with open(self.temp_file.name) as tmp_file:
              obtained = tmp_file.read()
          self.assertEqual(expected, obtained)
--    @patch("lava.config.AT_EXIT_CALLS", spec=set)
--    def test_config_atexit_call_list(self, mocked_calls):
--        # Tests that the save() method is added to the set of atexit calls.
--        config = Config()
--        config._config_file = self.config_file.name
--        config.put_parameter(self.param1, "foo")
--
--        expected = [call.add(config.save)]
--
--        self.assertEqual(expected, mocked_calls.mock_calls)
--
  class InteractiveConfigTest(ConfigTestCase):
--    def setUp(self):
++    @patch("lava.config.Config.save")
++    def setUp(self, mocked_save):
          super(InteractiveConfigTest, self).setUp()
--        self.config = MockedInteractiveConfig(
--            config_file=self.config_file.name)
++        self.config = InteractiveConfig()
++        self.config.config_file = self.temp_file.name
      @patch("lava.config.Config.get", new=MagicMock(return_value=None))
      def test_non_interactive_config_0(self):
--        # Mocked config default is not to be interactive.
          # Try to get a value that does not exists, users just press enter when
          # asked for a value. Value will be empty.
++        self.config.force_interactive = False
          sys.stdin = StringIO("\n")
          value = self.config.get(Parameter("foo"))
          self.assertEqual("", value)
      @patch("lava.config.Config.get", new=MagicMock(return_value="value"))
      def test_non_interactive_config_1(self):
--        # Parent class config returns a value, but we are not interactive.
++        # Parent class config returns value, but we are not interactive.
++        self.config.force_interactive = False
          value = self.config.get(Parameter("foo"))
          self.assertEqual("value", value)
      @patch("lava.config.Config.get", new=MagicMock(return_value=None))
      def test_non_interactive_config_2(self):
++        self.config.force_interactive = False
          expected = "bar"
          sys.stdin = StringIO(expected)
          value = self.config.get(Parameter("foo"))
@@ -216,7 +185,7 @@
      def test_interactive_config_0(self):
          # We force to be interactive, meaning that even if a value is found,
          # it will be asked anyway.
--        self.config._force_interactive = True
++        self.config.force_interactive = True
          expected = "a_new_value"
          sys.stdin = StringIO(expected)
          value = self.config.get(Parameter("foo"))
@@ -226,28 +195,28 @@
      def test_interactive_config_1(self):
          # Force to be interactive, but when asked for the new value press
          # Enter. The old value should be returned.
--        self.config._force_interactive = True
++        self.config.force_interactive = True
          sys.stdin = StringIO("\n")
          value = self.config.get(Parameter("foo"))
          self.assertEqual("value", value)
      def test_calculate_config_section_0(self):
--        self.config._force_interactive = True
++        self.config.force_interactive = True
          obtained = self.config._calculate_config_section(self.param1)
          expected = "DEFAULT"
          self.assertEqual(expected, obtained)
      def test_calculate_config_section_1(self):
++        self.param1.set("foo")
          self.param2.depends.asked = True
--        self.config._force_interactive = True
--        self.config.put(self.param1.id, "foo")
++        self.config.force_interactive = True
          obtained = self.config._calculate_config_section(self.param2)
          expected = "foo=foo"
          self.assertEqual(expected, obtained)
      def test_calculate_config_section_2(self):
--        self.config._force_interactive = True
--        self.config._config_backend.get = MagicMock(return_value=None)
++        self.config.force_interactive = True
++        self.config.config_backend.get = MagicMock(return_value=None)
          sys.stdin = StringIO("baz")
          expected = "foo=baz"
          obtained = self.config._calculate_config_section(self.param2)
@@ -256,10 +225,9 @@
      def test_calculate_config_section_3(self):
          # Tests that when a parameter has its value in the cache and also on
          # file, we honor the cached version.
++        self.param1.set("bar")
          self.param2.depends.asked = True
--        self.config._force_interactive = True
--        self.config._get_from_cache = MagicMock(return_value="bar")
--        self.config._config_backend.get = MagicMock(return_value="baz")
++        self.config.force_interactive = True
          expected = "foo=bar"
          obtained = self.config._calculate_config_section(self.param2)
          self.assertEqual(expected, obtained)
@@ -273,6 +241,32 @@
          mocked_raw.side_effect = KeyboardInterrupt()
--        self.config._force_interactive = True
++        self.config.force_interactive = True
          self.config.get(self.param1)
          self.assertTrue(mocked_sys_exit.called)
++
++    @patch("lava.parameter.raw_input", create=True)
++    def test_interactive_config_with_list_parameter(self, mocked_raw_input):
++        # Tests that we get a list back in the Config class when using
++        # ListParameter and that it contains the expected values.
++        expected = ["foo", "bar"]
++        mocked_raw_input.side_effect = expected + ["\n"]
++        obtained = self.config.get(ListParameter("list"))
++        self.assertIsInstance(obtained, list)
++        self.assertEqual(expected, obtained)
++
++    def test_interactive_save_list_param(self):
++        # Tests that when saved to file, the ListParameter parameter is stored
++        # correctly.
++        param_values = ["foo", "more than one words", "bar"]
++        list_param = ListParameter("list")
++        list_param.set(param_values)
++
++        self.config.put_parameter(list_param, param_values)
++        self.config.save()
++
++        expected = "[DEFAULT]\nlist = " + ",".join(param_values) + "\n\n"
++        obtained = ""
++        with open(self.temp_file.name, "r") as read_file:
++            obtained = read_file.read()
++        self.assertEqual(expected, obtained)
 === modified file 'lava/tests/test_parameter.py'
 --- lava/tests/test_parameter.py	2013-07-25 15:43:25 +0000
 +++ lava/tests/test_parameter.py	2013-07-25 15:43:26 +0000
@@ -20,16 +20,32 @@
  lava.parameter unit tests.
  """
--import sys
--
--from StringIO import StringIO
  from mock import patch
  from lava.helper.tests.helper_test import HelperTest
--from lava.parameter import Parameter
--
--
--class ParameterTest(HelperTest):
++from lava.parameter import (
++    ListParameter,
++    Parameter,
++)
++
++from lava_tool.utils import to_list
++
++
++class GeneralParameterTest(HelperTest):
++    """General class with setUp and tearDown methods for Parameter tests."""
++    def setUp(self):
++        super(GeneralParameterTest, self).setUp()
++        # Patch class raw_input, start it, and stop it on tearDown.
++        self.patcher1 = patch("lava.parameter.raw_input", create=True)
++        self.mocked_raw_input = self.patcher1.start()
++
++    def tearDown(self):
++        super(GeneralParameterTest, self).tearDown()
++        self.patcher1.stop()
++
++
++class ParameterTest(GeneralParameterTest):
++    """Tests for the Parameter class."""
      def setUp(self):
          super(ParameterTest, self).setUp()
@@ -38,14 +54,79 @@
      def test_prompt_0(self):
          # Tests that when we have a value in the parameters and the user press
          # Enter, we get the old value back.
--        sys.stdin = StringIO("\n")
++        self.mocked_raw_input.return_value = "\n"
          obtained = self.parameter1.prompt()
          self.assertEqual(self.parameter1.value, obtained)
--    @patch("lava.parameter.raw_input", create=True)
--    def test_prompt_1(self, mocked_raw_input):
++    def test_prompt_1(self,):
          # Tests that with a value stored in the parameter, if and EOFError is
          # raised when getting user input, we get back the old value.
--        mocked_raw_input.side_effect = EOFError()
++        self.mocked_raw_input.side_effect = EOFError()
          obtained = self.parameter1.prompt()
          self.assertEqual(self.parameter1.value, obtained)
++
++    def test_to_list_0(self):
++        value = "a_value"
++        expected = [value]
++        obtained = to_list(value)
++        self.assertIsInstance(obtained, list)
++        self.assertEquals(expected, obtained)
++
++    def test_to_list_1(self):
++        expected = ["a_value", "b_value"]
++        obtained = to_list(expected)
++        self.assertIsInstance(obtained, list)
++        self.assertEquals(expected, obtained)
++
++class ListParameterTest(GeneralParameterTest):
++    """Tests for the specialized ListParameter class."""
++
++    def setUp(self):
++        super(ListParameterTest, self).setUp()
++        self.list_parameter = ListParameter("list")
++
++    def test_prompt_0(self):
++        # Test that when pressing Enter, the prompt stops and the list is
++        # returned.
++        expected = []
++        self.mocked_raw_input.return_value = "\n"
++        obtained = self.list_parameter.prompt()
++        self.assertEqual(expected, obtained)
++
++    def test_prompt_1(self):
++        # Tests that when passing 3 values, a list with those values
++        # is returned
++        expected = ["foo", "bar", "foobar"]
++        self.mocked_raw_input.side_effect = expected + ["\n"]
++        obtained = self.list_parameter.prompt()
++        self.assertEqual(expected, obtained)
++
++    def test_serialize_0(self):
++        # Tests the serialize method of ListParameter passing a list.
++        expected = "foo,bar,baz,1"
++        to_serialize = ["foo", "bar", "baz", "", 1]
++
++        obtained = self.list_parameter.serialize(to_serialize)
++        self.assertEqual(expected, obtained)
++
++    def test_serialize_1(self):
++        # Tests the serialize method of ListParameter passing an int.
++        expected = "1"
++        to_serialize = 1
++
++        obtained = self.list_parameter.serialize(to_serialize)
++        self.assertEqual(expected, obtained)
++
++    def test_deserialize_0(self):
++        # Tests the deserialize method of ListParameter with a string
++        # of values.
++        expected = ["foo", "bar", "baz"]
++        to_deserialize = "foo,bar,,baz,"
++        obtained = self.list_parameter.deserialize(to_deserialize)
++        self.assertEqual(expected, obtained)
++
++    def test_deserialize_1(self):
++        # Tests the deserialization method of ListParameter passing a list.
++        expected = ["foo", 1, "", "bar"]
++        obtained = self.list_parameter.deserialize(expected)
++        self.assertEqual(expected, obtained)
 === modified file 'lava_tool/tests/__init__.py'
 --- lava_tool/tests/__init__.py	2013-07-25 15:43:25 +0000
 +++ lava_tool/tests/__init__.py	2013-07-25 15:43:26 +0000
@@ -35,19 +35,22 @@
  def test_modules():
      return [
++        'lava.device.tests.test_commands',
++        'lava.device.tests.test_device',
++        'lava.helper.tests.test_command',
++        'lava.helper.tests.test_dispatcher',
++        'lava.helper.tests.test_template',
++        'lava.job.tests.test_commands',
++        'lava.job.tests.test_job',
++        'lava.testdef.tests.test_commands',
++        'lava.tests.test_commands',
++        'lava.tests.test_config',
++        'lava.tests.test_parameter',
++        'lava_dashboard_tool.tests.test_commands',
++        'lava_tool.tests.test_auth_commands',
          'lava_tool.tests.test_authtoken',
--        'lava_tool.tests.test_auth_commands',
          'lava_tool.tests.test_commands',
          'lava_tool.tests.test_utils',
--        'lava_dashboard_tool.tests.test_commands',
--        'lava.job.tests.test_job',
--        'lava.job.tests.test_commands',
--        'lava.device.tests.test_device',
--        'lava.device.tests.test_commands',
--        'lava.tests.test_config',
--        'lava.tests.test_parameter',
--        'lava.helper.tests.test_command',
--        'lava.helper.tests.test_dispatcher',
+     ]
 === modified file 'lava_tool/tests/test_utils.py'
 --- lava_tool/tests/test_utils.py	2013-07-25 15:43:25 +0000
 +++ lava_tool/tests/test_utils.py	2013-07-25 15:43:26 +0000
@@ -18,16 +18,45 @@
  """lava_tool.utils tests."""
++import sys
++import os
  import subprocess
++import tempfile
  from unittest import TestCase
--from mock import patch
++from mock import (
++    MagicMock,
++    call,
++    patch,
++)
--from lava_tool.utils import has_command
++from lava.tool.errors import CommandError
++from lava_tool.utils import (
++    can_edit_file,
++    edit_file,
++    execute,
++    has_command,
++    retrieve_file,
++    verify_file_extension,
++)
  class UtilTests(TestCase):
++    def setUp(self):
++        self.original_stdout = sys.stdout
++        sys.stdout = open("/dev/null", "w")
++        self.original_stderr = sys.stderr
++        sys.stderr = open("/dev/null", "w")
++        self.original_stdin = sys.stdin
++        self.temp_file = tempfile.NamedTemporaryFile(delete=False)
++
++    def tearDown(self):
++        sys.stdin = self.original_stdin
++        sys.stdout = self.original_stdout
++        sys.stderr = self.original_stderr
++        os.unlink(self.temp_file.name)
++
      @patch("lava_tool.utils.subprocess.check_call")
      def test_has_command_0(self, mocked_check_call):
          # Make sure we raise an exception when the subprocess is called.
@@ -39,3 +68,149 @@
          # Check that a "command" exists. The call to subprocess is mocked.
          mocked_check_call.return_value = 0
          self.assertTrue(has_command(""))
++
++    def test_verify_file_extension_with_extension(self):
++        extension = ".test"
++        supported = [extension[1:]]
++        try:
++            temp_file = tempfile.NamedTemporaryFile(suffix=extension,
++                                                    delete=False)
++            obtained = verify_file_extension(
++                temp_file.name, extension[1:], supported)
++            self.assertEquals(temp_file.name, obtained)
++        finally:
++            if os.path.isfile(temp_file.name):
++                os.unlink(temp_file.name)
++
++    def test_verify_file_extension_without_extension(self):
++        extension = "json"
++        supported = [extension]
++        expected = "/tmp/a_fake.json"
++        obtained = verify_file_extension("/tmp/a_fake", extension, supported)
++        self.assertEquals(expected, obtained)
++
++    def test_verify_file_extension_with_unsupported_extension(self):
++        extension = "json"
++        supported = [extension]
++        expected = "/tmp/a_fake.json"
++        obtained = verify_file_extension(
++            "/tmp/a_fake.extension", extension, supported)
++        self.assertEquals(expected, obtained)
++
++    @patch("os.listdir")
++    def test_retrieve_job_file_0(self, mocked_os_listdir):
++        # Make sure that exception is raised if we go through all the elements
++        # returned by os.listdir().
++        mocked_os_listdir.return_value = ["a_file"]
++        self.assertRaises(CommandError, retrieve_file,
++                          "a_path", ["ext"])
++
++    @patch("os.listdir")
++    def test_retrieve_job_file_1(self, mocked_os_listdir):
++        # Pass some files and directories to retrieve_file(), and make
++        # sure a file with .json suffix is returned.
++        # Pass also a hidden file.
++        try:
++            json_file = tempfile.NamedTemporaryFile(suffix=".json")
++            json_file_name = os.path.basename(json_file.name)
++
++            file_name_no_suffix = tempfile.NamedTemporaryFile(delete=False)
++            file_name_with_suffix = tempfile.NamedTemporaryFile(
++                suffix=".bork", delete=False)
++
++            temp_dir_name = "submit_command_test_tmp_dir"
++            temp_dir_path = os.path.join(tempfile.gettempdir(), temp_dir_name)
++            os.makedirs(temp_dir_path)
++
++            hidden_file = tempfile.NamedTemporaryFile(
++                prefix=".tmp", delete=False)
++
++            mocked_os_listdir.return_value = [
++                temp_dir_name, file_name_no_suffix.name,
++                file_name_with_suffix.name, json_file_name, hidden_file.name]
++
++            obtained = retrieve_file(tempfile.gettempdir(), ["json"])
++            self.assertEqual(json_file.name, obtained)
++        finally:
++            os.removedirs(temp_dir_path)
++            os.unlink(file_name_no_suffix.name)
++            os.unlink(file_name_with_suffix.name)
++            os.unlink(hidden_file.name)
++
++    @patch("lava_tool.utils.subprocess")
++    def test_execute_0(self, mocked_subprocess):
++        mocked_subprocess.check_call = MagicMock()
++        execute("foo")
++        self.assertEqual(mocked_subprocess.check_call.call_args_list,
++                         [call(["foo"])])
++        self.assertTrue(mocked_subprocess.check_call.called)
++
++    @patch("lava_tool.utils.subprocess.check_call")
++    def test_execute_1(self, mocked_check_call):
++        mocked_check_call.side_effect = subprocess.CalledProcessError(1, "foo")
++        self.assertRaises(CommandError, execute, ["foo"])
++
++    @patch("lava_tool.utils.subprocess")
++    @patch("lava_tool.utils.has_command", return_value=False)
++    @patch("lava_tool.utils.os.environ.get", return_value=None)
++    @patch("lava_tool.utils.sys.exit")
++    def test_edit_file_0(self, mocked_sys_exit, mocked_env_get,
++                         mocked_has_command, mocked_subprocess):
++        edit_file(self.temp_file.name)
++        self.assertTrue(mocked_sys_exit.called)
++
++    @patch("lava_tool.utils.subprocess")
++    @patch("lava_tool.utils.has_command", side_effect=[True, False])
++    @patch("lava_tool.utils.os.environ.get", return_value=None)
++    def test_edit_file_1(self, mocked_env_get, mocked_has_command,
++                         mocked_subprocess):
++        mocked_subprocess.Popen = MagicMock()
++        edit_file(self.temp_file.name)
++        expected = [call(["sensible-editor", self.temp_file.name])]
++        self.assertEqual(expected, mocked_subprocess.Popen.call_args_list)
++
++    @patch("lava_tool.utils.subprocess")
++    @patch("lava_tool.utils.has_command", side_effect=[False, True])
++    @patch("lava_tool.utils.os.environ.get", return_value=None)
++    def test_edit_file_2(self, mocked_env_get, mocked_has_command,
++                         mocked_subprocess):
++        mocked_subprocess.Popen = MagicMock()
++        edit_file(self.temp_file.name)
++        expected = [call(["xdg-open", self.temp_file.name])]
++        self.assertEqual(expected, mocked_subprocess.Popen.call_args_list)
++
++    @patch("lava_tool.utils.subprocess")
++    @patch("lava_tool.utils.has_command", return_value=False)
++    @patch("lava_tool.utils.os.environ.get", return_value="vim")
++    def test_edit_file_3(self, mocked_env_get, mocked_has_command,
++                         mocked_subprocess):
++        mocked_subprocess.Popen = MagicMock()
++        edit_file(self.temp_file.name)
++        expected = [call(["vim", self.temp_file.name])]
++        self.assertEqual(expected, mocked_subprocess.Popen.call_args_list)
++
++    @patch("lava_tool.utils.subprocess")
++    @patch("lava_tool.utils.has_command", return_value=False)
++    @patch("lava_tool.utils.os.environ.get", return_value="vim")
++    def test_edit_file_4(self, mocked_env_get, mocked_has_command,
++                         mocked_subprocess):
++        mocked_subprocess.Popen = MagicMock()
++        mocked_subprocess.Popen.side_effect = Exception()
++        self.assertRaises(CommandError, edit_file, self.temp_file.name)
++
++    def test_can_edit_file(self):
++        # Tests the can_edit_file method of the config command.
++        # This is to make sure the device config file is not erased when
++        # checking if it is possible to open it.
++        expected = ("hostname = a_fake_panda02\nconnection_command = \n"
++                    "device_type = panda\n")
++
++        with open(self.temp_file.name, "w") as f:
++            f.write(expected)
++
++        self.assertTrue(can_edit_file(self.temp_file.name))
++        obtained = ""
++        with open(self.temp_file.name) as f:
++            obtained = f.read()
++
++        self.assertEqual(expected, obtained)
 === modified file 'lava_tool/utils.py'
 --- lava_tool/utils.py	2013-07-25 15:43:25 +0000
 +++ lava_tool/utils.py	2013-07-25 15:43:26 +0000
@@ -16,8 +16,17 @@
  # You should have received a copy of the GNU Lesser General Public License
  # along with lava-tool.  If not, see <http://www.gnu.org/licenses/>.
++import StringIO
++import base64
  import os
++import tarfile
++import tempfile
++import types
  import subprocess
++import sys
++import urlparse
++
++from lava.tool.errors import CommandError
  def has_command(command):
@@ -32,3 +41,280 @@
      except subprocess.CalledProcessError:
          command_available = False
      return command_available
++
++
++def to_list(value):
++    """Return a list from the passed value.
++
++    :param value: The parameter to turn into a list.
++    """
++    return_value = []
++    if isinstance(value, types.StringType):
++        return_value = [value]
++    else:
++        return_value = list(value)
++    return return_value
++
++
++def create_tar(paths):
++    """Creates a temporary tar file with the provided paths.
++
++    The tar file is not deleted at the end, it has to be delete by who calls
++    this function.
++
++    If just a directory is passed, it will be flattened out: its contents will
++    be added, but not the directory itself.
++
++    :param paths: List of paths to be included in the tar archive.
++    :type list
++    :return The path to the temporary tar file.
++    """
++    paths = to_list(paths)
++    try:
++        temp_tar_file = tempfile.NamedTemporaryFile(suffix=".tar",
++                                                    delete=False)
++        with tarfile.open(temp_tar_file.name, "w") as tar_file:
++            for path in paths:
++                full_path = os.path.abspath(path)
++                if os.path.isfile(full_path):
++                    arcname = os.path.basename(full_path)
++                    tar_file.add(full_path, arcname=arcname)
++                elif os.path.isdir(full_path):
++                    # If we pass a directory, flatten it out.
++                    # List its contents, and add them as they are.
++                    for element in os.listdir(full_path):
++                        arcname = element
++                        tar_file.add(os.path.join(full_path, element),
++                                     arcname=arcname)
++        return temp_tar_file.name
++    except tarfile.TarError:
++        raise CommandError("Error creating the temporary tar archive.")
++
++
++def base64_encode(path):
++    """Encode in base64 the provided file.
++
++    :param path: The path to a file.
++    :return The file content encoded in base64.
++    """
++    if os.path.isfile(path):
++        encoded_content = StringIO.StringIO()
++
++        try:
++            with open(path) as read_file:
++                base64.encode(read_file, encoded_content)
++
++            return encoded_content.getvalue().strip()
++        except IOError:
++            raise CommandError("Cannot read file "
++                               "'{0}'.".format(path))
++    else:
++        raise CommandError("Provided path does not exists or is not a file: "
++                           "{0}.".format(path))
++
++
++def retrieve_file(path, extensions):
++    """Searches for a file that has one of the supported extensions.
++
++    The path of the first file that matches one of the supported provided
++    extensions will be returned. The files are examined in alphabetical
++    order.
++
++    :param path: Where to look for the file.
++    :param extensions: A list of extensions the file to look for should
++                       have.
++    :return The full path of the file.
++    """
++    if os.path.isfile(path):
++        if check_valid_extension(path, extensions):
++            retrieved_path = path
++        else:
++            raise CommandError("The provided file '{0}' is not "
++                               "valid: extension not supported.".format(path))
++    else:
++        dir_listing = os.listdir(path)
++        dir_listing.sort()
++
++        for element in dir_listing:
++            if element.startswith("."):
++                continue
++
++            element_path = os.path.join(path, element)
++            if os.path.isdir(element_path):
++                continue
++            elif os.path.isfile(element_path):
++                if check_valid_extension(element_path, extensions):
++                    retrieved_path = element_path
++                    break
++        else:
++            raise CommandError("No suitable file found in '{0}'".format(path))
++
++    return retrieved_path
++
++
++def check_valid_extension(path, extensions):
++    """Checks that a file has one of the supported extensions.
++
++    :param path: The file to check.
++    :param extensions: A list of supported extensions.
++    """
++    is_valid = False
++
++    local_path, file_name = os.path.split(path)
++    name, full_extension = os.path.splitext(file_name)
++
++    if full_extension:
++        extension = full_extension[1:].strip().lower()
++        if extension in extensions:
++            is_valid = True
++    return is_valid
++
++
++def verify_file_extension(path, default, supported):
++    """Verifies if a file has a supported extensions.
++
++    If the file does not have one, it will add the default extension
++    provided.
++
++    :param path: The path of a file to verify.
++    :param default: The default extension to use.
++    :param supported: A list of supported extensions to check against.
++    :return The path of the file.
++    """
++    full_path, file_name = os.path.split(path)
++    name, extension = os.path.splitext(file_name)
++    if not extension:
++        path = ".".join([path, default])
++    elif extension[1:].lower() not in supported:
++        path = os.path.join(full_path, ".".join([name, default]))
++    return path
++
++
++def verify_path_existance(path):
++    """Verifies if a given path exists or not on the file system.
++
++    Raises a CommandError in case it exists.
++
++    :param path: The path to verify."""
++    if os.path.exists(path):
++        raise CommandError("{0} already exists.".format(path))
++
++
++def write_file(path, content):
++    """Creates a file with the specified content.
++
++    :param path: The path of the file to write.
++    :param content: What to write in the file.
++    """
++    try:
++        with open(path, "w") as to_write:
++            to_write.write(content)
++    except (OSError, IOError):
++        raise CommandError("Error writing file '{0}'".format(path))
++
++
++def execute(cmd_args):
++    """Executes the supplied command args.
++
++    :param cmd_args: The command, and its optional arguments, to run.
++    :return The command execution return code.
++    """
++    cmd_args = to_list(cmd_args)
++    try:
++        return subprocess.check_call(cmd_args)
++    except subprocess.CalledProcessError:
++        raise CommandError("Error running the following command: "
++                           "{0}".format(" ".join(cmd_args)))
++
++
++def can_edit_file(path):
++    """Checks if a file can be opend in write mode.
++
++    :param path: The path to the file.
++    :return True if it is possible to write on the file, False otherwise.
++    """
++    can_edit = True
++    try:
++        fp = open(path, "a")
++        fp.close()
++    except IOError:
++        can_edit = False
++    return can_edit
++
++
++def edit_file(file_to_edit):
++    """Opens the specified file with the default file editor.
++
++    :param file_to_edit: The file to edit.
++    """
++    editor = os.environ.get("EDITOR", None)
++    if editor is None:
++        if has_command("sensible-editor"):
++            editor = "sensible-editor"
++        elif has_command("xdg-open"):
++            editor = "xdg-open"
++        else:
++            # We really do not know how to open a file.
++            print >> sys.stdout, ("Cannot find an editor to open the "
++                                  "file '{0}'.".format(file_to_edit))
++            print >> sys.stdout, ("Either set the 'EDITOR' environment "
++                                  "variable, or install 'sensible-editor' "
++                                  "or 'xdg-open'.")
++            sys.exit(-1)
++    try:
++        subprocess.Popen([editor, file_to_edit]).wait()
++    except Exception:
++        raise CommandError("Error opening the file '{0}' with the "
++                           "following editor: {1}.".format(file_to_edit,
++                                                           editor))
++
++
++def verify_and_create_url(server, endpoint=""):
++    """Checks that the provided values make a correct URL.
++
++    If the server address does not contain a scheme, by default it will use
++    HTTPS.
++    The endpoint is then added at the URL.
++
++    :param server: A server URL to verify.
++    :return A URL.
++    """
++    scheme, netloc, path, params, query, fragment = \
++        urlparse.urlparse(server)
++    if not scheme:
++        scheme = "https"
++    if not netloc:
++        netloc, path = path, ""
++
++    if not netloc[-1:] == "/":
++        netloc += "/"
++
++    if endpoint:
++        if endpoint[0] == "/":
++            endpoint = endpoint[1:]
++        if not endpoint[-1:] == "/":
++            endpoint += "/"
++        netloc += endpoint
++
++    return urlparse.urlunparse(
++        (scheme, netloc, path, params, query, fragment))
++
++
++def create_dir(path, dir_name=None):
++    """Checks if a directory does not exists, and creates it.
++
++    :param path: The path where the directory should be created.
++    :param dir_name: An optional name for a directory to be created at
++                     path (dir_name will be joined with path).
++    :return The path of the created directory."""
++    created_dir = path
++    if dir_name:
++        created_dir = os.path.join(path, dir_name)
++
++    if not os.path.isdir(created_dir):
++        try:
++            os.makedirs(created_dir)
++        except OSError:
++            raise CommandError("Cannot create directory "
++                               "'{0}'.".format(created_dir))
++    return created_dir
 === modified file 'setup.py'
 --- setup.py	2013-07-25 15:43:25 +0000
 +++ setup.py	2013-07-25 15:43:26 +0000
@@ -46,6 +46,7 @@
          "Topic :: Software Development :: Testing",
      ],
      install_requires=[
++        'PyYAML >= 3.10',
          'argparse >= 1.1',
          'argcomplete >= 0.3',
          'keyring',

LAVA Tool

Merge lp:~milo/lava-tool/lava-165 into lp:~linaro-validation/lava-tool/trunk

Commit message

Description of the change

Preview Diff

Subscribers