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

[Cyril Roelandt]

On 03/07/2014 10:40 AM, Ludovic Courtès wrote:
> Cyril Roelandt <address@hidden> 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.