Executing autopilot test suite fails to close when piped to tee

autopilot.tests.functional.test_open_window.OpenWindowTests.test_open_window appears to be the wonky test. I can reproduce by just running this test:

autopilot run autopilot.tests.functional.test_open_window.OpenWindowTests.test_open_window | tee /tmp/foo

Upon further investigation, the problem seems to be that the test apps that are launched by that test leak their stdout descriptor.

What's interesting is if I run:

python -c "from gi.repository import Gio; Gio.DesktopAppInfo.new('gcalctool.desktop').launch_uris([], None)" | tee /tmp/foo

tee will exit as soon as I click the close button on the window. The test code closes the window by sending the NET_WM_CLOSE EWMH signal, which, AFAICT, should be the same thing.

launch_uris() indeed inherits stdout/stderr to the child, which is usually what you want. But not in this case, when we exit the parent process before the child.

The launch_uris_as_manager() method gives more flexibility, in particular you can give custom GLib.SpawnFlags to it (launch_uris() just uses GLib.SpawnFlags.SEARCH_PATH). Unfortunately this is currently rather awkward to use from Python as it's missing some annotations:

$ python -c "from gi.repository import GLib, Gio; Gio.DesktopAppInfo.new('gcalctool.desktop').launch_uris_as_manager([], None, GLib.SpawnFlags.STDOUT_TO_DEV_NULL, None, None, None, None)" | tee /tmp/foo
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: Argument 2 does not allow None as a value

If I hack the annotations to add the missing allow-none's, it works fine:

$ python -c "from gi.repository import GLib, Gio; Gio.DesktopAppInfo.new('gcalctool.desktop').launch_uris_as_manager([], None, GLib.SpawnFlags.SEARCH_PATH | GLib.SpawnFlags.STDOUT_TO_DEV_NULL, None, None, None, None)" | tee /tmp/foo

This now immediately returns.

To roll this fix out into the distro, we only need to upload gobject-introspection.

I'm afraid I can't reproduce the original problem here. If I run this test, I see a few windows (Mahjong, terminal, etc.) flying by, and after a few seconds it crashes with

$ autopilot run autopilot.tests.functional.test_open_window.OpenWindowTests.test_open_window
Loading tests from: /usr/lib/python2.7/dist-packages

Tests running...

** (gedit:16944): WARNING **: Could not load Gedit repository: Typelib file for namespace 'GtkSource', version '3.0' not found
======================================================================
ERROR: autopilot.tests.functional.test_open_window.OpenWindowTests.test_open_window(System Settings)
----------------------------------------------------------------------
Traceback (most recent call last):
_StringException: Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/autopilot/tests/functional/test_open_window.py", line 44, in test_open_window
    win = self.process_manager.start_app_window(self.app_name)
  File "/usr/lib/python2.7/dist-packages/autopilot/process/_bamf.py", line 139, in start_app_window
    window = self._open_window(app_name, files, locale)
  File "/usr/lib/python2.7/dist-packages/autopilot/process/_bamf.py", line 164, in _open_window
    self._launch_application(app['desktop-file'], files)
  File "/usr/lib/python2.7/dist-packages/autopilot/process/_bamf.py", line 329, in _launch_application
    proc = Gio.DesktopAppInfo.new(desktop_file)
TypeError: constructor returned NULL

Ran 7 tests in 8.227s
FAILED (failures=1)

and it behaves exactly the same way with "... | tee /tmp/foo". Trunk shows the exact same behaviour with

  python -m autopilot.run run autopilot.tests.functional.test_open_window.OpenWindowTests.test_open_window | tee /tmp/foo

I understand this would only hang if there is an application which does not close automatically again. Does that happen in your case?

Annotations fixed in glib upstream: https://git.gnome.org/browse/glib/commit/?id=e6c86d5

Hang on, I *can* reproduce the autopilot hang now. Perhaps it doesn't happen everytime, but I got it two times in a row now.

The linked MP fixes the hang, but with that we won't see the launched program's stdout any more. It's a rather blunt fix, but otherwise we need to make up our mind:

  - do we want to see subprocess stdout and be responsible for killing our subprocesses on test case tearDown

or

  - do we not want to care about cleaning up spawned processes (e. g. we don't guarantee that we kill them on teardown), but then we lose their stdout

We can't have both. We can probably fix the root cause in autopilot (if you take out remmina from autopilot/process/__init__.py it fixes this problem as well, as remmina stays running after the test case ends), but it might be desirable to impose this restriction on app tests that use AP?

I attached a second MP which fixes the remmina process leak. Pick your poison :-)

Personally I lean towards the second one and fix other tests which also leak processes, but you might have a different opinion.

Merged second MP, commented on first. Thanks for your help.

This bug was fixed in the package gobject-introspection - 1.39.0-0ubuntu1

---------------
gobject-introspection (1.39.0-0ubuntu1) trusty; urgency=low

  * New upstream release.
  * Bump libffi, glib, and gtk-doc build dependencies according to upstream
    configure.ac.
  * Add 00git_gio_annotations.patch: Update glib annotations to latest glib
    git head. In particular, add missing annotations to
    g_desktop_app_info_launch_uris_as_manager(). (LP: #1259721)
  * debian/tests/control: Separate the two autopkgtests for more fine-grained
    dependencies, and add missing "file" dependency for "tools". Allow stderr
    output in the latter as we sometimes have warnings.
 -- Martin Pitt <email address hidden> Wed, 11 Dec 2013 15:15:08 +0100

Fix committed into lp:autopilot at revision None, scheduled for release in autopilot, milestone 1.4

This bug was fixed in the package autopilot - 1.4+14.04.20140123.1-0ubuntu1

---------------
autopilot (1.4+14.04.20140123.1-0ubuntu1) trusty; urgency=low

  [ Barry Warsaw ]
  * Make Autopilot tracepoint extension module compatible with python 3.
    (LP: #1266574)

  [ Christopher Lee ]
  * Handle xml parse exception nicely as well as add some nicer
    logging/feedback.
  * Fixes issues with load_test_suite_from_name.
  * Make autopilot able to run it's own tests again.
  * When setting up logging take into account not all modes/commands
    have a verbose argument.
  * Fixes the issue when listing an non-existent test suite raised an
    uncaught exception. .
  * Minor fix for a failing functional test, now passes under python 2
    and 3.
  * Minor fixup of TypeError in platform docs re: skipping tests.
  * Add details to Faq docs re: launching (click) applications. (LP:
    #1257148)
  * Refactor of the application launching code (incl. tests).
  * Fix issue with launching click app and added 100% unit test to cover
    it.
  * Fixes the issue where test discovery gets confused when there are
    local (in cwd) and system installed tests.

  [ Thomi Richards ]
  * Fix tox.ini config file to restrict flake8 runs to the autopilot/
    directory.
  * Make autopilot fail when no command is specified. (LP: #1255334)
  * Move the contents of the 'autopilot' script into autopilot/run.py,
    and make setuptools generate the autopilot script for us.
  * Make autopilot understand how the "-qt=XXX" option works. (LP:
    #1255405)
  * Handle xml parse exception nicely as well as add some nicer
    logging/feedback.
  * Fix autopilot test case loading. (LP: #1255752, #1255659)
  * Move the BackendException class to solve a circular import.
  * Make autopilot support subunit bytestream output. (LP: #1255662)
  * Make autopilot able to run it's own tests again.
  * Remove an incorrect comment from the source code.
  * Fix some unit tests that were printing to stdout.
  * Lay the groundwork for attaching files to test results.
  * Fix failing tests in python3.
  * Add click package log file to test result. (LP: #1257453)
  * Add a simple shell script that can be used to generate unit test
    coverage for autopilot itself.
  * Add unit tests for common input code.
  * Remove some unused code code in the type support unit tests.
  * Don't include test lines in coverage count.
  * Add a few missing test cases, increase test case coverage.
  * Add unit tests for process snapshot support.
  * A few tweaks to the make_coverage script to make it a bit more
    useful.
  * Fix issue with launching click app and added 100% unit test to cover
    it.
  * Fix eventually matcher so it can match against complex types. (LP:
    #1269984)

  [ Martin Pitt ]
  * Fix initialization of GdkDisplay, to fix crash when accessing
    Window.geometry. (LP: #1258170)
  * tests: Ensure we don't leave remmina and other processes open. (LP:
    #1259721)
  * Don't inherit our stdout to spawned processes, to allow users to
    redirect autopilot's stdout to tee and other programs which wait for
    EOF. (LP: #1259721)
  * Fix functional tests to actually run against the build t...