Title: test_socket.CANTest is broken at HEAD on master
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.9
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: karlding
Priority: normal Keywords: patch

Created on 2020-04-16 03:25 by karlding, last changed 2020-05-22 07:47 by karlding.

Pull Requests
URL Status Linked Edit
PR 19548 open karlding, 2020-04-16 03:27
Messages (2)
msg366576 - (view) Author: Karl Ding (karlding) * Date: 2020-04-16 03:25
While working on, I was trying to run the SocketCAN tests to ensure that my changes weren't causing any regressions. However, I was seeing test failures at HEAD.

I'm running the tests like so:

# Kernel version
uname -r
# 5.4.23-1-MANJARO

# Load SocketCAN vcan kernel module
sudo modprobe vcan

# Start and set up a virtual SocketCAN interface
sudo ip link add dev vcan0 type vcan
sudo ip link set up vcan0

# Run the socket tests using locally built python
./python -m unittest -v test.test_socket.CANTest

After bisecting, I discovered that the test started failing back in 2017, with the introduction of the ISOTP protocol.

Here, the address family (the second tuple item) is explicitly removed:

diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c
index bf8d19fe2f..beadecfad5 100644
--- a/Modules/socketmodule.c
+++ b/Modules/socketmodule.c
@@ -1373,9 +1373,22 @@ makesockaddr(SOCKET_T sockfd, struct sockaddr *addr, size_t addrlen, int proto)
                 ifname = ifr.ifr_name;

-        return Py_BuildValue("O&h", PyUnicode_DecodeFSDefault,
-                                    ifname,
-                                    a->can_family);
+        switch (proto) {
+#ifdef CAN_ISOTP
+          case CAN_ISOTP:
+          {
+              return Py_BuildValue("O&kk", PyUnicode_DecodeFSDefault,
+                                          ifname,
+                                          a->,
+                                          a->;
+          }
+          default:
+          {
+              return Py_BuildValue("O&", PyUnicode_DecodeFSDefault,
+                                        ifname);
+          }
+        }

This seems to be an intentional breakage, since the code in getsockaddrarg also operates on just the interface name, ignoring the address family.

            if (!PyArg_ParseTuple(args,
                                  "O&;AF_CAN address must be a tuple "
                                  "(interface, )",
                                  PyUnicode_FSConverter, &interfaceName))

As such, I believe the test should have been updated, but was missed during the ISOTP changes. Can someone (a core member?) confirm that this is the correct approach?

Note: However, this also implies that the CANTest test was never being run (either via CI on PRs or on the Buildbot cluster) and instead was always skipped, since I would've expected this failure to show up right away...
msg369560 - (view) Author: Karl Ding (karlding) * Date: 2020-05-22 07:47
Related: I've started a thread on Discourse [0] looking into why the tests don't seem to be run on the Buildbot cluster (and what it would take to enable them). Hopefully it gains some traction.

Date User Action Args
2020-05-22 07:47:15karldingsetmessages: + msg369560
2020-04-16 03:27:00karldingsetkeywords: + patch
stage: patch review
pull_requests: + pull_request18894
2020-04-16 03:25:02karldingcreate