[bug#55954] [PATCH] gnu: public-inbox: Fixes to allow the testsuite to run

[Thiago Jung Bauermann via Guix-patches via <guix-patches@gnu.org>]

Hello Ludo,

Thank you for your review!

Ludovic Courtès <ludo@gnu.org> writes:

> Hi,
>
> Thiago Jung Bauermann <bauermann@kolabnow.com> skribis:
>
>> This patch makes the public-inbox testsuite pass. Some tests are skipped,
>> so the test coverage could likely be increased with more massaging.
>>
>> Perhaps the most significant change is using tini to run the testsuite so
>> that the testsuite's sub-processes are reaped. The ‘check’ phase is based on
>> the one from the mutter package. Thanks to Maxim Cournoyer for pointing out
>> this solution.
>>
>> * gnu/packages/patches/public-inbox-fix-spawn-test.patch: New file.
>> * gnu/local.mk (dist_patch_DATA): Add new patch.
>> * gnu/packages/mail.scm (public-inbox)[source]: Add new patch.
>> [arguments]<#:tests?>: Remove argument.
>> <#:imported-modules>: Add argument.
>> <#:modules>: Likewise.
>> <#:phases>{qualify-paths}: Substitute path for ‘/bin/cp’.
>> {pre-check}: Don't skip httpd-unix.t test.  Remove unnecessary path
>> substitutions for “env” and “/bin/sh”.
>> {check}: Replace with custom version that launches the tests under tini.
>> [native-inputs]: Add tini.
>
> Applied, thanks!

I guess you had second thoughts. :-)

>> +         (replace 'check
>> +           (lambda* (#:key target
>> +                     (tests? (not target)) (test-flags '())
>> +                     #:allow-other-keys)
>> +             (if tests?
>> +                 (match (primitive-fork)
>> +                   (0                     ;child process
>> +                    ;; lei tests build UNIX domain sockets in the temporary
>> +                    ;; directory, but the path of those sockets can be at most
>> +                    ;; 108 chars and Guix' default value for the variables
>> +                    ;; below already use 47 chars. Use the shortest temporary
>> +                    ;; path possible to avoid hitting the limit.
>> +                    (setenv "TEMP" "/tmp")
>> +                    (setenv "TEMPDIR" "/tmp")
>> +                    (setenv "TMP" "/tmp")
>> +                    (setenv "TMPDIR" "/tmp")
>> +
>> +                    ;; Use tini so that signals are properly handled and
>> +                    ;; doubly-forked processes get reaped; otherwise,
>> +                    ;; lei-daemon is kept as a zombie and the testsuite
>> +                    ;; fails thinking that it didn't quit as it should.
>> +                    (set-child-subreaper!)
>> +                    (apply execlp "tini" "--"
>> +                           "make" "check" test-flags))
>> +                   (pid
>> +                    (match (waitpid pid)
>> +                      ((_ . status)
>> +                       (unless (zero? status)
>> +                         (error "`make check' exited with status" status))))))
>
> It’s a bummer that we have to do all this.  Would this work:
>
>   (replace 'check
>     (lambda _
>       (sigaction SIGINT SIG_DFL)
>       (sigaction SIGTERM SIG_DFL)
>       (sigaction SIGCHLD (lambda _ (waitpid WAIT_ANY WNOHANG)))
>
>       ;; Ugly hack to make sure signal handler asyncs get a chance
>       ;; to run while we’re in ‘waitpid’ during the “make check”
>       ;; invocation.
>       (sigaction SIGALRM (lambda _ (alarm 1)))
>       (alarm 1)))
>
> ?

I assume there should be a "make check" in there somewhere. :-)

Unfortunately, it didn't work. This is the check phase I tried:

--8<---------------cut here---------------start------------->8---
  (replace 'check
    (lambda _
      (setenv "TEMP" "/tmp")
      (setenv "TEMPDIR" "/tmp")
      (setenv "TMP" "/tmp")
      (setenv "TMPDIR" "/tmp")

      (sigaction SIGINT SIG_DFL)
      (sigaction SIGTERM SIG_DFL)
      (sigaction SIGCHLD (lambda _ (waitpid WAIT_ANY WNOHANG)))

      ;; Ugly hack to make sure signal handler asyncs get a chance
      ;; to run while we’re in ‘waitpid’ during the “make check”
      ;; invocation.
      (sigaction SIGALRM (lambda _ (alarm 1)))
      (alarm 1)
      (unless (zero? (status:exit-val (system* "make" "check")))
        (error "`make check' exited with error"))))
--8<---------------cut here---------------end--------------->8---

And this was the end of the output of the phase:

--8<---------------cut here---------------start------------->8---
    ⋮
Bailout called.  Further testing stopped:  daemon still running (PID:5282)
FAILED--Further testing stopped: daemon still running (PID:5282)
make: *** [Makefile:4141: check-each] Error 255
error: in phase 'check': uncaught exception:
misc-error #f "~A" ("`make check' exited with error") #f
phase `check' failed after 27.7 seconds
--8<---------------cut here---------------end--------------->8---

“daemon still running” means that lei's daemon process wasn't reaped.

> If not, the solution you propose LGTM.

Thanks. Hopefully this can be fixed in Guix's build machinery itself in
the core-updates branch and all the packages that are using this hack
can drop it then.

-- 
Thanks
Thiago