Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve installed-tests and run them during Github CI #521

Merged
merged 10 commits into from Sep 14, 2020

Conversation

smcv
Copy link
Collaborator

@smcv smcv commented Aug 5, 2020

Includes #525, #520 and part of #503.

  • All three commits from Fix openuri test regression (#524) #525, see that PR for details

  • xdp: Send messages to stderr, not stdout

    This is done for two reasons:

    1. Logging on stdout can interfere with machine-readable protocols.

    2. stdout is normally fully buffered, whereas stderr is normally
      line-buffered. It's a lot easier to debug the service when messages
      don't lag behind.

  • tests: Divert service stdout to stderr, see PR tests: Divert service stdout to stderr #520 for details

  • tests: use kill from PATH (from @jtojnar on tests: Try to fix installed tests #503)

    It is not guaranteed that /bin/kill exists and we are already using the bash builtin elsewhere in the script.

  • Install session.conf.in to expected path for installed-tests

  • tests: call test-document-fuse.py directly (from @jtojnar on tests: Try to fix installed tests #503)

    It has a shebang so no need to pass it to python3 in the tests.

    It also makes it easier for downstreams to wrap the python file for setting extra environment variables.

    [smcv: chmod the script +x]

  • tests: Only run xdg-document-portal in foreground when uninstalled

    During build-time testing, we need to take special steps to run the
    just-built version of the portal. However, during "as-installed"
    testing we want to assert that it was installed correctly, and in
    particular that D-Bus .service file was installed correctly to make it
    activatable.

  • test-document-fuse: Quote more defensively

  • tests: Send SIGTERM to dbus-daemon

    There's no real need to kill it with SIGKILL, and in some configurations
    there is cleanup that is only performed when allowed to shut down
    gracefully.

  • CI: Correct the name of the GLib package

    We want development headers, so libglib2.0-dev rather than libglib2.0-0.

  • CI: Run installed-tests


Known omissions:

  • We don't run the FUSE tests (but they'll probably fail or be skipped anyway in any CI environment that is based on containers)
  • We don't have libcap2-bin (capsh) so some tests will be skipped
  • We don't have Pipewire

I'm in the process of getting Pipewire 0.3 into Debian. When that has happened, the run that includes installed-tests could maybe be switched from Ubuntu 18.04 to Debian 11?

@smcv
Copy link
Collaborator Author

smcv commented Aug 5, 2020

This works for me on Debian unstable, using:

./configure --prefix="$HOME/tmp/usr" --disable-pipewire --disable-libportal --enable-installed-tests
make
make check
make install
XDG_DATA_DIRS="$HOME/tmp/usr/share:$XDG_DATA_DIRS" ginsttest-runner -d "$HOME/tmp/usr/share" xdg-desktop-portal

(I know that doesn't run all tests, but in particular it does cover the FUSE document-portal test.)

@smcv
Copy link
Collaborator Author

smcv commented Aug 5, 2020

With TEST_IN_CI=1 and --enable-libportal, test-portals.test hangs after /portal/openuri/exists. I think that should probably be handled as a separate issue.

Fixed in #525

@smcv
Copy link
Collaborator Author

smcv commented Aug 5, 2020

I should drop the CI bits of this branch for now, since CI is consistently timing out anyway.

Fixed in #525

@smcv smcv force-pushed the installed-tests branch 4 times, most recently from a6569a0 to e004955 Compare September 6, 2020 22:28
@smcv smcv marked this pull request as ready for review September 6, 2020 22:34
@smcv smcv changed the title Improve installed-tests Improve installed-tests and run them during Github CI Sep 6, 2020
@smcv smcv force-pushed the installed-tests branch 2 times, most recently from d97cf60 to 478bd4e Compare September 7, 2020 13:20
@smcv
Copy link
Collaborator Author

smcv commented Sep 7, 2020

Rebased on latest #525.

smcv and others added 10 commits September 9, 2020 13:01
This is done for two reasons:

1. Logging on stdout can interfere with machine-readable protocols.

2. stdout is normally fully buffered, whereas stderr is normally
   line-buffered. It's a lot easier to debug the service when messages
   don't lag behind.

Signed-off-by: Simon McVittie <smcv@collabora.com>
These tests produce structured TAP output on stdout, so we cannot run
any subprocesses that would produce unstructured stdout. Unfortunately,
by default g_debug() writes to stdout, so running with G_MESSAGES_DEBUG
to get better logging from the various subprocesses can break the test.
Use stderr instead.

Signed-off-by: Simon McVittie <smcv@debian.org>
It is not guaranteed that /bin/kill exists and we are already using the bash builtin elsewhere in the script.
Signed-off-by: Simon McVittie <smcv@debian.org>
It has a shebang so no need to pass it to python3 in the tests.

It also makes it easier for downstreams to wrap the python file for setting extra environment variables.

[smcv: chmod the script +x]
During build-time testing, we need to take special steps to run the
just-built version of the portal. However, during "as-installed"
testing we want to assert that it was installed correctly, and in
particular that D-Bus .service file was installed correctly to make it
activatable.

Signed-off-by: Simon McVittie <smcv@debian.org>
Signed-off-by: Simon McVittie <smcv@debian.org>
There's no real need to kill it with SIGKILL, and in some configurations
there is cleanup that is only performed when allowed to shut down
gracefully.

Signed-off-by: Simon McVittie <smcv@debian.org>
We want development headers, so libglib2.0-dev rather than libglib2.0-0.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Signed-off-by: Simon McVittie <smcv@collabora.com>
@smcv
Copy link
Collaborator Author

smcv commented Sep 9, 2020

Rebased to reflect #525 having been merged; no practical changes.

@matthiasclasen matthiasclasen merged commit 024fc30 into flatpak:master Sep 14, 2020
@smcv smcv deleted the installed-tests branch September 14, 2020 16:09
smcv added a commit to flatpak/ppa-xdg-desktop-portal that referenced this pull request Sep 10, 2021
This includes upstream 1.7.2-37-g089e72b plus merge request
<flatpak/xdg-desktop-portal#521>.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants