This may happen in headless environments.
The return value of `gtk_init_check` was not examined before; according to [^1], _"calling any GTK function or instantiating any GTK type after this function returns FALSE results in undefined behavior."_
The value 77 can be returned to tell Automake that the test was skipped.[^2][^3]
---
Based on the example [here](https://www.gnu.org/software/automake/manual/automake.html#Use-TAP-with-the-...), I assumed that Autotools would mark `test_sidebar` as `SKIP` if all test cases inside are marked as skipped using [`g_test_skip`](https://docs.gtk.org/glib/func.test_skip.html) despite the return value being `0`, but with the way it is plugged together (each binary being one test from the view of the harness without regard for the more granular test cases inside), this doesn't work without further ado:
`test_sidebar.log`: ``` TAP version 13 # random seed: R02Seafe1e22758e0c4a49fe548501665a2d 1..3 # Start of sidebar tests ok 1 /sidebar/openfiles_none # SKIP Could not initizlize GTK, skipping. Headless environment? ok 2 /sidebar/openfiles_path # SKIP Could not initizlize GTK, skipping. Headless environment? ok 3 /sidebar/openfiles_tree # SKIP Could not initizlize GTK, skipping. Headless environment? # End of sidebar tests PASS test_sidebar (exit status: 0) ```
`test_sidebar.trs`: ``` :test-result: PASS :global-test-result: PASS :recheck: no :copy-in-global-log: no ```
`geany-2.0/tests $ env TESTS="test_sidebar" make -e check SUBDIRS=` ``` PASS: test_sidebar ============================================================================ Testsuite summary for Geany 2.0 ============================================================================ # TOTAL: 1 # PASS: 1 # SKIP: 0 # XFAIL: 0 # FAIL: 0 # XPASS: 0 # ERROR: 0 ============================================================================ ```
---
However, returning `77` works as intended:
`test_sidebar.log`: ``` GTK initialisation failed; skipping. Running inside a headless environment? SKIP test_sidebar (exit status: 77) ```
`test_sidebar.trs`: ``` :test-result: SKIP :global-test-result: SKIP :recheck: no :copy-in-global-log: yes ```
`geany-2.0/tests $ env TESTS="test_sidebar" make -e check SUBDIRS=` ``` SKIP: test_sidebar ============================================================================ Testsuite summary for Geany 2.0 ============================================================================ # TOTAL: 1 # PASS: 0 # SKIP: 1 # XFAIL: 0 # FAIL: 0 # XPASS: 0 # ERROR: 0 ============================================================================ ```
Fixes #3674
[^1]: https://docs.gtk.org/gtk3/func.init_check.html [^2]: https://www.gnu.org/software/automake/manual/automake.html#index-Exit-status... [^3]: https://docs.gtk.org/glib/func.test_run.html You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3676
-- Commit Summary --
* Skip `tests/test_sidebar` if GTK initialisation fails
-- File Changes --
M tests/test_sidebar.c (8)
-- Patch Links --
https://github.com/geany/geany/pull/3676.patch https://github.com/geany/geany/pull/3676.diff
Looking at the Automake-based Linux build logs here:
``` <snip> PASS: test_utils SKIP: test_sidebar ============================================================================ Testsuite summary for Geany 2.1 ============================================================================ # TOTAL: 2 # PASS: 1 # SKIP: 1 # XFAIL: 0 # FAIL: 0 # XPASS: 0 # ERROR: 0 ============================================================================ <snip> ```
And from the Meson-based Linux build log: ``` <snip> 349/351 ctags/processing-order OK 0.07 s 350/351 utils OK 0.02 s 351/351 sidebar SKIP 0.02 s
Ok: 350 Expected Fail: 0 Fail: 0 Unexpected Pass: 0 Skipped: 1 Timeout: 0
Full log written to /home/runner/work/geany/geany/_build/meson-logs/testlog.txt ```
So GTK initialisation fails (as expected, due to the headless environment) and the proposed change is working as intended with both build systems.
Making display-requiring tests work in CI would call for something like Xvfb, or a more modern alternative based on a headless Wayland/XWayland server, as mentioned in the linked issue. But `gtk_init_check`'s return value should still be properly acted upon in this case.
@kugel- requested changes on this pull request.
/* Not sure if we can really continue without DISPLAY. Fake X display perhaps?
* * This test seems to work, at least. */ - gtk_init_check(&argc, &argv); + if(!gtk_init_check(&argc, &argv)) { + printf("GTK initialisation failed; skipping. Running inside a headless environment?\n");
Please adapt the above comment to the new code. I'm also unsure if printf output is really desirable during test runs.
@6t8k pushed 1 commit.
88cea0a747be1ba06d4981d91702b736f5969e89 `tests/test_sidebar.c`: update comment on what happens in headless envs
@6t8k pushed 1 commit.
2670354a89f3184eefd63048a7bcb9fcbf746675 `tests/test_sidebar.c`: use `g_test_message` instead of `printf`
@6t8k commented on this pull request.
/* Not sure if we can really continue without DISPLAY. Fake X display perhaps?
* * This test seems to work, at least. */ - gtk_init_check(&argc, &argv); + if(!gtk_init_check(&argc, &argv)) { + printf("GTK initialisation failed; skipping. Running inside a headless environment?\n");
Both done, feel free to squash (or let me know).
Note: `printf` is already called [here](https://github.com/geany/geany/blob/2.0.0/tests/test_sidebar.c#L51), but for the purposes of this PR, `g_test_message` is indeed preferable.
@6t8k commented on this pull request.
/* Not sure if we can really continue without DISPLAY. Fake X display perhaps?
* * This test seems to work, at least. */ - gtk_init_check(&argc, &argv); + if(!gtk_init_check(&argc, &argv)) { + printf("GTK initialisation failed; skipping. Running inside a headless environment?\n");
Actually, you might want to consider changing that existing `printf` too, as its output might in principle interfere with TAP. `g_test_message` prefixes the message with `#`, which seems to be the equivalent of a comment, so it should be better practice.
@b4n requested changes on this pull request.
Looks reasonable, at least better than the finger crossing we had before.
However, skipping the test is clearly not ideal, and using xvfb-run/xwfb-run would probably be better, but might be OK as a future improvement.
- if(!gtk_init_check(&argc, &argv)) {
+ g_test_message("%s", "GTK initialisation failed; skipping. Running inside a headless environment?"); + return 77; + }
```suggestion if (! gtk_init_check(&argc, &argv)) { g_test_message("GTK initialization failed; skipping. Running inside a headless environment?"); return 77; } ```
@6t8k pushed 1 commit.
a7b4dbb144c8dfa788a087298a0b47183e92a107 Skip `tests/test_sidebar` if GTK initialisation fails
@6t8k commented on this pull request.
- if(!gtk_init_check(&argc, &argv)) {
+ g_test_message("%s", "GTK initialisation failed; skipping. Running inside a headless environment?"); + return 77; + }
Done & squashed.
If nobody beats me to it, I might propose a change to `.github/workflows/build.yml` sometime during the coming weeks to make use of a virtual framebuffer.
Trying to build geany for void linux [PR](https://github.com/void-linux/void-packages/pull/46783) But test_sidebar also fail for me.
test_sidebar.log: ``` TAP version 13 # random seed: R02S461ac3cd464971e4a48634c6d1556a03 1..3 # Start of sidebar tests a b x ok 1 /sidebar/openfiles_none ** Geany:ERROR:test_sidebar.c:69:do_test_sidebar_openfiles: 'g_strv_equal(expected, (const gchar * const *) data)' should be TRUE ~ x ~/b a b not ok /sidebar/openfiles_path - Geany:ERROR:test_sidebar.c:69:do_test_sidebar_openfiles: 'g_strv_equal(expected, (const gchar * const *) data)' should be TRUE Bail out! ``` Not sure if is same issue, but posting here as related.
Tried to make patches from this and nix packaging, but no luck.
Any suggestions?
PS: Thanks for geany ❤️
@zen0bit approved this pull request.
@6t8k pushed 1 commit.
65d4b667571ea89949ed7bf2eae63ec872da5ac6 CI: run tests inside an Xvfb X server
If nobody beats me to it, I might propose a change to `.github/workflows/build.yml` sometime during the coming weeks to make use of a virtual framebuffer.
See 65d4b66. I opted for an X based solution with `xvfb-run` as that can easily be installed via the official Ubuntu repositories that the workflow already relies on.
The Mingw and macOS builds were out of scope, as the former doesn't run tests and the latter is not defined here (and xvfb-run doesn't work on Darwin).
@zen0bit: Sorry, I have no experience with Void Linux ^^; You might get more exposure by posting this as a separate issue. You could still link it to this PR and the accompanying issue.
@b4n CI no longer skips `test_sidebar`, could you please re-review?
Sorry, I have no experience with Void Linux ^^; You might get more exposure by posting this as a separate issue. You could still link it to this PR and the accompanying issue.
Just to clarify, Void sets `$HOME` to `/tmp` during the build, which results in gtk evaluating `/tmp` to `~`. https://github.com/void-linux/void-packages/blob/1c5c95649891f0ffd58e3a368f6... Setting home to somewhere else during the check (e.g. `$PWD`) does work.
@oreo639 this is a pull request that fixes an issue with a single test, it is not an issue, please do not hijack it. As suggested above please raise a separate issue
I'm explaining that it isn't an issue, sorry.
github-comments@lists.geany.org