Merge lp:~lool/linaro-image-tools/testsuite-when-installed-v2 into lp:linaro-image-tools/11.11

[Loïc Minier]

This series fixes bug #711312, but it also shuffles code around so that modules at the top of the hierarchy don't depend on modules down below (e.g. so that linaro_image_tools.utils doesn't depend on linaro_image_tools.hwpack or .media_create).

I'm aware of some minor issues with this patchset:
- there is one class of change I didn't finish: converting the "%s foo bar" % sudo_args calls to SUDO_ARGS + ["foo", "bar"]; I only did half of the changes -- I'm happy to finish it if reviewers think that's the right way to pursue the change
- it's a bit confusing to have linaro_image_tools.(hwpack.testing, media_create.tests.fixtures, testing, tests.fixtures) modules which all contain helpers for testing; maybe hwpack.tests.utils, media_create.tests.utils, and tests.utils would be a clearer set? I'm a bit worried about having too many utils though
- I didn't add a test for preferred_tools_dir when I promoted it to linaro_image_tools.utils; this is because it uses __file__; I guess I could make that an optional argument, but it would make callers slightly more complex; I think it's ok to keep this untested while it's that short

I did run the testsuite both from the source tree and from an installed version; I did the later with:
sudo ./setup.py install
python -m subunit.run linaro_image_tools.tests.test_suite|grep -v ^successful:|grep -v ^test:

[Loïc Minier (lool)]

Incidentally, the find_command() changes also fix bug #710104. I managed to complete a run of:
linaro-media-create --image_file efikamx.img --image_size 1536M --dev efikamx --rootfs btrfs --hwpack ~/downloads/linaro/11.05/hwpack_linaro-efikamx_20110217-1_armel.tar.gz --binary ~/downloads/linaro/11.05/linaro-n-developer-tar-20110308-0.tar.gz --hwpack-force-yes

with this branch with the locally installed (not packaged -- sudo ./setup.py install) linaro-image-tools.

[Guilherme Salgado (salgado)]

On Thu, 2011-03-24 at 11:31 +0000, Loïc Minier wrote:
> This series fixes bug #711312, but it also shuffles code around so
> that modules at the top of the hierarchy don't depend on modules down
> below (e.g. so that linaro_image_tools.utils doesn't depend on
> linaro_image_tools.hwpack or .media_create).

The code shuffling is most welcome, but given the size of the diff, it's
very tricky to find out what are the exact bits that fix bug 711312.
Care to point them out for me?

>
> I'm aware of some minor issues with this patchset:
> - there is one class of change I didn't finish: converting the "%s foo
> bar" % sudo_args calls to SUDO_ARGS + ["foo", "bar"]; I only did half
> of the changes -- I'm happy to finish it if reviewers think that's the
> right way to pursue the change

Well, for that you reverted my recent changes to use the
mock's .commands_executed instead of .calls. The former makes for more
readable tests and makes it easier to spot differences when tests fail,
so I'm against your changes here. If all you want is to use the
SUDO_ARGS constant from cmd_runner.py you could just define sudo_args at
the top of test_media_create.py as

  sudo_args = " ".join(SUDO_ARGS)

instead of the current definition. That would allow the tests to
continue using .commands_executed instead of .calls

> - it's a bit confusing to have linaro_image_tools.(hwpack.testing,
> media_create.tests.fixtures, testing, tests.fixtures) modules which
> all contain helpers for testing; maybe hwpack.tests.utils,
> media_create.tests.utils, and tests.utils would be a clearer set? I'm
> a bit worried about having too many utils though

I think the best way forward here would be to have a testing directory
under linaro_image_tools, l_i_t.hwpack and l_i_t.media_create, with all
test helpers/fixtures in there. We could then have separate files for
utils and fixtures there if we think it's useful, but by making all
those names available in testing/__init__.py the tests themselves
wouldn't have to worry about the exact location of the code as they'd be
able to import helpers/fixtures directly from <whatever>.testing.

Oh, and the reason why I suggest a 'testing' dir is to separate actual
test code from test helpers/fixtures.

These changes can be saved for later, though, if you'd prefer.

> - I didn't add a test for preferred_tools_dir when I promoted it to
> linaro_image_tools.utils; this is because it uses __file__; I guess I
> could make that an optional argument, but it would make callers
> slightly more complex; I think it's ok to keep this untested while
> it's that short

That's not ideal, but I don't think it's a big deal either.

>
> I did run the testsuite both from the source tree and from an
> installed version; I did the later with:
> sudo ./setup.py install
> python -m subunit.run linaro_image_tools.tests.test_suite|grep -v
> ^successful:|grep -v ^test:

Does that actually use the system installed version or would it prefer
the one in the current directory if you had one? IOW, is '.' prepended
or appended to sys.path?

[Loïc Minier (lool)]

On Thu, Mar 24, 2011, Guilherme Salgado wrote:
> The code shuffling is most welcome, but given the size of the diff, it's
> very tricky to find out what are the exact bits that fix bug 711312.
> Care to point them out for me?

 It's the very first revision of the series which has the lp:xyz tag,
 r302 in this branch; it moves from hwpack's find_script to
 media_create's find_command().

> > I'm aware of some minor issues with this patchset:
> > - there is one class of change I didn't finish: converting the "%s foo
> > bar" % sudo_args calls to SUDO_ARGS + ["foo", "bar"]; I only did half
> > of the changes -- I'm happy to finish it if reviewers think that's the
> > right way to pursue the change
>
> Well, for that you reverted my recent changes to use the
> mock's .commands_executed instead of .calls. The former makes for more
> readable tests and makes it easier to spot differences when tests fail,
> so I'm against your changes here. If all you want is to use the
> SUDO_ARGS constant from cmd_runner.py you could just define sudo_args at
> the top of test_media_create.py as
>
> sudo_args = " ".join(SUDO_ARGS)
>
> instead of the current definition. That would allow the tests to
> continue using .commands_executed instead of .calls

 Ok; this is exactly what I guessed might be the case, and the reason I
 stopped mass-converting commands; I actually added a sudo_args
 definition as you suggest. I'll revert the changs from string to list
 in the tests.

> These changes can be saved for later, though, if you'd prefer.

 I don't mind deferring this to a later submission; I'll open a bug to
 not forget about it.

> > I did run the testsuite both from the source tree and from an
> > installed version; I did the later with:
> > sudo ./setup.py install
> > python -m subunit.run linaro_image_tools.tests.test_suite|grep -v
> > ^successful:|grep -v ^test:
>
> Does that actually use the system installed version or would it prefer
> the one in the current directory if you had one? IOW, is '.' prepended
> or appended to sys.path?

 It would probably pick the local version instead of the system one by
 default, but I had taken care to change to another directory before
 running the system-wide tests.

--
Loïc Minier

[lp:~lool/linaro-image-tools/testsuite-when-installed-v2]

315. By Loïc Minier on 2011-03-24

Revert parts of r309; use MockCmdRunnerPopenFixture.mock.commands_executed
instead of .calls for clarity in the test cases, and simply define sudo_args
from SUDO_ARGS.

[Guilherme Salgado (salgado)]

This looks very good; I only have a few comments.

438 +from linaro_image_tools import cmd_runner
439 +
440 +from linaro_image_tools.cmd_runner import SUDO_ARGS

There's no need to import the latter as it's easily accessible from the cmd_runner you already have defined. That's on linaro_image_tools/media_create/tests/test_media_create.py but it's duplicated in linaro_image_tools/tests/test_cmd_runner.py as well.

And you don't need blank lines between those imports -- we only use blank lines to separate between stdlib/external/local imports.

review: Approve

[lp:~lool/linaro-image-tools/testsuite-when-installed-v2]

316. By Loïc Minier on 2011-03-24

Use cmd_runner.SUDO_ARGS instead of importing it; tidy up imports.

[Loïc Minier (lool)]

On Thu, Mar 24, 2011, Guilherme Salgado wrote:
> There's no need to import the latter as it's easily accessible from
> the cmd_runner you already have defined. That's on
> linaro_image_tools/media_create/tests/test_media_create.py but it's
> duplicated in linaro_image_tools/tests/test_cmd_runner.py as well.

 (There was a third case in linaro_image_tools/tests/test_utils.py as
 well.)

> And you don't need blank lines between those imports -- we only use
> blank lines to separate between stdlib/external/local imports.

 Thanks for the advice; all fixed and pushed, will merge.

--
Loïc Minier

=== modified file 'linaro-media-create'
--- linaro-media-create	2011-03-23 22:55:29 +0000
+++ linaro-media-create	2011-03-24 18:00:58 +0000
@@ -23,6 +23,8 @@
 import sys
 import tempfile
 
+from linaro_image_tools import cmd_runner
+
 from linaro_image_tools.media_create.boards import board_configs
 from linaro_image_tools.media_create.check_device import (
     confirm_device_selection_and_ensure_it_is_ready)
@@ -36,14 +38,8 @@
 from linaro_image_tools.media_create.unpack_binary_tarball import (
     unpack_binary_tarball,
     )
-from linaro_image_tools.media_create.utils import (
-    ensure_command,
-    is_arm_host,
-    )
-from linaro_image_tools.media_create import (
-    cmd_runner,
-    get_args_parser
-    )
+from linaro_image_tools.media_create import get_args_parser
+from linaro_image_tools.utils import ensure_command, is_arm_host
 
 # Just define the global variables
 TMP_DIR = None

=== renamed file 'linaro_image_tools/media_create/cmd_runner.py' => 'linaro_image_tools/cmd_runner.py'
--- linaro_image_tools/media_create/cmd_runner.py	2011-03-09 15:11:28 +0000
+++ linaro_image_tools/cmd_runner.py	2011-03-24 18:00:58 +0000
@@ -21,6 +21,9 @@
 import subprocess
 
 
+SUDO_ARGS = ['sudo', '-E']
+
+
 def run(args, as_root=False, stdin=None, stdout=None, stderr=None):
     """Run the given command as a sub process.
 
@@ -37,8 +40,10 @@
     """
     assert isinstance(args, (list, tuple)), (
         "The command to run must be a list or tuple, found: %s" % type(args))
+    if isinstance(args, tuple):
+        args = list(args)
     if as_root and os.getuid() != 0:
-        args = ['sudo', '-E'] + args
+        args = SUDO_ARGS + args
     return Popen(args, stdin=stdin, stdout=stdout, stderr=stderr)
 
 

=== modified file 'linaro_image_tools/hwpack/testing.py'
--- linaro_image_tools/hwpack/testing.py	2011-03-23 22:25:10 +0000
+++ linaro_image_tools/hwpack/testing.py	2011-03-24 18:00:58 +0000
@@ -32,7 +32,6 @@
 
 from debian.deb822 import Packages
 
-from testtools import TestCase
 from testtools.matchers import Annotate, Equals, Matcher, Mismatch
 
 from linaro_image_tools.hwpack.better_tarfile import writeable_tarfile
@@ -173,39 +172,6 @@
         return "file:" + os.path.abspath(self.rootdir) +" ./"
 
 
-class TestCaseWithFixtures(TestCase):
-    """A TestCase with the ability to easily add 'fixtures'.
-
-    A fixture is an object which can be created and cleaned up, and
-    this test case knows how to manage them to ensure that they will
-    always be cleaned up at the end of the test.
-    """
-
-    def useFixture(self, fixture):
-        """Make use of a fixture, ensuring that it will be cleaned up.
-
-        Given a fixture, this method will run the `setUp` method of
-        the fixture, and ensure that its `tearDown` method will be
-        called at the end of the test, regardless of success or failure.
-
-        :param fixture: the fixture to use.
-        :type fixture: an object with setUp and tearDown methods.
-        :return: the fixture that was passed in.
-        """
-        self.addCleanup(fixture.tearDown)
-        fixture.setUp()
-        return fixture
-
-    def createTempFileAsFixture(self, prefix='tmp', dir=None):
-        """Create a temp file and make sure it is removed on tearDown.
-
-        :return: The filename of the file created.
-        """
-        _, filename = tempfile.mkstemp(prefix=prefix, dir=dir)
-        self.addCleanup(os.unlink, filename)
-        return filename
-
-
 class ConfigFileFixture(object):
 
     def __init__(self, contents):

=== modified file 'linaro_image_tools/hwpack/tests/test_builder.py'
--- linaro_image_tools/hwpack/tests/test_builder.py	2011-03-23 22:25:10 +0000
+++ linaro_image_tools/hwpack/tests/test_builder.py	2011-03-24 18:00:58 +0000
@@ -48,8 +48,8 @@
     IsHardwarePack,
     MatchesStructure,
     Not,
-    TestCaseWithFixtures,
     )
+from linaro_image_tools.testing import TestCaseWithFixtures
 
 
 class ConfigFileMissingTests(TestCase):

=== modified file 'linaro_image_tools/hwpack/tests/test_packages.py'
--- linaro_image_tools/hwpack/tests/test_packages.py	2011-03-23 22:25:10 +0000
+++ linaro_image_tools/hwpack/tests/test_packages.py	2011-03-24 18:00:58 +0000
@@ -50,8 +50,8 @@
     ContextManagerFixture,
     DummyFetchedPackage,
     MatchesPackage,
-    TestCaseWithFixtures,
     )
+from linaro_image_tools.testing import TestCaseWithFixtures
 
 
 class GetPackagesFileTests(TestCase):

=== modified file 'linaro_image_tools/hwpack/tests/test_script.py'
--- linaro_image_tools/hwpack/tests/test_script.py	2011-03-23 22:25:10 +0000
+++ linaro_image_tools/hwpack/tests/test_script.py	2011-03-24 18:00:58 +0000
@@ -19,7 +19,6 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
 # USA.
 
-import os
 import subprocess
 
 from linaro_image_tools.hwpack.hardwarepack import Metadata
@@ -29,8 +28,9 @@
     ConfigFileFixture,
     DummyFetchedPackage,
     IsHardwarePack,
-    TestCaseWithFixtures,
     )
+from linaro_image_tools.testing import TestCaseWithFixtures
+from linaro_image_tools.utils import find_command
 
 
 class ScriptTests(TestCaseWithFixtures):
@@ -38,26 +38,9 @@
 
     def setUp(self):
         super(ScriptTests, self).setUp()
-        self.script_path = self.find_script()
+        self.script_path = find_command("linaro-hwpack-create")
         self.useFixture(ChdirToTempdirFixture())
 
-    def find_script(self):
-        script_name = "linaro-hwpack-create"
-        this_path = os.path.abspath(__file__)
-        parent_path = this_path
-        for i in range(4):
-            parent_path = os.path.dirname(parent_path)
-        possible_paths = [
-            os.path.join(parent_path, script_name),
-            os.path.join("usr", "local", "bin", script_name),
-            os.path.join("usr", "bin", script_name),
-        ]
-        for script_path in possible_paths:
-            if os.path.exists(script_path):
-                return script_path
-        raise AssertionError(
-            "Could not find linaro-hwpack-create script to test.")
-
     def run_script(self, args, expected_returncode=0):
         cmdline = [self.script_path] + args
         proc = subprocess.Popen(

=== modified file 'linaro_image_tools/media_create/boards.py'
--- linaro_image_tools/media_create/boards.py	2011-03-23 23:14:51 +0000
+++ linaro_image_tools/media_create/boards.py	2011-03-24 18:00:58 +0000
@@ -32,7 +32,8 @@
 import struct
 from binascii import crc32
 
-from linaro_image_tools.media_create import cmd_runner
+from linaro_image_tools import cmd_runner
+
 from linaro_image_tools.media_create.partitions import SECTOR_SIZE
 
 # Notes:

=== modified file 'linaro_image_tools/media_create/hwpack.py'
--- linaro_image_tools/media_create/hwpack.py	2011-03-23 22:25:10 +0000
+++ linaro_image_tools/media_create/hwpack.py	2011-03-24 18:00:58 +0000
@@ -20,8 +20,8 @@
 import os
 import sys
 
-from linaro_image_tools.media_create import cmd_runner
-from linaro_image_tools.media_create.utils import (
+from linaro_image_tools import cmd_runner
+from linaro_image_tools.utils import (
     is_arm_host,
     find_command,
     )

=== modified file 'linaro_image_tools/media_create/partitions.py'
--- linaro_image_tools/media_create/partitions.py	2011-03-23 22:25:10 +0000
+++ linaro_image_tools/media_create/partitions.py	2011-03-24 18:00:58 +0000
@@ -30,7 +30,7 @@
     PARTITION_NORMAL,
     )
 
-from linaro_image_tools.media_create import cmd_runner
+from linaro_image_tools import cmd_runner
 
 
 HEADS = 128

=== added file 'linaro_image_tools/media_create/populate_boot.py.OTHER'
--- linaro_image_tools/media_create/populate_boot.py.OTHER	1970-01-01 00:00:00 +0000
+++ linaro_image_tools/media_create/populate_boot.py.OTHER	2011-03-24 18:00:58 +0000
@@ -0,0 +1,58 @@
+# Copyright (C) 2010, 2011 Linaro
+#
+# Author: Guilherme Salgado <[e-mail]>
+#
+# This file is part of Linaro Image Tools.
+# 
+# Linaro Image Tools is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Linaro Image Tools 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 General Public License
+# along with Linaro Image Tools.  If not, see <http://www.gnu.org/licenses/>.
+
+import os
+
+from linaro_image_tools import cmd_runner
+
+
+def populate_boot(board_config, chroot_dir, rootfs_uuid, boot_partition,
+                  boot_disk, boot_device_or_file, is_live, is_lowmem,
+                  consoles):
+
+    parts_dir = 'boot'
+    if is_live:
+        parts_dir = 'casper'
+    uboot_parts_dir = os.path.join(chroot_dir, parts_dir)
+
+    cmd_runner.run(['mkdir', '-p', boot_disk]).wait()
+    cmd_runner.run(['mount', boot_partition, boot_disk], as_root=True).wait()
+
+    if board_config.uboot_in_boot_part:
+        uboot_flavor = board_config.uboot_flavor
+        assert uboot_flavor is not None, (
+            "uboot_in_boot_part is set but not uboot_flavor")
+        uboot_bin = os.path.join(
+            chroot_dir, 'usr', 'lib', 'u-boot', uboot_flavor, 'u-boot.bin')
+        cmd_runner.run(
+            ['cp', '-v', uboot_bin, boot_disk], as_root=True).wait()
+
+    boot_script = "%(boot_disk)s/%(boot_script_name)s" % (
+        dict(boot_disk=boot_disk,
+             boot_script_name=board_config.boot_script))
+
+    board_config.make_boot_files(
+        uboot_parts_dir, is_live, is_lowmem, consoles, chroot_dir, rootfs_uuid,
+        boot_disk, boot_script, boot_device_or_file)
+
+    cmd_runner.run(['sync']).wait()
+    try:
+        cmd_runner.run(['umount', boot_disk], as_root=True).wait()
+    except cmd_runner.SubcommandNonZeroReturnValue:
+        pass

=== modified file 'linaro_image_tools/media_create/rootfs.py'
--- linaro_image_tools/media_create/rootfs.py	2011-03-23 22:25:10 +0000
+++ linaro_image_tools/media_create/rootfs.py	2011-03-24 18:00:58 +0000
@@ -21,7 +21,7 @@
 import os
 import tempfile
 
-from linaro_image_tools.media_create import cmd_runner
+from linaro_image_tools import cmd_runner
 
 
 def populate_rootfs(content_dir, root_disk, partition, rootfs_type,

=== modified file 'linaro_image_tools/media_create/tests/fixtures.py'
--- linaro_image_tools/media_create/tests/fixtures.py	2011-03-23 22:25:10 +0000
+++ linaro_image_tools/media_create/tests/fixtures.py	2011-03-24 18:00:58 +0000
@@ -18,28 +18,11 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
-import shutil
 import subprocess
-import tempfile
 
 from linaro_image_tools.media_create import partitions
-from linaro_image_tools.media_create import cmd_runner
-
-
-class CreateTempDirFixture(object):
-
-    def __init__(self):
-        self.tempdir = None
-
-    def setUp(self):
-        self.tempdir = tempfile.mkdtemp()
-
-    def tearDown(self):
-        if os.path.exists(self.tempdir):
-            shutil.rmtree(self.tempdir)
-
-    def get_temp_dir(self):
-        return self.tempdir
+
+from linaro_image_tools.tests.fixtures import MockSomethingFixture
 
 
 class CreateTarballFixture(object):
@@ -66,79 +49,6 @@
     def get_tarball(self):
         return self.tarball
 
-
-class MockSomethingFixture(object):
-    """A fixture which mocks something on the given object.
-
-    Replaces attr_name on obj with the given mock, undoing that upon
-    tearDown().
-    """
-
-    def __init__(self, obj, attr_name, mock):
-        self.obj = obj
-        self.attr_name = attr_name
-        self.mock = mock
-        self.orig_attr = getattr(obj, attr_name)
-
-    def setUp(self):
-        setattr(self.obj, self.attr_name, self.mock)
-
-    def tearDown(self):
-        setattr(self.obj, self.attr_name, self.orig_attr)
-
-
-class MockCmdRunnerPopen(object):
-    """A mock for cmd_runner.Popen() which stores the args given to it."""
-    calls = None
-    # A variable that is set to False in __call__() and only set back to True
-    # when wait() is called, to indicate that tht subprocess has finished. Is
-    # used in tests to make sure all callsites wait for their child.
-    child_finished = True
-
-    def __call__(self, cmd, *args, **kwargs):
-        self.child_finished = False
-        if self.calls is None:
-            self.calls = []
-        if isinstance(cmd, basestring):
-            all_args = [cmd]
-        else:
-            all_args = cmd
-        all_args.extend(args)
-        self.calls.append(all_args)
-        self.returncode = 0
-        return self
-
-    def communicate(self, input=None):
-        self.wait()
-        return '', ''
-
-    def wait(self):
-        self.child_finished = True
-        return self.returncode
-
-    @property
-    def commands_executed(self):
-        return [' '.join(args) for args in self.calls]
-
-
-class MockCmdRunnerPopenFixture(MockSomethingFixture):
-    """A test fixture which mocks cmd_runner.do_run with the given mock.
-
-    If no mock is given, a MockCmdRunnerPopen instance is used.
-    """
-
-    def __init__(self):
-        super(MockCmdRunnerPopenFixture, self).__init__(
-            cmd_runner, 'Popen', MockCmdRunnerPopen())
-
-    def tearDown(self):
-        super(MockCmdRunnerPopenFixture, self).tearDown()
-        if not self.mock.child_finished:
-            raise AssertionError(
-                "You should call wait() or communicate() to ensure "
-                "the subprocess is finished before proceeding.")
-
-
 class MockCallableWithPositionalArgs(object):
     """A callable mock which just stores the positional args given to it.
 

=== modified file 'linaro_image_tools/media_create/tests/test_media_create.py'
--- linaro_image_tools/media_create/tests/test_media_create.py	2011-03-24 14:49:25 +0000
+++ linaro_image_tools/media_create/tests/test_media_create.py	2011-03-24 18:00:58 +0000
@@ -21,7 +21,6 @@
 import glob
 import os
 import random
-import stat
 import string
 import subprocess
 import sys
@@ -31,17 +30,14 @@
 
 from testtools import TestCase
 
-from linaro_image_tools.hwpack.testing import TestCaseWithFixtures
-
+from linaro_image_tools import cmd_runner
+import linaro_image_tools.media_create
 from linaro_image_tools.media_create import (
     check_device,
-    cmd_runner,
     boards,
     partitions,
     rootfs,
-    utils,
     )
-import linaro_image_tools.media_create
 from linaro_image_tools.media_create.boards import (
     LOADER_MIN_SIZE_S,
     SECTOR_SIZE,
@@ -88,23 +84,20 @@
     populate_rootfs,
     write_data_to_protected_file,
     )
+from linaro_image_tools.media_create.tests.fixtures import (
+    CreateTarballFixture,
+    MockRunSfdiskCommandsFixture,
+    )
 from linaro_image_tools.media_create.unpack_binary_tarball import (
     unpack_binary_tarball,
     )
-from linaro_image_tools.media_create.utils import (
-    ensure_command,
-    find_command,
-    install_package_providing,
-    UnableToFindPackageProvidingCommand,
-    )
-
-from linaro_image_tools.media_create.tests.fixtures import (
+from linaro_image_tools.testing import TestCaseWithFixtures
+from linaro_image_tools.tests.fixtures import (
     CreateTempDirFixture,
-    CreateTarballFixture,
     MockCmdRunnerPopenFixture,
     MockSomethingFixture,
-    MockRunSfdiskCommandsFixture,
     )
+<<<<<<< TREE
 
 
 sudo_args = 'sudo -E'
@@ -185,6 +178,12 @@
         self.assertRaises(
             UnableToFindPackageProvidingCommand,
             install_package_providing, 'mkfs.lean')
+=======
+from linaro_image_tools.utils import find_command, preferred_tools_dir
+
+
+sudo_args = " ".join(cmd_runner.SUDO_ARGS)
+>>>>>>> MERGE-SOURCE
 
 
 class TestGetMLOFile(TestCaseWithFixtures):
@@ -582,52 +581,6 @@
         self.assertEquals("67d641db-ea7d-4acf-9f46-5f1f8275dce2", uuid)
 
 
-class TestCmdRunner(TestCaseWithFixtures):
-
-    def test_run(self):
-        fixture = self.useFixture(MockCmdRunnerPopenFixture())
-        proc = cmd_runner.run(['foo', 'bar', 'baz'])
-        # Call wait or else MockCmdRunnerPopenFixture() raises an
-        # AssertionError().
-        proc.wait()
-        self.assertEqual(0, proc.returncode)
-        self.assertEqual([['foo', 'bar', 'baz']], fixture.mock.calls)
-
-    def test_run_as_root_with_sudo(self):
-        fixture = self.useFixture(MockCmdRunnerPopenFixture())
-        self.useFixture(MockSomethingFixture(os, 'getuid', lambda: 1000))
-        cmd_runner.run(['foo', 'bar'], as_root=True).wait()
-        self.assertEqual(
-            ['%s foo bar' % sudo_args], fixture.mock.commands_executed)
-
-    def test_run_as_root_as_root(self):
-        fixture = self.useFixture(MockCmdRunnerPopenFixture())
-        self.useFixture(MockSomethingFixture(os, 'getuid', lambda: 0))
-        cmd_runner.run(['foo', 'bar'], as_root=True).wait()
-        self.assertEqual([['foo', 'bar']], fixture.mock.calls)
-
-    def test_run_succeeds_on_zero_return_code(self):
-        proc = cmd_runner.run(['true'])
-        # Need to wait() here as we're using the real Popen.
-        proc.wait()
-        self.assertEqual(0, proc.returncode)
-
-    def test_run_raises_exception_on_non_zero_return_code(self):
-        def run_and_wait():
-            proc = cmd_runner.run(['false'])
-            proc.wait()
-        self.assertRaises(
-            cmd_runner.SubcommandNonZeroReturnValue, run_and_wait)
-
-    def test_run_must_be_given_list_as_args(self):
-        self.assertRaises(AssertionError, cmd_runner.run, 'true')
-
-    def test_Popen(self):
-        proc = cmd_runner.Popen('true')
-        returncode = proc.wait()
-        self.assertEqual(0, returncode)
-
-
 class TestBoards(TestCaseWithFixtures):
 
     def _mock_get_file_matching(self):

=== modified file 'linaro_image_tools/media_create/unpack_binary_tarball.py'
--- linaro_image_tools/media_create/unpack_binary_tarball.py	2011-03-23 22:25:10 +0000
+++ linaro_image_tools/media_create/unpack_binary_tarball.py	2011-03-24 18:00:58 +0000
@@ -17,7 +17,7 @@
 # You should have received a copy of the GNU General Public License
 # along with Linaro Image Tools.  If not, see <http://www.gnu.org/licenses/>.
 
-from linaro_image_tools.media_create import cmd_runner
+from linaro_image_tools import cmd_runner
 
 
 def unpack_binary_tarball(tarball, unpack_dir, as_root=True):

=== added file 'linaro_image_tools/testing.py'
--- linaro_image_tools/testing.py	1970-01-01 00:00:00 +0000
+++ linaro_image_tools/testing.py	2011-03-24 18:00:58 +0000
@@ -0,0 +1,59 @@
+# Copyright (C) 2010, 2011 Linaro
+#
+# Author: James Westby <[e-mail]>
+#
+# This file is part of Linaro Image Tools.
+#
+# Linaro Image Tools is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+# 
+# Linaro Image Tools 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 General Public License
+# along with Linaro Image Tools; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
+# USA.
+
+import os
+import tempfile
+
+from testtools import TestCase
+
+
+class TestCaseWithFixtures(TestCase):
+    """A TestCase with the ability to easily add 'fixtures'.
+
+    A fixture is an object which can be created and cleaned up, and
+    this test case knows how to manage them to ensure that they will
+    always be cleaned up at the end of the test.
+    """
+
+    def useFixture(self, fixture):
+        """Make use of a fixture, ensuring that it will be cleaned up.
+
+        Given a fixture, this method will run the `setUp` method of
+        the fixture, and ensure that its `tearDown` method will be
+        called at the end of the test, regardless of success or failure.
+
+        :param fixture: the fixture to use.
+        :type fixture: an object with setUp and tearDown methods.
+        :return: the fixture that was passed in.
+        """
+        self.addCleanup(fixture.tearDown)
+        fixture.setUp()
+        return fixture
+
+    def createTempFileAsFixture(self, prefix='tmp', dir=None):
+        """Create a temp file and make sure it is removed on tearDown.
+
+        :return: The filename of the file created.
+        """
+        _, filename = tempfile.mkstemp(prefix=prefix, dir=dir)
+        self.addCleanup(os.unlink, filename)
+        return filename
+

=== modified file 'linaro_image_tools/tests/__init__.py'
--- linaro_image_tools/tests/__init__.py	2011-03-23 22:25:10 +0000
+++ linaro_image_tools/tests/__init__.py	2011-03-24 18:00:58 +0000
@@ -1,10 +1,18 @@
+import unittest
+
 from linaro_image_tools.hwpack.tests import test_suite as hwpack_suite
 from linaro_image_tools.media_create.tests import (
     test_suite as media_create_suite,
     )
 
-
 def test_suite():
-    suite = hwpack_suite()
+    module_names = [
+        'linaro_image_tools.tests.test_cmd_runner',
+        'linaro_image_tools.tests.test_utils',
+        ]
+    loader = unittest.TestLoader()
+    suite = loader.loadTestsFromNames(module_names)
+    suite.addTests(hwpack_suite())
     suite.addTests(media_create_suite())
     return suite
+

=== added file 'linaro_image_tools/tests/fixtures.py'
--- linaro_image_tools/tests/fixtures.py	1970-01-01 00:00:00 +0000
+++ linaro_image_tools/tests/fixtures.py	2011-03-24 18:00:58 +0000
@@ -0,0 +1,113 @@
+# Copyright (C) 2010, 2011 Linaro
+#
+# Author: Guilherme Salgado <[e-mail]>
+#
+# This file is part of Linaro Image Tools.
+#
+# Linaro Image Tools is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Linaro Image Tools 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 General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import os
+import shutil
+import tempfile
+
+from linaro_image_tools import cmd_runner
+
+
+class CreateTempDirFixture(object):
+
+    def __init__(self):
+        self.tempdir = None
+
+    def setUp(self):
+        self.tempdir = tempfile.mkdtemp()
+
+    def tearDown(self):
+        if os.path.exists(self.tempdir):
+            shutil.rmtree(self.tempdir)
+
+    def get_temp_dir(self):
+        return self.tempdir
+
+
+class MockSomethingFixture(object):
+    """A fixture which mocks something on the given object.
+
+    Replaces attr_name on obj with the given mock, undoing that upon
+    tearDown().
+    """
+
+    def __init__(self, obj, attr_name, mock):
+        self.obj = obj
+        self.attr_name = attr_name
+        self.mock = mock
+        self.orig_attr = getattr(obj, attr_name)
+
+    def setUp(self):
+        setattr(self.obj, self.attr_name, self.mock)
+
+    def tearDown(self):
+        setattr(self.obj, self.attr_name, self.orig_attr)
+
+
+class MockCmdRunnerPopen(object):
+    """A mock for cmd_runner.Popen() which stores the args given to it."""
+    calls = None
+    # A variable that is set to False in __call__() and only set back to True
+    # when wait() is called, to indicate that the subprocess has finished. Is
+    # used in tests to make sure all callsites wait for their child.
+    child_finished = True
+
+    def __call__(self, cmd, *args, **kwargs):
+        self.child_finished = False
+        if self.calls is None:
+            self.calls = []
+        if isinstance(cmd, basestring):
+            all_args = [cmd]
+        else:
+            all_args = cmd
+        all_args.extend(args)
+        self.calls.append(all_args)
+        self.returncode = 0
+        return self
+
+    def communicate(self, input=None):
+        self.wait()
+        return '', ''
+
+    def wait(self):
+        self.child_finished = True
+        return self.returncode
+
+    @property
+    def commands_executed(self):
+        return [' '.join(args) for args in self.calls]
+
+
+class MockCmdRunnerPopenFixture(MockSomethingFixture):
+    """A test fixture which mocks cmd_runner.do_run with the given mock.
+
+    If no mock is given, a MockCmdRunnerPopen instance is used.
+    """
+
+    def __init__(self):
+        super(MockCmdRunnerPopenFixture, self).__init__(
+            cmd_runner, 'Popen', MockCmdRunnerPopen())
+
+    def tearDown(self):
+        super(MockCmdRunnerPopenFixture, self).tearDown()
+        if not self.mock.child_finished:
+            raise AssertionError(
+                "You should call wait() or communicate() to ensure "
+                "the subprocess is finished before proceeding.")
+

=== added file 'linaro_image_tools/tests/test_cmd_runner.py'
--- linaro_image_tools/tests/test_cmd_runner.py	1970-01-01 00:00:00 +0000
+++ linaro_image_tools/tests/test_cmd_runner.py	2011-03-24 18:00:58 +0000
@@ -0,0 +1,84 @@
+# Copyright (C) 2010, 2011 Linaro
+#
+# Author: Guilherme Salgado <[e-mail]>
+#
+# This file is part of Linaro Image Tools.
+#
+# Linaro Image Tools is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Linaro Image Tools 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 General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import os
+
+from linaro_image_tools import cmd_runner
+from linaro_image_tools.testing import TestCaseWithFixtures
+from linaro_image_tools.tests.fixtures import (
+    MockCmdRunnerPopenFixture,
+    MockSomethingFixture,
+    )
+
+
+sudo_args = " ".join(cmd_runner.SUDO_ARGS)
+
+
+class TestCmdRunner(TestCaseWithFixtures):
+
+    def test_run(self):
+        fixture = self.useFixture(MockCmdRunnerPopenFixture())
+        proc = cmd_runner.run(['foo', 'bar', 'baz'])
+        # Call wait or else MockCmdRunnerPopenFixture() raises an
+        # AssertionError().
+        proc.wait()
+        self.assertEqual(0, proc.returncode)
+        self.assertEqual(['foo bar baz'], fixture.mock.commands_executed)
+
+    def test_run_as_root_with_sudo(self):
+        fixture = self.useFixture(MockCmdRunnerPopenFixture())
+        self.useFixture(MockSomethingFixture(os, 'getuid', lambda: 1000))
+        cmd_runner.run(['foo', 'bar'], as_root=True).wait()
+        self.assertEqual(
+            ['%s foo bar' % sudo_args], fixture.mock.commands_executed)
+
+    def test_run_as_root_as_root(self):
+        fixture = self.useFixture(MockCmdRunnerPopenFixture())
+        self.useFixture(MockSomethingFixture(os, 'getuid', lambda: 0))
+        cmd_runner.run(['foo', 'bar'], as_root=True).wait()
+        self.assertEqual(['foo bar'], fixture.mock.commands_executed)
+
+    def test_tuple_with_sudo(self):
+        fixture = self.useFixture(MockCmdRunnerPopenFixture())
+        self.useFixture(MockSomethingFixture(os, 'getuid', lambda: 1000))
+        cmd_runner.run(('foo', 'bar',), as_root=True).wait()
+        self.assertEqual(
+            ['%s foo bar' % sudo_args], fixture.mock.commands_executed)
+
+    def test_run_succeeds_on_zero_return_code(self):
+        proc = cmd_runner.run(['true'])
+        # Need to wait() here as we're using the real Popen.
+        proc.wait()
+        self.assertEqual(0, proc.returncode)
+
+    def test_run_raises_exception_on_non_zero_return_code(self):
+        def run_and_wait():
+            proc = cmd_runner.run(['false'])
+            proc.wait()
+        self.assertRaises(
+            cmd_runner.SubcommandNonZeroReturnValue, run_and_wait)
+
+    def test_run_must_be_given_list_as_args(self):
+        self.assertRaises(AssertionError, cmd_runner.run, 'true')
+
+    def test_Popen(self):
+        proc = cmd_runner.Popen('true')
+        returncode = proc.wait()
+        self.assertEqual(0, returncode)
+

=== added file 'linaro_image_tools/tests/test_utils.py'
--- linaro_image_tools/tests/test_utils.py	1970-01-01 00:00:00 +0000
+++ linaro_image_tools/tests/test_utils.py	2011-03-24 18:00:58 +0000
@@ -0,0 +1,111 @@
+# Copyright (C) 2010, 2011 Linaro
+#
+# Author: Guilherme Salgado <[e-mail]>
+#
+# This file is part of Linaro Image Tools.
+#
+# Linaro Image Tools is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Linaro Image Tools 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 General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import os
+import stat
+import subprocess
+import sys
+
+from linaro_image_tools import cmd_runner, utils
+from linaro_image_tools.testing import TestCaseWithFixtures
+from linaro_image_tools.tests.fixtures import (
+    CreateTempDirFixture,
+    MockCmdRunnerPopenFixture,
+    MockSomethingFixture,
+    )
+from linaro_image_tools.utils import (
+    ensure_command,
+    find_command,
+    install_package_providing,
+    preferred_tools_dir,
+    UnableToFindPackageProvidingCommand,
+    )
+
+
+sudo_args = " ".join(cmd_runner.SUDO_ARGS)
+
+
+class TestEnsureCommand(TestCaseWithFixtures):
+
+    install_pkg_providing_called = False
+
+    def setUp(self):
+        super(TestEnsureCommand, self).setUp()
+        self.useFixture(MockSomethingFixture(
+            sys, 'stdout', open('/dev/null', 'w')))
+
+    def test_command_already_present(self):
+        self.mock_install_package_providing()
+        ensure_command('apt-get')
+        self.assertFalse(self.install_pkg_providing_called)
+
+    def test_command_not_present(self):
+        self.mock_install_package_providing()
+        ensure_command('apt-get-two-o')
+        self.assertTrue(self.install_pkg_providing_called)
+
+    def mock_install_package_providing(self):
+        def mock_func(command):
+            self.install_pkg_providing_called = True
+        self.useFixture(MockSomethingFixture(
+            utils, 'install_package_providing', mock_func))
+
+
+class TestFindCommand(TestCaseWithFixtures):
+
+    def test_preferred_dir(self):
+        tempdir = self.useFixture(CreateTempDirFixture()).get_temp_dir()
+        lmc = 'linaro-media-create'
+        path = os.path.join(tempdir, lmc)
+        open(path, 'w').close()
+        os.chmod(path, stat.S_IXUSR)
+        self.assertEquals(path, find_command(lmc, tempdir))
+
+    def test_existing_command(self):
+        lmc = 'linaro-media-create'
+        prefer_dir = preferred_tools_dir()
+        if prefer_dir is None:
+            expected, _ = cmd_runner.run(
+                ['which', lmc, ],
+                stdout=subprocess.PIPE).communicate()
+            expected = expected.strip()
+        else:
+            expected = os.path.join(prefer_dir, lmc)
+        self.assertEquals(expected, find_command(lmc))
+
+    def test_nonexisting_command(self):
+        self.assertEquals(find_command('linaro-moo'), None)
+
+
+class TestInstallPackageProviding(TestCaseWithFixtures):
+
+    def test_found_package(self):
+        self.useFixture(MockSomethingFixture(
+            sys, 'stdout', open('/dev/null', 'w')))
+        fixture = self.useFixture(MockCmdRunnerPopenFixture())
+        install_package_providing('mkfs.vfat')
+        self.assertEqual(
+            ['%s apt-get install dosfstools' % sudo_args],
+            fixture.mock.commands_executed)
+
+    def test_not_found_package(self):
+        self.assertRaises(
+            UnableToFindPackageProvidingCommand,
+            install_package_providing, 'mkfs.lean')
+

=== renamed file 'linaro_image_tools/media_create/utils.py' => 'linaro_image_tools/utils.py'
--- linaro_image_tools/media_create/utils.py	2011-03-24 11:49:40 +0000
+++ linaro_image_tools/utils.py	2011-03-24 18:00:58 +0000
@@ -25,7 +25,7 @@
 except ImportError:
     CommandNotFound = None
 
-from linaro_image_tools.media_create import cmd_runner
+from linaro_image_tools import cmd_runner
 
 
 def install_package_providing(command):
@@ -99,5 +99,13 @@
     return platform.machine().startswith('arm')
 
 
+def preferred_tools_dir():
+    prefer_dir = None
+    # running from bzr checkout?
+    if not os.path.isabs(__file__):
+        prefer_dir = os.getcwd()
+    return prefer_dir
+
+
 class UnableToFindPackageProvidingCommand(Exception):
     """We can't find a package which provides the given command."""