[tor-commits] [ooni-probe/master] Cleaning up test abort and skip code.

isis at torproject.org isis at torproject.org
Tue Dec 18 05:53:46 UTC 2012


commit 673aaa698a0cce371ef404ca4868dae21d3365e6
Author: Isis Lovecruft <isis at torproject.org>
Date:   Wed Dec 5 20:38:58 2012 +0000

    Cleaning up test abort and skip code.
    
    * Using log.catch in the form
          log.catch(func, *args, **kwargs)
      somehow causes '_catch' to be registered as a test_method, so calls in this
      manner have been removed.
    * Cleaned up excessive calls to reactor.run() and reactor.stop(). reactor.stop()
      should only be called if reactor.running is True.
---
 nettests/bridge_reachability/tcpsyn.py |    2 +-
 ooni/nettest.py                        |   13 +++--
 ooni/oonicli.py                        |    3 +-
 ooni/reporter.py                       |   15 +++---
 ooni/runner.py                         |   81 ++++++++++++++++----------------
 5 files changed, 59 insertions(+), 55 deletions(-)

diff --git a/nettests/bridge_reachability/tcpsyn.py b/nettests/bridge_reachability/tcpsyn.py
index f92ab09..548fc0e 100644
--- a/nettests/bridge_reachability/tcpsyn.py
+++ b/nettests/bridge_reachability/tcpsyn.py
@@ -73,7 +73,7 @@ class TCPFlagTest(nettest.NetTestCase):
                 setattr(self, key, value)
         if not self.interface:
             try:
-                iface = log.catch(net.getDefaultIface())
+                iface = net.getDefaultIface()
             except net.IfaceError, ie:
                 log.warn("Could not find a working network interface!")
                 log.fail(ie)
diff --git a/ooni/nettest.py b/ooni/nettest.py
index 9aa7a70..1d1477d 100644
--- a/ooni/nettest.py
+++ b/ooni/nettest.py
@@ -200,12 +200,15 @@ class NetTestCase(object):
                           category=DeprecationWarning)
             return txtrutil.DEFAULT_TIMEOUT_DURATION
     
-    def _abort(self, reason, obj=None):
+    def _abort(self, reason):
         """
-        Abort running an input, test_method, or test_class. If called with only
-        one argument, assume we're going to ignore the current input. Otherwise,
-        the name of the method or class in relation to the test_instance,
-        i.e. "self" should be given as value for the keyword argument "obj".
+
+        Abort running the current input. Raises
+        :class:`twisted.trial.test.skipping.SkipTest <SkipTest>` test_method,
+        or test_class. If called with only one argument, assume we're going to
+        ignore the current input. Otherwise, the name of the method or class
+        in relation to the test_instance, i.e. "self" should be given as value
+        for the keyword argument "obj".
 
         XXX call oreporter.allDone() from parent stack frame
         """
diff --git a/ooni/oonicli.py b/ooni/oonicli.py
index 9c2b4e6..3362d06 100644
--- a/ooni/oonicli.py
+++ b/ooni/oonicli.py
@@ -134,5 +134,6 @@ def run():
                                   cmd_line_options, yamloo_filename)
     tests_d.addBoth(testsEnded)
 
-    reactor.run()
+    ## it appears that tests run without this?
+    #reactor.run()
 
diff --git a/ooni/reporter.py b/ooni/reporter.py
index aec1e1b..193d056 100644
--- a/ooni/reporter.py
+++ b/ooni/reporter.py
@@ -130,13 +130,14 @@ class OReporter(object):
         return self.writeReportEntry(report)
 
     def allDone(self):
-        log.debug("allDone: Finished running all tests")
-        try:
-            log.debug("Stopping the reactor")
-            reactor.stop()
-        except:
-            log.debug("Unable to stop the reactor")
-            pass
+        log.debug("Running pending timed reactor calls")
+        reactor.runUntilCurrent()
+        if reactor.running:
+            log.debug("Reactor running. Stopping the reactor...")
+            try:
+                reactor.stop()
+            except:
+                log.debug("Unable to stop the reactor")
         return None
 
 class YAMLReporter(OReporter):
diff --git a/ooni/runner.py b/ooni/runner.py
index 1895d8b..2b41d59 100644
--- a/ooni/runner.py
+++ b/ooni/runner.py
@@ -21,6 +21,7 @@ from twisted.trial.runner import filenameToModule
 from twisted.trial import util as txtrutil
 from twisted.trial import reporter as txreporter
 from twisted.trial.unittest import utils as txtrutils
+from twisted.trial.unittest import SkipTest
 from twisted.internet import reactor, threads
 
 from ooni.inputunit import InputUnitFactory
@@ -75,7 +76,7 @@ def processTest(obj, cmd_line_options):
             obj.localOptions = options
 
         if input_file and options:
-            log.debug("Got input file")
+            log.debug("Added input file to options list")
             obj.inputFile = options[input_file[0]]
 
         try:
@@ -181,80 +182,78 @@ def runTestWithInput(test_class, test_method, test_input, oreporter):
     """
     log.debug("Running %s with %s" % (test_method, test_input))
 
-    def test_abort_single_input(reason, test_instance, test_name):
-        pass
-
     def test_timeout(d):
-        err = defer.TimeoutError("%s test for %s timed out after %s seconds"
-                                 % (test_name, test_instance.input, 
-                                    test_instance.timeout))
-        fail = failure.Failure(err)
+        timeout_error = defer.TimeoutError(
+            "%s test for %s timed out after %s seconds"
+            % (test_name, test_instance.input, test_instance.timeout))
+        timeout_fail = failure.Failure(err)
         try:
-            d.errback(fail)
+            d.errback(timeout_fail)
         except defer.AlreadyCalledError:
             # if the deferred has already been called but the *back chain is
             # still unfinished, crash the reactor and report the timeout
             reactor.crash()
             test_instance._timedOut = True    # see test_instance._wait
-            # XXX result is TestResult utils? 
             test_instance._test_result.addExpectedFailure(test_instance, fail)
     test_timeout = txtrutils.suppressWarnings(
         test_timeout, txtrutil.suppress(category=DeprecationWarning))
 
     def test_done(result, test_instance, test_name):
-        log.debug("runTestWithInput: concluded %s" % test_name)
+        log.debug("Concluded %s with inputs %s"
+                  % (test_name, test_instance.input))
         return oreporter.testDone(test_instance, test_name)
 
     def test_error(error, test_instance, test_name):
-        log.exception(error)
+        if isinstance(error, SkipTest):
+            log.info("%s" % error.message)
+        else:
+            log.exception(error)
 
     test_instance = test_class()
     test_instance.input = test_input
     test_instance.report = {}
-    log.debug("Processing %s" % test_instance.name)
+    # XXX TODO
+    # the twisted.trial.reporter.TestResult is expected by test_timeout(),
+    # but we should eventually replace it with a stub class
+    test_instance._test_result = txreporter.TestResult()
     # use this to keep track of the test runtime
     test_instance._start_time = time.time()
     test_instance.timeout = test_instance._getTimeout()
-    test_instance._test_result = txreporter.TestResult()
     # call setups on the test
     test_instance._setUp()
     test_instance.setUp()
-    test_ignored = txtrutil.acquireAttribute(test_instance._parents, 
-                                             'skip', None)
-
-    test = getattr(test_instance, test_method)
-
-    # check if we've aborted
-    test_skip = test_instance._getSkip()
-    if test_skip is not None:
-        log.debug("%s.getSkip() returned %s" % (str(test_class), 
-                                                str(test_skip)) )
- 
-    abort_reason, abort_what = getattr(test_instance, 'abort', ('input', None))
-    if abort_reason is not None:
-        do_abort = abortTestWasCalled(abort_reason, abort_what, test_class,
-                                      test_instance, test_method, test_input,
-                                      oreporter)
-        return defer.maybeDeferred(do_abort)
+
+    # check that we haven't inherited a skip
+    test_ignored = txtrutil.acquireAttribute(
+        test_instance._parents, 'skip', None)
+    if test_ignored is not None:
+        # XXX we'll need to do something more than warn
+        log.warn("test_skip is %s" % test_ignored)
+
+    # now check our instance for test_methods set to be skipped:
+    skip_list = test_instance._getSkip()
+    if skip_list is not None:
+        log.debug("%s marked these tests to be skipped: %s"
+                  % (test_instance.name, skip_list))
     else:
+        log.debug("No tests marked as skip")
+    skip_list = [skip_list]
+
+    if not test_method in skip_list:
+        test = getattr(test_instance, test_method)
         d = defer.maybeDeferred(test)
 
         # register the timer with the reactor
-        call = reactor.callLater(test_timeout, test_timed_out, d)
+        call = reactor.callLater(test_instance.timeout, test_timeout, d)
         d.addBoth(lambda x: call.active() and call.cancel() or x)
-
-        # XXX check if test called test_abort...
-        d.addCallbacks(test_abort, 
-                       test_error, 
-                       callbackArgs=(test_instance, test_method), 
-                       errbackArgs=(test_instance, test_method) )
+    
         d.addCallback(test_done, test_instance, test_method)
         d.addErrback(test_error, test_instance, test_method)
         log.debug("returning %s input" % test_method)
+    else:
+        d = defer.Deferred()
 
-        ignored = d.getSkip()
-
-        return d
+    return d
 
 def runTestWithInputUnit(test_class, test_method, input_unit, oreporter):
     """





More information about the tor-commits mailing list