ui-smoke phase 3: screenshots, writable config mirrors, homing settle

[grandixximo]

Builds on the merged phase 2 (#4054). This phase makes the ui-smoke runs more diagnosable and adds writable-config handling so the GUIs start cleanly headless.

What's here:

• Screenshot on failure, confirm on pass: each GUI smoke test captures a screenshot, attached as a CI artifact, so a failure shows the actual on-screen state instead of just a log.
• gmoccapy against a writable config mirror: gmoccapy writes its preference file next to the config; with the workspace mounted read-only that raises a PermissionError mid-init and pops error dialogs. The test mirrors the sim config to a tmp dir and pre-seeds hide_startup_messsage so the "Important change(s)" modal stays hidden (the same effect as ticking "Don't show this again"). This is the headless-start fix discussed in gmoccapy: modal startup "Important change(s)" dialog blocks headless / non-interactive startup #4072.
• Settle after homing before requesting AUTO: removes a race where the AUTO-mode request arrived before the motion controller finished homing.

Scope note: the touchy window-fit smoke test is intentionally held back on a separate branch, since it depends on the touchy fitting changes in #4131 which are not merged yet. I will send it as a follow-up once #4131 lands.

[grandixximo]

For now the screenshots of the UI are uploaded, and it is up to the reviewers to check, if wanted we could save in source a screenshot to verify against that nothing changed, but then it will be a bit of a bother when anything is supposed to actually change, and for UI with multiple pages, we may want to verify with more than one screenshot at probably more than one resolution? that could take a while, or we can just call it done here, verify UI works with uploaded screenshot is already pretty good,

[grandixximo]

Added the known-good image comparison. On a clean run each GUI's confirm.png is compared (ImageMagick) against a committed reference.png, and both the shot and a highlighted diff.png are uploaded as CI artifacts.

It never fails a test: font/freetype versions differ across distros, so we just record the diff to gather experience first. Even same-machine the 3D preview drifts a few hundred pixels, a handy sanity check.

Regenerate locally on a built RIP tree with tests/ui-smoke/_lib/make-references.sh (sets UI_SMOKE_UPDATE_REFERENCE=1). The four baselines were captured on Debian trixie; other distros will differ, which is what we want to observe. Also rebased onto master.

@BsAtHome This should fit the shape we discussed in the meeting and by email.
@NTULINUX Fedora and Gentoo testing please :-)

[NTULINUX]

Fedora:

=== make-references: axis ===
Running test: /home/fedora/dev/linuxcnc/tests/ui-smoke/axis
Runtest: 1 tests run, 1 successful, 0 failed + 0 expected, 0 skipped, 0 shmem errors
make-references: wrote /home/fedora/dev/linuxcnc/tests/ui-smoke/axis/reference.png
=== make-references: gmoccapy ===
Running test: /home/fedora/dev/linuxcnc/tests/ui-smoke/gmoccapy
Runtest: 1 tests run, 1 successful, 0 failed + 0 expected, 0 skipped, 0 shmem errors
make-references: wrote /home/fedora/dev/linuxcnc/tests/ui-smoke/gmoccapy/reference.png
=== make-references: touchy ===
Running test: /home/fedora/dev/linuxcnc/tests/ui-smoke/touchy
Runtest: 1 tests run, 1 successful, 0 failed + 0 expected, 0 skipped, 0 shmem errors
make-references: wrote /home/fedora/dev/linuxcnc/tests/ui-smoke/touchy/reference.png
=== make-references: qtdragon ===
Running test: /home/fedora/dev/linuxcnc/tests/ui-smoke/qtdragon
Runtest: 1 tests run, 1 successful, 0 failed + 0 expected, 0 skipped, 0 shmem errors
make-references: wrote /home/fedora/dev/linuxcnc/tests/ui-smoke/qtdragon/reference.png

Have yet to test on Gentoo, still battling networking issues.

[BsAtHome]

I don't really understand the diff images. They have a gray tint and reveal the actual image. I expected them to be nearly or completely black. A (biased) difference is not interesting for which direction it goes. The absolute difference would be more telling. Essentially: for each pixel and color |b - a|.

[grandixximo]

Anything that differs is bright red, I have not looked at the latest ones yet, if they are mostly greyish means no changes

[grandixximo]

Tomorrow I will do some builds, and have different screenshots so we can pick a winner, at least now it seems to be working alright, the final look it's just a matter of taste 😉

[hdiethelm]

Hmm, this makes no sense, checking if we are root and then if RUNTESTS_NOSUDO not doing it.
Here, you can only be root if you use sudo runtests right? However, sudo runtests will always fail.
I would just check for "${RUNTESTS_NOSUDO:-0}" != "1" and then do sudo sysctl

Did you ever test if you get a crashdump if there is a segfault?

[hdiethelm]

I missed one, you have to add gdb here to:
https://github.com/grandixximo/linuxcnc/blob/c4de595efdb60e9c3c65bf315289b32a4b39d63c/.github/workflows/ci.yml#L225-L229

Suggestion:
Try to get a test failing with a crashdump locally (Like adding a random segfault somewhere) and then push the test commit to see if the CI behaves as it should. I tried adding a segfault in rtapi_app locally but the test just freezes forever.
When creating tests, it is always good to test what happens if they fail.

The same for the image tests, break a gui on purpose and see what happens. Having tests pass is good but having them fail when there is an issue is even better... ;-)

Otherwise, the concept looks really good! Sadly I don't have the time right now to do a proper review.

[grandixximo]

@BsAtHome
with the current method this is how you would see if touchy was smaller than it is supposed to be

this is the absdiff method

this is gmoccapy current method the icon and time differ

this is absdiff method

axis current method the preview differs, the fonts I fuzzed a bit cause they were always coming up as positive all red with my reference picture, this is so when running locally you don't get all red

this is axis with absdiff with slightly different fonts, and the plot

this is with added fuzz on the absdiff method, only see plot difference

Both methods express the same thing in a different way, now that you see them next to each other, do you have a preference?

[grandixximo]

@hdiethelm
Thanks, good catches both.

Moved gdb as suggested.

The crashdump did work initially and was tested, but I did not run it again after recent permissions changes, my bad, I did testing again results are as follows

Failure path: I injected a real segfault into a GUI and pushed it through CI to watch the actual runners. Good news: it fails fast (the test goes red in seconds, no hang), the Python faulthandler traceback in linuxcnc.err names the crash frame, the watchdog catches it with UI_SMOKE_FAIL: linuxcnc exited before the driver finished, and the failure screenshots still upload. Result line was 291 tests run, 290 successful, 1 failed.

Your freeze was most likely the rtapi_app path: when the RT loader dies, linuxcnc hangs bringing HAL up rather than the GUI process exiting, so the launcher-PID watch never fires and only the outer timeout ends it, slowly. The GUI-crash path, which is what the crash dump is for, behaves.

Testing it again also exposed and fixed two crash-dump bugs: it could pick up a stale root-owned core from /tmp (gdb then just printed Permission denied), and a non-root run produces no core at all since we will not sudo to set core_pattern. Fixed: only use a readable core from this run, otherwise say so and lean on the Python traceback. The native gdb backtrace stays a best-effort extra for when the suite runs as root.

One note from the CI run: because the runners execute the suite non-root, the native backtrace path correctly no-ops and prints no readable core dump; see the Python traceback. The Python traceback is the crash signal CI actually delivers, and it does.

[BsAtHome]

Actually, I like both image diffs (without fuzz) when viewed together. Do we need to choose or can we have them both?

[grandixximo]

Shouldn't be a problem to have both, can even have multiples fuzz levels, the text rendering I think goes away around 20% fuzz with no fuzz the text will always be almost all red, so it will be harder to spot if any labels were changed

[grandixximo]

Instead of having them as separate diffs, I could collage everything in a single huge image, with tiles, where each tite is titled sensibly, like original and build next to each other on top, then more tiles for the "a-b diff" and the "red marks the diff" at 0 and 20% fuzz, 6 tiles, 2x3, I think that should give a proper overview, or am I overdoing it?

[BsAtHome]

Combining them in one image will make the lighter parts hide the darker parts (eye accommodation problem). Therefore, having them separate is best.

The small text rendering differences are not bright red as long as they are just rendering differences. A real text change would make that much more visible. I think it be good to see small rendering changes because it says something about stability. Therefore, no fuzz would be fine.

If we are to look at more differences, then we probably should look at differences between distributions. However, this is not a ui-smoke test problem. This would more be like a "let me see what I broke yesterday" matrix comparison. You'd need to collect images from all distros/versions and then be able to do a local RIP comparison. A different project ;-)

[hdiethelm]

@hdiethelm Thanks, good catches both.

Moved gdb as suggested.

Now gdb is installed with apt-get build-dep right? This just means everybody that builds linuxcnc will have gdb installed. But gdb is anyway needed to run the test, so fine in my opinion. An alternative would be to add it to the two places in the CI. If anyone has an issue with this, I can send you a patch to install it CI only.

The crashdump did work initially and was tested, but I did not run it again after recent permissions changes, my bad, I did testing again results are as follows

Failure path: I injected a real segfault into a GUI and pushed it through CI to watch the actual runners. Good news: it fails fast (the test goes red in seconds, no hang), the Python faulthandler traceback in linuxcnc.err names the crash frame, the watchdog catches it with UI_SMOKE_FAIL: linuxcnc exited before the driver finished, and the failure screenshots still upload. Result line was 291 tests run, 290 successful, 1 failed.

Your freeze was most likely the rtapi_app path: when the RT loader dies, linuxcnc hangs bringing HAL up rather than the GUI process exiting, so the launcher-PID watch never fires and only the outer timeout ends it, slowly. The GUI-crash path, which is what the crash dump is for, behaves.

Possibly I was not patient enough.

Testing it again also exposed and fixed two crash-dump bugs: it could pick up a stale root-owned core from /tmp (gdb then just printed Permission denied), and a non-root run produces no core at all since we will not sudo to set core_pattern. Fixed: only use a readable core from this run, otherwise say so and lean on the Python traceback. The native gdb backtrace stays a best-effort extra for when the suite runs as root.

One note from the CI run: because the runners execute the suite non-root, the native backtrace path correctly no-ops and prints no readable core dump; see the Python traceback. The Python traceback is the crash signal CI actually delivers, and it does.

You can use sudo in CI, it has nopasswd set. Changing:
if [ "$(id -u)" = 0 ] && [ "${RUNTESTS_NOSUDO:-0}" != "1" ]; then
to
if [ "${RUNTESTS_NOSUDO:-0}" != "1" ]; then
should be fine for users as much as I understood the tests readme and it should work in CI.
The [ "$(id -u)" = 0 ] check makes no sense, tests can not be run as root anyway because linuxcnc checks for root and refuses to start if you start it as root. Or did you manage to do sudo runtests ...?

[grandixximo]

You're right, the id -u = 0 check was pointless since the suite never runs as root, so core_pattern was never armed and the native backtrace never ran. Dropped the check and set it via sudo, still gated by RUNTESTS_NOSUDO for runtests -u.

I tested the full path on a real host with passwordless sudo: a GUI segfault now writes a core into our dir and the crashdump prints the gdb backtrace (the crash frame, ffi_call, g_main_loop_run, gtk_main) plus all-threads. So CI should now produce a native backtrace instead of the "no readable core dump" fallback. (In a docker container /proc/sys is read-only so the arm no-ops and it falls back to the Python traceback, which is fine.)

One open question for you: crashdump_arm runs on every test to be ready for a crash, so it leaves core_pattern pointed at our (then-deleted) temp dir after the suite. On CI that's ephemeral and a non-issue, but a local dev running the full suite without -u would have their core_pattern altered until reboot. Worth a second sudo sysctl to save it in arm and restore it in report, or do you think that's overkill?

[hdiethelm]

You're right, the id -u = 0 check was pointless since the suite never runs as root, so core_pattern was never armed and the native backtrace never ran. Dropped the check and set it via sudo, still gated by RUNTESTS_NOSUDO for runtests -u.

I tested the full path on a real host with passwordless sudo: a GUI segfault now writes a core into our dir and the crashdump prints the gdb backtrace (the crash frame, ffi_call, g_main_loop_run, gtk_main) plus all-threads. So CI should now produce a native backtrace instead of the "no readable core dump" fallback. (In a docker container /proc/sys is read-only so the arm no-ops and it falls back to the Python traceback, which is fine.)

One open question for you: crashdump_arm runs on every test to be ready for a crash, so it leaves core_pattern pointed at our (then-deleted) temp dir after the suite. On CI that's ephemeral and a non-issue, but a local dev running the full suite without -u would have their core_pattern altered until reboot. Worth a second sudo sysctl to save it in arm and restore it in report, or do you think that's overkill?

The crashdump_arm might not by nice for some people, not finding their crasdumps any more. Probably more an edgecase but who knows. At least for me, I would never run the tests allowing sudo except in a VM.

An other option: Add a new option in scripts/runtests.in that enables crashdumps like d) export ENABLE_CRASHDUMPS=1 ;; and add -d in the three places in ci.yml: runtests -p -> runtests -p -d

Looking at your crashdump script: Instead of having it exclusive for your UI tests: Why not enable it globally for all tests in runtest.in? It looks useful for everything. It might be possible to add it just in the run_test() function.

[grandixximo]

Good call, done. Crash dumps are now opt-in behind runtests -d (ENABLE_CRASHDUMPS=1), off by default, and I pass -d in the three CI invocations. Without -d, crashdump_arm/report no-op: no sudo, no core_pattern change, so a normal local run leaves the system untouched. The Python faulthandler traceback is independent of all this and stays on, so you still get a crash signal without -d.

On enabling it globally for every test in run_test(): I agree it is useful beyond the GUI tests, but I would rather do that as a follow-up PR to keep this one focused. There is also one thing to generalize first: the native backtrace currently assumes the crashing process is python3 (gdb $(command -v python3) $core) and the readable Python traceback comes from PYTHONFAULTHANDLER, which the GUI launch env sets. Most non-GUI tests are C (halcmd, rtapi_app, comps), so a global version needs to detect the crashing binary rather than assume python3. Happy to take that on separately once this lands.