Merge lp:~pwlars/lava-test/list into lp:lava-test/0.0

[Paul Larson]

These two commands are pretty straightforward - one to list the tests that have been defined, another to list the tests that have been installed. Not sure how to go about testing them since it prints it directly, unless I separate out a function to generate the list. Considering how simple they are right now, and the limited usefulness of such a function in other places, I think that might be overkill. But if you disagree, or have a better idea for how to unit test this, I'm all ears.

Thanks,
Paul Larson

[lp:~pwlars/lava-test/list]

16. By Paul Larson on 2010-07-08

Add a command to list saved results

[Paul Larson (pwlars)]

Updated to also add a command to list saved results, since that's also small and related.

[James Westby (james-w)]

On Thu, 08 Jul 2010 21:50:58 -0000, Paul Larson <email address hidden> wrote:
> Paul Larson has proposed merging lp:~pwlars/abrek/list into lp:abrek.
>
> Requested reviews:
> Linaro Infrastructure (arm-infrastructure)
>
>
> These two commands are pretty straightforward - one to list the tests that have been defined, another to list the tests that have been installed. Not sure how to go about testing them since it prints it directly, unless I separate out a function to generate the list. Considering how simple they are right now, and the limited usefulness of such a function in other places, I think that might be overkill. But if you disagree, or have a better idea for how to unit test this, I'm all ears.

So, while I agree there isn't a lot of value to testing these things,
the danger is that you make it harder for your to add tests later when
you want to, as there is more and more to do everytime you add something
to them.

Separating out functions to list the tests etc. could be useful, and
would mean that you could have simpler tests for what is actually
printed.

As for testing the commands themselves there are a couple of things you
can do. One is to pass in to the command a file object that they write
to, which is normally arranged to be sys.stdout, but can be a StringIO
or something in a test. The other is to have test methods that exec the
commands in a subprocess and capture the output. This is slower, but
gives you end-to-end coverage.

Some union of all three approaches would probably suit you as things
grow.

  * Funtions to do the work, separate from the UI classes. This allows
    you to test logic with targeted unit tests.

  * Tests of the command logic using test doubles for things like
    sys.stdout.

  * Tests that actually exec the commands and check that they did
    something. As these are the slowest and the hardest to debug you
    don't want to use them too much, but by this point there is little
    let to test except the fact that the command is available and
    actually does something, so they can be little more than smoke
    tests.

Your actual code looks fine to me, though things such as "walk_packages"
aren't immediately obvious to me, so I would have reached for tests to
ensure that it did what I thought.

Thanks,

James

[Paul Larson (pwlars)]

> * Tests of the command logic using test doubles for things like
> sys.stdout.
I was able to something for this pretty easily for testing the command to list known tests. However writing a test for listing the installed tests and the results is something I'm having trouble seeing past. Obviously it's unlikely that tests would be installed or results would exist at the time of running the unittest, and I really don't want to rely on the actual default directories. I'd much rather have the config object return a fake path, that I can pre-fill with test data. I don't know of a way to do that though. Is it just because of how I handled the config object?

[James Westby (james-w)]

> > * Tests of the command logic using test doubles for things like
> > sys.stdout.
> I was able to something for this pretty easily for testing the command to list
> known tests. However writing a test for listing the installed tests and the
> results is something I'm having trouble seeing past. Obviously it's unlikely
> that tests would be installed or results would exist at the time of running
> the unittest, and I really don't want to rely on the actual default
> directories. I'd much rather have the config object return a fake path, that
> I can pre-fill with test data. I don't know of a way to do that though. Is
> it just because of how I handled the config object?

Having a global config makes this tricky.

If you are not forking a subprocess to run these commands then you can do
something like replace AbrekConfig with an object with known values.

If you are forking then you can set the env to point $HOME to a tempdir
that you create, containing config files with the desired contents.

Thanks,

James

[lp:~pwlars/lava-test/list]

17. By Paul Larson on 2010-07-09

Handle errors gracefully when the directories for results and installed
tests don't even exist

18. By Paul Larson on 2010-07-09

improved config handling, thanks jamesw

19. By Paul Larson on 2010-07-09

Added tests for listing installed/known tests, and results

[Paul Larson (pwlars)]

Thanks for the help! I've updated the branch with tests for the 3 commands that I added

review: Needs Resubmitting

=== modified file 'abrek/builtins.py'
--- abrek/builtins.py	2010-07-06 11:36:06 +0000
+++ abrek/builtins.py	2010-07-09 21:51:42 +0000
@@ -1,3 +1,4 @@
+import os
 import sys
 
 import abrek.command
@@ -82,3 +83,42 @@
         except Exception as strerror:
             print "Test uninstall error: %s" % strerror
             sys.exit(1)
+
+class cmd_list_installed(abrek.command.AbrekCmd):
+    """
+    List tests that are currently installed
+    """
+    def run(self, argv):
+        from abrek.config import get_config
+        config = get_config()
+        print "Installed tests:"
+        try:
+            for dir in os.listdir(config.installdir):
+                print dir
+        except OSError:
+            print "No tests installed"
+
+class cmd_list_tests(abrek.command.AbrekCmd):
+    """
+    List all known tests
+    """
+    def run(self, argv):
+        from abrek import test_definitions
+        from pkgutil import walk_packages
+        print "Known tests:"
+        for importer, mod, ispkg in walk_packages(test_definitions.__path__):
+            print mod
+
+class cmd_list_results(abrek.command.AbrekCmd):
+    """
+    List results of previous runs
+    """
+    def run(self, argv):
+        from abrek.config import get_config
+        config = get_config()
+        print "Saved results:"
+        try:
+            for dir in os.listdir(config.resultsdir):
+                print dir
+        except OSError:
+            print "No results found"

=== modified file 'abrek/config.py'
--- abrek/config.py	2010-06-21 19:45:49 +0000
+++ abrek/config.py	2010-07-09 21:51:42 +0000
@@ -11,3 +11,15 @@
         self.installdir = os.path.join(basedata, 'abrek', 'installed-tests')
         self.resultsdir = os.path.join(basedata, 'abrek', 'results')
 
+_config = None
+
+def get_config():
+    global _config
+    if _config is not None:
+        return _config
+    return AbrekConfig()
+
+def set_config(config):
+    global _config
+    _config = config
+

=== modified file 'abrek/testdef.py'
--- abrek/testdef.py	2010-06-29 16:47:51 +0000
+++ abrek/testdef.py	2010-07-09 21:51:42 +0000
@@ -7,7 +7,7 @@
 from commands import getstatusoutput
 from datetime import datetime
 
-import abrek.config
+from abrek.config import get_config
 from abrek.utils import geturl, write_file
 
 class AbrekTest(object):
@@ -25,7 +25,7 @@
     """
     def __init__(self, testname, version="", installer=None, runner=None,
                  parser=None):
-        self.config = abrek.config.AbrekConfig()
+        self.config = get_config()
         self.testname = testname
         self.version = version
         self.installer = installer

=== modified file 'tests/__init__.py'
--- tests/__init__.py	2010-06-28 20:23:02 +0000
+++ tests/__init__.py	2010-07-09 21:51:42 +0000
@@ -1,7 +1,8 @@
 import unittest
 
 def test_suite():
-    module_names = ['tests.test_abrekcmd',
+    module_names = ['tests.test_builtins',
+                    'tests.test_abrekcmd',
                     'tests.test_abrektestinstaller',
                     'tests.test_abrektestrunner']
     loader = unittest.TestLoader()

=== added file 'tests/test_builtins.py'
--- tests/test_builtins.py	1970-01-01 00:00:00 +0000
+++ tests/test_builtins.py	2010-07-09 21:51:42 +0000
@@ -0,0 +1,53 @@
+import os
+import shutil
+import sys
+import tempfile
+import unittest
+import StringIO
+
+import abrek.builtins
+from abrek.config import set_config
+
+class FakeOutputTests(unittest.TestCase):
+    def setUp(self):
+        self.origstdout = sys.stdout
+        sys.stdout = self.fakestdout = StringIO.StringIO()
+
+    def tearDown(self):
+        sys.stdout = self.origstdout
+
+class ListKnown(FakeOutputTests):
+    def test_list_tests(self):
+        cmd = abrek.builtins.cmd_list_tests()
+        cmd.run("argv_junk")
+        self.assertTrue("stream" in self.fakestdout.getvalue())
+
+class FakeConfigTests(FakeOutputTests):
+    def setUp(self):
+        super(FakeConfigTests, self).setUp()
+        class fakeconfig:
+            def __init__(self, basedir):
+                self.configdir = os.path.join(basedir, "config")
+                self.installdir = os.path.join(basedir, "install")
+                self.resultsdir = os.path.join(basedir, "results")
+        self.tmpdir = tempfile.mkdtemp()
+        self.config = fakeconfig(self.tmpdir)
+        set_config(self.config)
+
+    def tearDown(self):
+        super(FakeConfigTests, self).tearDown()
+        shutil.rmtree(self.tmpdir)
+
+    def test_list_installed(self):
+        test_name="test_list_installed000"
+        os.makedirs(os.path.join(self.config.installdir, test_name))
+        cmd = abrek.builtins.cmd_list_installed()
+        cmd.run("argv_junk")
+        self.assertTrue(test_name in self.fakestdout.getvalue())
+
+    def test_list_results(self):
+        result_name = "test_list_results000"
+        os.makedirs(os.path.join(self.config.resultsdir, result_name))
+        cmd = abrek.builtins.cmd_list_results()
+        cmd.run("argv_junk")
+        self.assertTrue(result_name in self.fakestdout.getvalue())