Re: [PATCH] build: Add a scheme custom test driver using SRFI-64.

[Mathieu Lirzin]

Hi,

address@hidden (Ludovic Courtès) writes:

> Mathieu Lirzin <address@hidden> skribis:
>
>> While reading the Automake manual I have discovered that it provides an
>> API for custom test drivers, so I've tried to implement something for
>> the scheme API for test suites (SRFI-64) which Guix is using.  The
>> result provides support for multiple test cases in a single file, and
>> allow ‘make check’ display the results individually:
>>
>>   PASS: tests/base32.scm - bytevector->base32-string
>>   PASS: tests/base32.scm - base32-string->bytevector
>>   PASS: tests/base32.scm - nix-base32-string->bytevector
>>   PASS: tests/base32.scm - &invalid-base32-character
>>   SKIP: tests/base32.scm - sha256 & bytevector->nix-base32-string
>>   PASS: tests/base64.scm - empty string
>
> Awesome!  Are the “inner” tests displayed by default?  Or is there some
> environment variable to control that?

Yes, each test case is displayed by default.

> I’d prefer the default to display only file names, as is currently the
> case (it’s more concise.)

IMO This would be reasonable to have an option for making the output more
concise since there are 563 test cases in Guix.  this is not part of the
test driver API specification, but it does not seem too hard to add an
additional option to the test driver script like what the TAP driver is
doing:

  
https://www.gnu.org/software/automake/manual/automake.html#Use-TAP-with-the-Automake-test-harness

I think ‘--brief’ could be a good name for this option.  Default
verbosity would specified by adding “AM_SCM_LOG_DRIVER= --brief=yes” in
Makefile.am, then the user can overide that with:

  make check -j4 TESTS=tests/rocket-science.scm SCM_LOG_DRIVER="--brief=no"

>> Finally the ‘.log’ files are no longer fragmented between
>> “${top_srcdir}” and “tests/”.  If something went wrong (FAIL, SKIP,
>> XPASS) then the entire log file is copied in ‘test-suite.log’ so no need
>> to ask for an additional file when someone is reporting some ‘make
>> check’ error.
>
> Nice.  “Running the Test Suite” needs to be updated accordingly.

Thanks for reminding me this.  ;)

>> One caveat is that ‘tests/cpio.scm‘ is now failing.
>
> Why is that?  Does it relate to this change?

I didn't try to debug the problem but it has appeared when I started
messing with redirection of the output/error ports.  Here is the failure
output.

--8<---------------cut here---------------start------------->8---
test-name: bit-identical to GNU cpio's output
location: /home/mthl/src/gnu/guix/tests/cpio.scm:49
source:
+ (test-assert
+   "bit-identical to GNU cpio's output"
+   (call-with-temporary-output-file
+     (lambda (link _)
+       (delete-file link)
+       (symlink "chbouib" link)
+       (let ((files (cons* "/"
+                           (canonicalize-path
+                             (dirname (search-path %load-path "guix.scm")))
+                           link
+                           (map (compose
+                                  canonicalize-path
+                                  (cut search-path %load-path <>))
+                                '("guix.scm"
+                                  "guix/build/syscalls.scm"
+                                  "guix/packages.scm")))))
+         (call-with-temporary-output-file
+           (lambda (ref-file _)
+             (let ((pipe (open-pipe*
+                           OPEN_WRITE
+                           %cpio-program
+                           "-o"
+                           "-O"
+                           ref-file
+                           "-H"
+                           "newc"
+                           "--null")))
+               (for-each
+                 (lambda (file) (format pipe "~a\x00" file))
+                 files)
+               (and (zero? (close-pipe pipe))
+                    (call-with-temporary-output-file
+                      (lambda (file port)
+                        (write-cpio-archive files port)
+                        (close-port port)
+                        (or (file=? ref-file file)
+                            (throw 'cpio-archives-differ
+                                   files
+                                   ref-file
+                                   file
+                                   (stat:size (stat ref-file))
+                                   (stat:size (stat file))))))))))))))
value: #f
result: FAIL
--8<---------------cut here---------------end--------------->8---

>> Since this script is not intented to be exclusively used by Guix, I have
>> used a generic copyright notice.  I guess Guix is the best place to
>> challenge and improve it, but IMO it will be better hosted elsewhere
>> like in Gnulib.  Opinions?
>
> I think we could start using it and testing it for a while in Guix, and
> eventually submit it for inclusion in Gnulib once we are more confident.

So you recommend to add ”this file is part of GNU Guix” and use “GNU
Guix” instead of “this program” for now?

>> From 977e0586a6689df204fd9b522f8508ec318180c0 Mon Sep 17 00:00:00 2001
>> From: Mathieu Lirzin <address@hidden>
>> Date: Mon, 26 Oct 2015 23:47:24 +0100
>> Subject: [PATCH] build: Add a scheme custom test driver using SRFI-64.
>>
>> This provides support for multiple scheme test cases in a unique file and
>> fixes the fragmentation of '.log' files.
>>
>> * build-aux/test-driver.scm: New file.
>> * Makefile.am (SCM_LOG_DRIVER): New variable.
>>   (SCM_LOG_COMPILER, AM_SCM_LOG_FLAGS): Delete variables.
>>   (AM_TESTS_ENVIRONMENT): Set GUILE_AUTO_COMPILE to 0.
>> * tests/base32.scm, tests/build-utils.scm, tests/builders.scm,
>>   tests/challenge.scm, tests/containers.scm, tests/cpan.scm,
>>   tests/cpio.scm, tests/cran.scm, tests/derivations.scm, tests/elpa.scm,
>>   tests/file-systems.scm, tests/gem.scm, tests/gexp.scm,
>>   tests/graph.scm, tests/gremlin.scm, tests/hackage.scm, tests/hash.scm,
>>   tests/lint.scm, tests/monads.scm, tests/nar.scm, tests/packages.scm,
>>   tests/pk-crypto.scm, tests/pki.scm, tests/profiles.scm,
>>   tests/publish.scm, tests/pypi.scm, tests/records.scm,
>>   tests/scripts.scm, tests/services.scm, tests/sets.scm, tests/size.scm,
>>   tests/snix.scm, tests/store.scm, tests/substitute.scm,
>>   tests/syscalls.scm, tests/ui.scm, tests/union.scm, tests/utils.scm:
>>   Don't exit at the end of each test.
>
> AIUI each tests/foo.scm file is still run with its own Guile instance,
> in ./test-env, right?  (This is important.  It also means tests can run
> in parallel.)
>
> If this is OK, it LGTM.

I think it is OK :)

> However, I’m unsure if we should push it now, or after the release.  On
> one hand, I’d rather avoid potentially disrupting changes like this
> now.  On the other hand, since it makes it easier (and different) to
> report test failures, it’s nice.
>
> How confident are you?  :-)

IMHO we should wait after the next release in order to make this test
driver more bullet proof.

Thanks for your comments!

--
Mathieu Lirzin