Merge branch '1.3' into 1.4

Author: bitprophet

.travis.yml

@@ -16,6 +16,11 @@ python:
 matrix:
   allow_failures:
     - python: "3.8-dev"
+# WHY does this have to be in before_install and not install? o_O
+before_install:
+  # Used by 'inv regression' (more performant/safe/likely to expose real issues
+  # than in-Python threads...)
+  - sudo apt-get -y install parallel
 install:
   # For some reason Travis' build envs have wildly different pip/setuptools
   # versions between minor Python versions, and this can cause many hilarious
@@ -45,6 +50,8 @@ before_script:
 script:
   # Execute full test suite + coverage, as the new sudo-capable user
   - inv travis.sudo-coverage
+  # Perform extra "not feasible inside pytest for no obvious reason" tests
+  - inv regression
   # Websites build OK? (Not on PyPy3, Sphinx is all "who the hell are you?" =/
   - "if [[ $TRAVIS_PYTHON_VERSION != 'pypy3' ]]; then inv sites; fi"
   # Doctests in websites OK? (Same caveat as above...)

integration/_support/regression.py

@@ -0,0 +1,34 @@
+"""
+Barebones regression-catching script that looks for ephemeral run() failures.
+
+Intended to be run from top level of project via ``inv regression``. In an
+ideal world this would be truly part of the integration test suite, but:
+
+- something about the outer invoke or pytest environment seems to prevent such
+  issues from appearing reliably (see eg issue #660)
+- it can take quite a while to run, even compared to other integration tests.
+"""
+
+
+import sys
+
+from invoke import task
+
+
+@task
+def check(c):
+    count = 0
+    failures = []
+    for _ in range(0, 1000):
+        count += 1
+        try:
+            # 'ls' chosen as an arbitrary, fast-enough-for-looping but
+            # does-some-real-work example (where eg 'sleep' is less useful)
+            response = c.run("ls", hide=True)
+            if not response.ok:
+                failures.append(response)
+        except Exception as e:
+            failures.append(e)
+    if failures:
+        print("run() FAILED {}/{} times!".format(len(failures), count))
+        sys.exit(1)

integration/runners.py

@@ -126,12 +126,12 @@ def nonpty_subproc_should_not_hang_if_IO_thread_has_an_exception(self):
 
     class timeouts:
         def does_not_fire_when_command_quick(self):
-            assert Local(Context()).run("sleep 1", timeout=5)
+            assert run("sleep 1", timeout=5)
 
         def triggers_exception_when_command_slow(self):
             before = time.time()
             with raises(CommandTimedOut) as info:
-                Local(Context()).run("sleep 5", timeout=0.5)
+                run("sleep 5", timeout=0.5)
             after = time.time()
             # Fudge real time check a bit, <=0.5 typically fails due to
             # overhead etc. May need raising further to avoid races? Meh.

invoke/util.py

@@ -271,8 +271,7 @@ def is_dead(self):
         # NOTE: it seems highly unlikely that a thread could still be
         # is_alive() but also have encountered an exception. But hey. Why not
         # be thorough?
-        alive = self.is_alive() and (self.exc_info is None)
-        return not alive
+        return (not self.is_alive()) and self.exc_info is not None
 
     def __repr__(self):
         # TODO: beef this up more

sites/www/changelog.rst

@@ -2,6 +2,11 @@
 Changelog
 =========
 
+- :bug:`660` Fix an issue with `~invoke.run` & friends having intermittent
+  problems at exit time (symptom was typically about the exit code value being
+  ``None`` instead of an integer; often with an exception trace). Thanks to
+  Frank Lazzarini for the report and to the numerous others who provided
+  reproduction cases.
 - :bug:`518` Close pseudoterminals opened by the `~invoke.runners.Local` class
   during ``run(..., pty=True)``. Previously, these were only closed
   incidentally at process shutdown, causing file descriptor leakage in

tasks.py

@@ -72,10 +72,23 @@ def coverage(c, report="term", opts=""):
     return coverage_(c, report=report, opts=opts, tester=test)
 
 
+@task
+def regression(c, jobs=8):
+    """
+    Run an expensive, hard-to-test-in-pytest run() regression checker.
+
+    :param int jobs: Number of jobs to run, in total. Ideally num of CPUs.
+    """
+    os.chdir("integration/_support")
+    cmd = "seq {} | parallel -n0 --halt=now,fail=1 inv -c regression check"
+    c.run(cmd.format(jobs))
+
+
 ns = Collection(
     test,
     coverage,
     integration,
+    regression,
     vendorize,
     release,
     www,

tests/concurrency.py

@@ -32,14 +32,20 @@ def catches_exceptions(self):
             assert isinstance(wrapper.value, AttributeError)
 
         def exhibits_is_dead_flag(self):
+            # Spin up a thread that will except internally (can't put() on a
+            # None object)
             t = EHThread(target=self.worker, args=[None])
             t.start()
             t.join()
+            # Excepted -> it's dead
             assert t.is_dead
+            # Spin up a happy thread that can exit peacefully (it's not "dead",
+            # though...maybe we should change that terminology)
             t = EHThread(target=self.worker, args=[Queue()])
             t.start()
             t.join()
-            assert t.is_dead
+            # Not dead, just uh...sleeping?
+            assert not t.is_dead
 
     class via_subclassing:
         def setup(self):
@@ -73,11 +79,17 @@ def catches_exceptions(self):
             assert isinstance(wrapper.value, AttributeError)
 
         def exhibits_is_dead_flag(self):
+            # Spin up a thread that will except internally (can't put() on a
+            # None object)
             t = self.klass(queue=None)
             t.start()
             t.join()
+            # Excepted -> it's dead
             assert t.is_dead
+            # Spin up a happy thread that can exit peacefully (it's not "dead",
+            # though...maybe we should change that terminology)
             t = self.klass(queue=Queue())
             t.start()
             t.join()
-            assert t.is_dead
+            # Not dead, just uh...sleeping?
+            assert not t.is_dead