Re: [PATCH 3/5] gnu: Enable tests in Python 3.

[Cyril Roelandt <tipecaml@gmail.com>]

On 03/07/2014 10:40 AM, Ludovic Courtès wrote:
> Cyril Roelandt <tipecaml@gmail.com> skribis:
>
>> * gnu/packages/python.scm: enable tests for Python 3
>> * gnu/packages/python-fix-tests.patch: New file.
>> * gnu/packages/gnu-sysem.am (dist_patch_DATA): add it.
>
> Typo here.
>

Fixed.

>> diff --git a/gnu/packages/patches/python-fix-tests.patch b/gnu/packages/patches/python-fix-tests.patch
>> new file mode 100644
>> index 0000000..23ef17f
>> --- /dev/null
>> +++ b/gnu/packages/patches/python-fix-tests.patch
>> @@ -0,0 +1,62 @@
>> +--- Lib/test/test_shutil.py	2014-03-01 04:56:37.768311000 +0100
>> ++++ Lib/test/test_shutil.py	2014-03-01 03:02:36.088311000 +0100
>> +@@ -1053,7 +1053,6 @@
>> +         self.assertRaises(ValueError, make_archive, base_name, 'xxx')
>> +
>
> Could you add a couple of lines of summary and (possibly) and a link to
> the upstream report/discussion (or a statement on the upstream status)?

Done.

>
>> +     @requires_zlib
>> +-    @unittest.skipIf(True, "getgrgid(0)[0] raises a KeyError on Guix")
>
> It seems that this part of the patch is reversed, no?
>

Oops. Fixed.

>> +--- Lib/test/test_posixpath.py	2014-03-01 05:46:56.984311000 +0100
>> ++++ Lib/test/test_posixpath.py	2014-03-01 06:20:50.704311000 +0100
>> +@@ -319,7 +319,11 @@
>> +                 del env['HOME']
>> +                 home = pwd.getpwuid(os.getuid()).pw_dir
>> +                 # $HOME can end with a trailing /, so strip it (see #17809)
>> +-                self.assertEqual(posixpath.expanduser("~"), home.rstrip("/"))
>> ++		# The Guix builders have '/' as a home directory, so
>> ++		# home.rstrip("/") will be an empty string and the test will
>> ++		# fail. Let's just disable it since it does not really make
>> ++		# sense with such a bizarre setup.
>> ++                # self.assertEqual(posixpath.expanduser("~"), home.rstrip("/"))
>
> I see that the recipe’s ‘pre-check’ phase does:
>
>    (setenv "HOME" (getcwd))
>
> so $HOME should actually be /tmp/nix-build-xxx, not just /, no?
>

Yes, but I think it uses getpwent() or something that does not use $HOME 
to retrieve the home directory in this case.

>> +--- Lib/test/test_socket.py	2014-03-02 22:14:12.264311000 +0100
>> ++++ Lib/test/test_socket.py	2014-03-03 01:12:21.360311000 +0100
>> +@@ -819,6 +819,8 @@
>> +             self.assertRaises(OverflowError, socket.htonl, k)
>> +             self.assertRaises(OverflowError, socket.htons, k)
>> +
>> ++    @unittest.skipUnless(os.path.exists("/etc/services"),
>> ++                         "getservbyname uses /etc/services, which is not in the chroot")
>
> (I think that should go upstream.)
>
>> ++    @unittest.skipUnless(os.path.exists("/etc/gai.conf"),
>> ++                         "getaddrinfo uses /etc/gai.conf, which is not in the chroot")
>
> I don’t think gai.conf is the problem.  Instead, the problem is that one
> cannot rely on name lookups in the build environment at all, and even in
> general.
>
> So this patch may be OK for ourselves (except the comment), but upstream
> Python should really just gracefully skip the test if name lookup
> fails.
>
>> +    (arguments
>> +     (let ((args `(#:modules ((guix build gnu-build-system)
>> +                              (guix build utils)
>> +                              (srfi srfi-1)
>> +                              (srfi srfi-26))
>> +                   ,@(package-arguments python-2))))
>> +       (substitute-keyword-arguments args
>> +         ((#:tests? _)
>> +          `(list #t)))))
>
> #t instead of `(list #t) should do the job.  :-)
>

Fixed.


Cyril.