Merge lp:~dobey/ubuntuone-dev-tools/trial-args into lp:ubuntuone-dev-tools

[dobey]

Use the Options handling API of trial so unhandled options get passed through
Fix some style issues
Make test_dbus work with --loop argument

[lp:~dobey/ubuntuone-dev-tools/trial-args]

49. By dobey on 2011-11-21

Also need to override opt_r in Options

[Sidnei da Silva (sidnei)]

1. Typo at 'Need to et list of reactors and print them here.'

2. Does help-reactors need to be overriden? Seems like trial already implements it.

Other than that that, +1!

review: Approve

[lp:~dobey/ubuntuone-dev-tools/trial-args]

50. By dobey on 2011-11-21

Update (C) years also

51. By dobey on 2011-11-21

Fix the typo from review

[dobey (dobey)]

On Mon, 2011-11-21 at 19:37 +0000, Sidnei da Silva wrote:
> 2. Does help-reactors need to be overriden? Seems like trial already implements it.

Typo fixed. And yes, we need to override it, as we support some reactors
which are not installed in the twisted directory, as well as needing to
do some additional work on top of just importing them, for running some
tests that poke at the UI. I would be very happy to not have to
implement the reactor bits here, but for now we have to. :)

[Roberto Alsina (ralsina)]

+1

review: Approve

=== modified file 'bin/u1lint'
--- bin/u1lint	2011-11-10 20:32:35 +0000
+++ bin/u1lint	2011-11-21 20:08:24 +0000
@@ -4,7 +4,7 @@
 #
 # Author: Rodney Dawes <[e-mail]>
 #
-# Copyright 2009-2010 Canonical Ltd.
+# Copyright 2009-2011 Canonical Ltd.
 #
 # This program is free software: you can redistribute it and/or modify it
 # under the terms of the GNU General Public License version 3, as published
@@ -209,7 +209,7 @@
     else:
         pylint_args = get_subprocess_start_info('pylint')
         # append the extra args to the start info
-        pylint_args.extend(['--output-format=parseable', 
+        pylint_args.extend(['--output-format=parseable',
             '--include-ids=yes'])
         if PYLINTRC:
             pylint_args.append("--rcfile=" + PYLINTRC)
 
=== modified file 'bin/u1trial'
--- bin/u1trial	2011-10-24 15:55:17 +0000
+++ bin/u1trial	2011-11-21 20:08:24 +0000
@@ -4,7 +4,7 @@
 #
 # Author: Rodney Dawes <[e-mail]>
 #
-# Copyright 2009-2010 Canonical Ltd.
+# Copyright 2009-2011 Canonical Ltd.
 #
 # This program is free software: you can redistribute it and/or modify it
 # under the terms of the GNU General Public License version 3, as published
@@ -28,12 +28,15 @@
 import sys
 import unittest
 
+from twisted.python.usage import UsageError
+from twisted.scripts import trial
 from twisted.trial.runner import TrialRunner
 
 sys.path.insert(0, os.path.abspath("."))
 
 from ubuntuone.devtools.testing.txcheck import TXCheckSuite
 
+
 def _is_in_ignored_path(testcase, paths):
     """Return if the testcase is in one of the ignored paths."""
     for ignored_path in paths:
@@ -45,12 +48,10 @@
 class TestRunner(TrialRunner):
     """The test runner implementation."""
 
-    def __init__(self, force_gc=False):
-        from twisted.trial.reporter import TreeReporter
-
+    def __init__(self, config=None):
         # set $HOME to the _trial_temp dir, to avoid breaking user files
         trial_temp_dir = os.environ.get('TRIAL_TEMP_DIR', os.getcwd())
-        homedir = os.path.join(trial_temp_dir, '_trial_temp')
+        homedir = os.path.join(trial_temp_dir, config['temp-directory'])
         os.environ['HOME'] = homedir
 
         # setup $XDG_*_HOME variables and create the directories
@@ -71,12 +72,29 @@
         # setup the ROOTDIR env var
         os.environ['ROOTDIR'] = os.getcwd()
 
+        # Need an attribute for tempdir so we can use it later
         self.tempdir = homedir
-        working_dir = os.path.join(self.tempdir, 'tmp')
-        super(TestRunner, self).__init__(reporterFactory=TreeReporter,
-                                         realTimeErrors=True,
-                                         workingDirectory=working_dir,
-                                         forceGarbageCollection=force_gc)
+        working_dir = os.path.join(self.tempdir, 'trial')
+
+        # Handle running trial in debug or dry-run mode
+        mode = None
+        if config['debug']:
+            mode = TrialRunner.DEBUG
+        if config['dry-run']:
+            mode = TrialRunner.DRY_RUN
+
+        # Hook up to the parent test runner
+        super(TestRunner, self).__init__(
+            reporterFactory=config['reporter'],
+            mode=mode,
+            profile=config['profile'],
+            logfile=config['logfile'],
+            tracebackFormat=config['tbformat'],
+            realTimeErrors=config['rterrors'],
+            uncleanWarnings=config['unclean-warnings'],
+            workingDirectory=working_dir,
+            forceGarbageCollection=config['force-gc'])
+
         self.required_services = []
         self.source_files = []
 
@@ -117,18 +135,6 @@
             pattern = re.compile('.*%s.*' % test_pattern)
         else:
             pattern = None
-            
-        # get the ignored modules/tests
-        if ignored_modules:
-            ignored_modules = map(str.strip, ignored_modules.split(','))
-        else:
-            ignored_modules = []
-
-        # get the ignored paths
-        if ignored_paths:
-            ignored_paths = map(str.strip, ignored_paths.split(','))
-        else:
-            ignored_paths = []
 
         # Disable this lint warning as we need to access _tests in the
         # test suites, to collect the tests
@@ -169,100 +175,150 @@
                             suite.addTests(module_suite)
         return suite
 
-    # pylint: disable=E0202
-    def run(self, args, options=None):
+    def get_suite(self, config):
+        """Get the test suite to use."""
+        suite = unittest.TestSuite()
+        for path in config['tests']:
+            suite.addTest(self._collect_tests(path, config['test'],
+                                              config['ignore-modules'],
+                                              config['ignore-paths']))
+        if config['loop']:
+            old_suite = suite
+            suite = unittest.TestSuite()
+            for _ in xrange(config['loop']):
+                suite.addTest(old_suite)
+
+        return suite
+
+    # pylint: disable=C0103
+    def _runWithoutDecoration(self, test):
         """run the tests."""
-        success = 0
+        result = None
         running_services = []
-        if options.coverage:
-            coverage.erase()
-            coverage.start()
 
         try:
-            suite = unittest.TestSuite()
-            for path in args:
-                print "Adding path"
-                suite.addTest(self._collect_tests(path, options.test,
-                                                  options.ignored_modules,
-                                                  options.ignored_paths))
-            if options.loops:
-                old_suite = suite
-                suite = unittest.TestSuite()
-                for _ in xrange(options.loops):
-                    suite.addTest(old_suite)
-
             # Start any required services
             for service in self.required_services:
                 runner = service()
                 runner.start_service(tempdir=self.tempdir)
                 running_services.append(runner)
 
-            result = super(TestRunner, self).run(suite)
-            success = result.wasSuccessful()
+            result = super(TestRunner, self)._runWithoutDecoration(test)
         finally:
             # Stop all the running services
             for runner in running_services:
                 runner.stop_service()
 
-        if options.coverage:
-            coverage.stop()
-            coverage.report(self.source_files, ignore_errors=True,
-                            show_missing=False)
-
-        if not success:
-            sys.exit(1)
-        else:
-            sys.exit(0)
+        return result
+
+
+class Options(trial.Options):
+    """Class for options handling."""
+
+    optFlags = [["coverage", "c"],
+                ["gui", None],
+                ["help-reactors", None],
+                ]
+
+    optParameters = [["test", "t", None],
+                     ["loop", None, 1],
+                     ["ignore-modules", "i", ""],
+                     ["ignore-paths", "p", ""],
+                     ["reactor", "r", "glib"],
+                     ]
+
+    def __init__(self):
+        self['tests'] = set()
+        super(Options, self).__init__()
+        self['rterrors'] = True
+
+    def opt_coverage(self):
+        """Generate a coverage report for the run tests"""
+        self['coverage'] = True
+
+    def opt_gui(self):
+        """Use the GUI mode of some reactors"""
+        self['gui'] = True
+
+    def opt_help_reactors(self):
+        """Help on available reactors for use with tests"""
+        synopsis = ('')
+        print synopsis
+        print 'Need to get list of reactors and print them here.'
+        print
+        sys.exit(0)
+
+    def opt_test(self, option):
+        """Run specific tests, e.g: className.methodName"""
+        self['test'] = option
+
+    def opt_loop(self, option):
+        """Loop tests the specified number of times."""
+        try:
+            self['loop'] = long(option)
+        except ValueError:
+            raise UsageError('A positive integer value must be specified.')
+
+    def opt_ignore_modules(self, option):
+        """Comma-separate list of test modules to ignore,
+           e.g: test_gtk.py, test_account.py
+           """
+        self['ignore-modules'] = map(str.strip, option.split(','))
+
+    def opt_ignore_paths(self, option):
+        """Comma-separated list of relative paths to ignore,
+           e.g: tests/platform/windows, tests/platform/macosx
+           """
+        self['ignore-paths'] = map(str.strip, option.split(','))
+
+    def opt_reactor(self, option):
+        """Which reactor to use (see --help-reactors for a list
+           of possibilities)
+           """
+        self['reactor'] = option
+    opt_r = opt_reactor
 
 
 def main():
     """Do the deed."""
-    from optparse import OptionParser
-    usage = '%prog [options] path'
-    parser = OptionParser(usage=usage)
-    parser.add_option("-t", "--test", dest="test",
-                  help = "run specific tests, e.g: className.methodName")
-    parser.add_option("-l", "--loop", dest="loops", type="int", default=1,
-                      help = "loop selected tests LOOPS number of times",
-                      metavar="LOOPS")
-    parser.add_option("-c", "--coverage", action="store_true", dest="coverage",
-                      help="print a coverage report when finished")
-    parser.add_option("-i", "--ignored-modules", dest="ignored_modules",
-                      default=None, help="comma-separated test moodules "
-                      + "to ignore, e.g: test_gtk.py, test_account.py")
-    parser.add_option("-p", "--ignore-paths", dest="ignored_paths",
-                      default=None, help="comma-separated relative "
-                      + "paths to ignore. "
-                      + "e.g: tests/platform/windows, tests/platform/macosx")
-    parser.add_option("--force-gc", action="store_true", dest="force_gc", 
-                      default=False, help="Run gc.collect() before and after "
-                      "each test case.")
-    parser.add_option("--reactor", type="string", dest="reactor",
-                      default='glib',
-                      help="Run the tests using the specified reactor.",
-                      metavar="REACTOR")
-    parser.add_option("--gui", action="store_true", dest="use_gui",
-                      help="Use the GUI mode of some reactors.")
-    (options, args) = parser.parse_args()
-    if not args:
-        parser.print_help()
-        sys.exit(2)
+    if len(sys.argv) == 1:
+        sys.argv.append('--help')
+
+    config = Options()
+    config.parseOptions()
 
     try:
-        reactor_name = 'ubuntuone.devtools.reactors.%s' % options.reactor
+        reactor_name = 'ubuntuone.devtools.reactors.%s' % config['reactor']
         reactor = __import__(reactor_name, None, None, [''])
     except ImportError:
         print 'The specified reactor is not supported.'
         sys.exit(1)
     else:
         try:
-            reactor.install(options=options)
+            reactor.install(options=config)
         except ImportError:
             print('The Python package providing the requested reactor is not '
                   'installed. You can find it here: %s' % reactor.REACTOR_URL)
             raise
 
-    TestRunner(force_gc=options.force_gc).run(args, options)
+    trial_runner = TestRunner(config=config)
+    suite = trial_runner.get_suite(config)
+
+    if config['coverage']:
+        coverage.erase()
+        coverage.start()
+
+    if config['until-failure']:
+        result = trial_runner.runUntilFailure(suite)
+    else:
+        result = trial_runner.run(suite)
+
+    if config['coverage']:
+        coverage.stop()
+        coverage.report(trial_runner.source_files, ignore_errors=True,
+                        show_missing=False)
+
+    sys.exit(not result.wasSuccessful())
 
 
 if __name__ == '__main__':
 
=== modified file 'run-tests'
--- run-tests	2011-05-26 15:49:41 +0000
+++ run-tests	2011-11-21 20:08:24 +0000
@@ -1,7 +1,7 @@
 #!/bin/bash
 # Author: Natalia Bidart <[e-mail]>
 #
-# Copyright 2010 Canonical Ltd.
+# Copyright 2010-2011 Canonical Ltd.
 #
 # This program is free software: you can redistribute it and/or modify it
 # under the terms of the GNU General Public License version 3, as published
@@ -18,7 +18,8 @@
 
 bin/u1trial -c ubuntuone
 bin/u1trial --reactor=twisted ubuntuone
+echo "Running style checks..."
 bin/u1lint
-pep8 --repeat .
+pep8 --repeat . bin/*
 rm -rf _trial_temp
 rm -rf .coverage
 
=== modified file 'ubuntuone/devtools/reactors/glib.py'
--- ubuntuone/devtools/reactors/glib.py	2011-05-26 15:49:41 +0000
+++ ubuntuone/devtools/reactors/glib.py	2011-11-21 20:08:24 +0000
@@ -17,7 +17,7 @@
 def install(options=None):
     """Install the reactor and parse any options we might need."""
     reactor_name = None
-    if options is not None and options.use_gui:
+    if options is not None and options['gui']:
         reactor_name = 'twisted.internet.gtk2reactor'
     else:
         reactor_name = 'twisted.internet.glib2reactor'
 
=== modified file 'ubuntuone/devtools/reactors/qt4.py'
--- ubuntuone/devtools/reactors/qt4.py	2011-11-11 13:31:18 +0000
+++ ubuntuone/devtools/reactors/qt4.py	2011-11-21 20:08:24 +0000
@@ -22,7 +22,7 @@
 
 def install(options=None):
     """Install the reactor and parse any options we might need."""
-    if options is not None and options.use_gui:
+    if options is not None and options['gui']:
         from PyQt4.QtGui import QApplication
         # We must assign this to a variable, or we will get crashes in Qt
         # pylint: disable=W0612
 
=== modified file 'ubuntuone/devtools/services/tests/test_dbus.py'
--- ubuntuone/devtools/services/tests/test_dbus.py	2011-02-03 14:32:01 +0000
+++ ubuntuone/devtools/services/tests/test_dbus.py	2011-11-21 20:08:24 +0000
@@ -1,6 +1,7 @@
 """Tests for the test runner."""
 
 import os
+import shutil
 
 from ubuntuone.devtools.testcase import DBusTestCase
 from ubuntuone.devtools.services.dbus import DBusRunner
@@ -25,4 +26,5 @@
         # pylint: disable=W0212
         os.makedirs(self.tmpdir)
         runner._find_config_file(tempdir=self.tmpdir)
+        shutil.rmtree(self.tmpdir)
         self.assertEqual(expected, runner.config_file)