This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: test_socket.CANTest is broken at HEAD on master
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.11, Python 3.10, Python 3.9, Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: karlding, lukasz.langa, miss-islington, ned.deily, pablogsal, zach.ware
Priority: normal Keywords: patch

Created on 2020-04-16 03:25 by karlding, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 19548 merged karlding, 2020-04-16 03:27
PR 25902 merged miss-islington, 2021-05-04 20:37
PR 25903 merged miss-islington, 2021-05-04 20:37
PR 25957 merged miss-islington, 2021-05-06 21:00
PR 25960 merged zach.ware, 2021-05-07 02:21
Messages (7)
msg366576 - (view) Author: Karl Ding (karlding) * Date: 2020-04-16 03:25
While working on https://bugs.python.org/issue40291, 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. https://github.com/python/cpython/pull/2956/files#diff-a47fd74731aeb547ad780900bb8e6953L1376-R1391

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->can_addr.tp.rx_id,
+                                          a->can_addr.tp.tx_id);
+          }
+#endif
+          default:
+          {
+              return Py_BuildValue("O&", PyUnicode_DecodeFSDefault,
+                                        ifname);
+          }
+        }
     }
 #endif

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.

[0] https://discuss.python.org/t/what-would-it-take-to-run-socketcan-tests-on-a-buildbot/4137
msg392952 - (view) Author: miss-islington (miss-islington) Date: 2021-05-04 21:01
New changeset 7b4725a210bcfe3dc65f37cbada423cbd1e9f063 by Miss Islington (bot) in branch '3.10':
bpo-40297: Fix test_socket.CANTest.testSendFrame (GH-19548)
https://github.com/python/cpython/commit/7b4725a210bcfe3dc65f37cbada423cbd1e9f063
msg392953 - (view) Author: miss-islington (miss-islington) Date: 2021-05-04 21:03
New changeset df99532a05e4cfba8d9835375d4a3830b84472ad by Miss Islington (bot) in branch '3.9':
bpo-40297: Fix test_socket.CANTest.testSendFrame (GH-19548)
https://github.com/python/cpython/commit/df99532a05e4cfba8d9835375d4a3830b84472ad
msg392954 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2021-05-04 21:05
Thanks for the thoroughly researched patch and your patience!  Sorry it took so long to get it merged.
msg393185 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-05-07 13:54
New changeset 8a12f46dd8c780de84d78e6dd8350230e52e0c46 by Miss Islington (bot) in branch '3.8':
bpo-40297: Fix test_socket.CANTest.testSendFrame (GH-19548) (#25957)
https://github.com/python/cpython/commit/8a12f46dd8c780de84d78e6dd8350230e52e0c46
msg393210 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2021-05-07 19:36
New changeset 1beae7e523d2db4e3ce383b2032095ef956f21c4 by Zachary Ware in branch '3.7':
[3.7] bpo-40297: Fix test_socket.CANTest.testSendFrame (GH-25960)
https://github.com/python/cpython/commit/1beae7e523d2db4e3ce383b2032095ef956f21c4
History
Date User Action Args
2022-04-11 14:59:29adminsetgithub: 84477
2021-07-18 15:30:23pablogsalsetpull_requests: - pull_request25776
2021-07-18 15:26:31pablogsalsetnosy: + pablogsal

pull_requests: + pull_request25776
2021-05-07 19:36:39ned.deilysetnosy: + ned.deily
messages: + msg393210
2021-05-07 13:54:55lukasz.langasetnosy: + lukasz.langa
messages: + msg393185
2021-05-07 02:21:52zach.waresetpull_requests: + pull_request24619
2021-05-07 02:11:15zach.waresetversions: + Python 3.7, Python 3.8
2021-05-06 21:00:42miss-islingtonsetpull_requests: + pull_request24618
2021-05-04 21:05:44zach.waresetstatus: open -> closed

versions: + Python 3.10, Python 3.11
nosy: + zach.ware

messages: + msg392954
resolution: fixed
stage: patch review -> resolved
2021-05-04 21:03:55miss-islingtonsetmessages: + msg392953
2021-05-04 21:01:48miss-islingtonsetmessages: + msg392952
2021-05-04 20:37:29miss-islingtonsetpull_requests: + pull_request24571
2021-05-04 20:37:24miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request24570
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