Merge ~ahasenack/ubuntu/+source/pyzmq:eoan-pyzmq-test-poll-timeout-bump into ~ahasenack/ubuntu/+source/pyzmq:master

[Andreas Hasenack]

This package isn't imported into git-ubuntu, so I imported the debian package using gbp (that's the "master" branch) and branched off with my changes. I'm proposing my changes against this master branch.

pyzmq is failing DEP8 in autopkgtest.ubuntu.com, specifically the test_curve_user_id test. This also runs during package build, where it passes, but not as DEP8, and not in the autopkgtest.u.c infrastructure. The history is quite bad also: http://autopkgtest.ubuntu.com/packages/pyzmq/eoan/amd64

I couldn't reproduce it reliably enough in a local vm, with or without proposed, to troubleshoot. What I came up with here is to just bump the poll timeout. I never saw the error locally with that bump, but, as I said, not reliable. I might just not have tried long enough.

[Andreas Hasenack (ahasenack)]

Here is a ppa with a test build: https://launchpad.net/~ahasenack/+archive/ubuntu/zeromq3-merge

[Bryce Harrington (bryce)]

The LP generated debdiff appears to be against a test build, so I constructed one against what's in eoan: http://paste.ubuntu.com/p/GBS8nD68Zg/

- Verified upstream-maintainer is updated
- Verified patch is correctly listed in debian/patches/series
- Verified debdiff targets correct release codename

Bumping the timeout seems safe and I don't have any strong arguments *against* doing so, and the packaging changes LGTM, so I feel comfortable giving a provisional +1 approval.

However, my guess is this will only partially solve the actual issue, so further thinking below:

The test cases in test_auth.py appear to be doing a bunch of connection tests against localhost. One should think these connections would be super-speedy. If they don't connect in under a second, then while giving it a larger timeout should reduce the frequency of the failures, presumably there'll still be cases where it will fail. I wonder if maybe a) use an even larger timeout??, b) skip the test if upstream already runs it "enough", or c) investigate why this particular test fails.

Spot checking the failure logs, the failures due indeed all seem to be occurring with the test_curve_user_id test case, although sometimes are under python2 and sometimes python3 so its an intermittent failure. The proposed patch would increase the timeout for the can_connect() routine, but there are a number of test cases in test_auth.py which call this routine so why is it this one test where that call fails? If the root cause here was simply machine sluggishness I would expect the failures to occur semi-randomly across a variety of callers; the test case consistency makes me wonder if there is a legit bug.

The most notable thing test_curve_user_id() is doing differently from the other test cases is setting up CURVE-based authentication certs. I wouldn't think the mere tacking on of some certs would cause this much delay, and wonder if something is attempting a network lookup somewhere under the hood. The failure errors in the log file aren't detailed enough to explore fully, but digging through the code I got as far as zmq/backend/cffi/_poll.py:zmq_poll() which has a 'while True' loop that might be where things are getting stuck; I couldn't guess why, but maybe tracing into this (or adding debug print's) would verify it's hanging in this routine, and at what part.

If bumping the timeout doesn't eliminate the test failures entirely, it would probably be a good idea to ensure there's a bug report for this for further investigation. If it ends up being more than just a sluggish machine, then this could be worth raising with upstream - particularly if a reproducible test case can be found.

review: Approve

[Andreas Hasenack (ahasenack)]

If this still fails, then I'm inclined to stop this test from running and ask for help from upstream indeed, as using the ubuntu archive to troubleshoot this is far from optimal. If only it happened more reliably locally.

[Andreas Hasenack (ahasenack)]

Debian uploaded a -3 release, and we synced that automatically. It's failing again in excuses, and in ppc64el too now.

[Andreas Hasenack (ahasenack)]

I rebased

[Christian Ehrhardt  (paelzer)]

The status still is WIP, did the "I rebased" mean this is fully ready for re-review?

[Andreas Hasenack (ahasenack)]

Not yet, I was trying a bit harder to reproduce the failures locally. I'll switch the status back to needs review when it's ready (even if there are no changes)

[Andreas Hasenack (ahasenack)]

Got bileto working, and tests passed there:

https://bileto.ubuntu.com/excuses/3763/eoan.html

Ticket: https://bileto.ubuntu.com/#/ticket/3763

[Andreas Hasenack (ahasenack)]

But didn't try with the unchanged timeout, so I still don't know if bileto is a good enough replica of the real archive testing environment.

[Christian Ehrhardt  (paelzer)]

The change itself looks fine, if you can confirm the issue triggers with the unchanged timeout I think you are good to push this to Eoan and to Debian to get rid of the delta at some point.

OTOH Debian might suggest doing the same upstream instead, anyway up to you and probably fine for now.

review: Approve

[Christian Ehrhardt  (paelzer)]

Since the target is in your repo I can't add back (and could only consume) canonical-server - please add it back yourself.

[Andreas Hasenack (ahasenack)]

Reverting the timeout bump triggered a failure in bileto in the same curve_id test:

https://bileto.ubuntu.com/excuses/3763/eoan.html

\o/

I'm uploading with the timeout bump then.

[Andreas Hasenack (ahasenack)]

This is not in g-u, so no tag to push, just a regular upload:
$ dput ubuntu ../pyzmq_17.1.2-3ubuntu1_source.changes
Checking signature on .changes
gpg: ../pyzmq_17.1.2-3ubuntu1_source.changes: Valid signature from AC983EB5BF6BCBA9
Checking signature on .dsc
gpg: ../pyzmq_17.1.2-3ubuntu1.dsc: Valid signature from AC983EB5BF6BCBA9
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading pyzmq_17.1.2-3ubuntu1.dsc: done.
  Uploading pyzmq_17.1.2-3ubuntu1.debian.tar.xz: done.
  Uploading pyzmq_17.1.2-3ubuntu1_source.buildinfo: done.
  Uploading pyzmq_17.1.2-3ubuntu1_source.changes: done.
Successfully uploaded packages.

That was fa3aeae065c912676f07abd8a50bcc6e5bf0dc9a

diff --git a/debian/changelog b/debian/changelog
index 33dae4a..54e35df 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+pyzmq (17.1.2-3ubuntu1) eoan; urgency=medium
+
+  * d/p/bump-poll-timeout-auth-test.patch: increase poll timeout to
+    two seconds to avoid test failure on slow machines.
+
+ -- Andreas Hasenack <[e-mail]>  Tue, 23 Jul 2019 11:56:58 -0300
+
 pyzmq (17.1.2-3) unstable; urgency=medium
 
   * Team upload.
diff --git a/debian/control b/debian/control
index 9ae5f9b..d3451e8 100644
--- a/debian/control
+++ b/debian/control
@@ -1,7 +1,8 @@
 Source: pyzmq
 Section: python
 Priority: optional
-Maintainer: Debian Python Modules Team <[e-mail]>
+Maintainer: Ubuntu Developers <[e-mail]>
+XSBC-Original-Maintainer: Debian Python Modules Team <[e-mail]>
 Uploaders: Julian Taylor <[e-mail]>,
            Vincent Bernat <[e-mail]>,
            Laszlo Boszormenyi (GCS) <[e-mail]>
diff --git a/debian/patches/bump-poll-timeout-auth-test.patch b/debian/patches/bump-poll-timeout-auth-test.patch
new file mode 100644
index 0000000..655b7b3
--- /dev/null
+++ b/debian/patches/bump-poll-timeout-auth-test.patch
@@ -0,0 +1,24 @@
+Description: Bump poll timeout to avoid test error on slower machines
+ Not forwarded yet because this is hard to reproduce outside of the Ubuntu
+ autopkgtest infrastructure and we first want to see if it helps our case.
+Author: Andreas Hasenack <[e-mail]>
+Forwarded: no
+Last-Update: 2019-07-18
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+diff --git a/zmq/tests/test_auth.py b/zmq/tests/test_auth.py
+index 003f171..53596ca 100644
+--- a/zmq/tests/test_auth.py
++++ b/zmq/tests/test_auth.py
+@@ -102,9 +102,9 @@ class TestThreadAuthentication(BaseAuthTestCase):
+         port = server.bind_to_random_port(iface)
+         client.connect("%s:%i" % (iface, port))
+         msg = [b"Hello World"]
+-        if server.poll(1000, zmq.POLLOUT):
++        if server.poll(2000, zmq.POLLOUT):
+             server.send_multipart(msg)
+-        if client.poll(1000):
++        if client.poll(2000):
+             rcvd_msg = client.recv_multipart()
+             self.assertEqual(rcvd_msg, msg)
+             result = True
diff --git a/debian/patches/series b/debian/patches/series
index 7931f9e..9756f48 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -2,3 +2,4 @@ noncopysend-test.patch
 cffi-fix.patch
 skip_large_send
 fix_monitor_test.patch
+bump-poll-timeout-auth-test.patch