Skip `tests/test_sidebar` if GTK initialisation fails #3676

6t8k · 2023-11-05T16:34:09Z

This may happen in headless environments.

The return value of gtk_init_check was not examined before; according to ¹, "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.²³
This should work on Meson too⁴, but I have not tested this here.

Based on the example here, I assumed that Autotools would mark test_sidebar as SKIP if all test cases inside are marked as skipped using g_test_skip 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, as proposed here, 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

6t8k · 2023-11-06T21:35:05Z

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.

tests/test_sidebar.c

https://www.geany.org/documentation/releasenotes/2.0/ Upstream now supports building via Meson, which is declared experimental at the time of writing; Autotools is still the default. [1] The correct license for Geany is GPL-2.0-or-later, as indicated by [2]. Add a patch that disables `test_sidebar` because it runs into undefined behavior in headless environments, and crashes at least on headless Darwin. The patch can be removed if [3] is merged (or the issue fixed otherwise). [1] https://github.com/geany/geany/pull/2761/commits [2] https://github.com/geany/geany/tree/2.0.0#license [3] geany/geany#3676

b4n

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.

tests/test_sidebar.c

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 and Meson that the test was skipped.[2][3][4] Fixes geany#3674 [1] https://docs.gtk.org/gtk3/func.init_check.html [2] https://www.gnu.org/software/automake/manual/automake.html#index-Exit-status-77_002c-special-interpretation [3] https://docs.gtk.org/glib/func.test_run.html [4] https://mesonbuild.com/Unit-tests.html#skipped-tests-and-hard-errors

6t8k · 2023-12-18T20:14:31Z

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.

zen0bit · 2024-01-10T08:57:56Z

Trying to build geany for void linux
PR
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 ❤️

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).

6t8k · 2024-01-21T23:31:35Z

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.

6t8k · 2024-01-21T23:51:44Z

@b4n CI no longer skips test_sidebar, could you please re-review?

oreo639 · 2024-02-29T00:17:46Z

Just to clarify regarding the behavior @zen0bit noticed, Void sets $HOME to /tmp during the build, which is why gtk evaluated /tmp to ~.
https://github.com/void-linux/void-packages/blob/1c5c95649891f0ffd58e3a368f6b391db219de24/common/xbps-src/shutils/chroot.sh#L99

elextr · 2024-02-29T00:55:46Z

@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

oreo639 · 2024-02-29T00:57:24Z

I'm trying to clarify that the behavior noticed isn't related to this PR and isn't an issue, sorry for the noise.

6t8k mentioned this pull request Nov 5, 2023

2.0: test_sidebar failing on macOS in headless environment #3674

Open

kugel- requested changes Nov 6, 2023

View reviewed changes

tests/test_sidebar.c Outdated Show resolved Hide resolved

6t8k requested a review from kugel- November 7, 2023 00:17

6t8k mentioned this pull request Nov 10, 2023

geany: 1.38 -> 2.0 NixOS/nixpkgs#265362

Merged

13 tasks

b4n requested changes Dec 12, 2023

View reviewed changes

tests/test_sidebar.c Outdated Show resolved Hide resolved

6t8k force-pushed the test_sidebar_skip_if_gtk_init_fail branch from 2670354 to a7b4dbb Compare December 18, 2023 20:07

6t8k requested a review from b4n December 18, 2023 20:07

zen0bit mentioned this pull request Jan 10, 2024

geany: update to 2.0.0 void-linux/void-packages#46783

Closed

zen0bit approved these changes Jan 15, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Skip `tests/test_sidebar` if GTK initialisation fails #3676

Skip `tests/test_sidebar` if GTK initialisation fails #3676

6t8k commented Nov 5, 2023 •

edited

6t8k commented Nov 6, 2023

b4n left a comment

6t8k commented Dec 18, 2023

zen0bit commented Jan 10, 2024

6t8k commented Jan 21, 2024

6t8k commented Jan 21, 2024

oreo639 commented Feb 29, 2024 •

edited

elextr commented Feb 29, 2024

oreo639 commented Feb 29, 2024 •

edited

Skip tests/test_sidebar if GTK initialisation fails #3676

Are you sure you want to change the base?

Skip tests/test_sidebar if GTK initialisation fails #3676

Conversation

6t8k commented Nov 5, 2023 • edited

Footnotes

6t8k commented Nov 6, 2023

b4n left a comment

Choose a reason for hiding this comment

6t8k commented Dec 18, 2023

zen0bit commented Jan 10, 2024

6t8k commented Jan 21, 2024

6t8k commented Jan 21, 2024

oreo639 commented Feb 29, 2024 • edited

elextr commented Feb 29, 2024

oreo639 commented Feb 29, 2024 • edited

Skip `tests/test_sidebar` if GTK initialisation fails #3676

Skip `tests/test_sidebar` if GTK initialisation fails #3676

6t8k commented Nov 5, 2023 •

edited

oreo639 commented Feb 29, 2024 •

edited

oreo639 commented Feb 29, 2024 •

edited