Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpo-40297: Fix test_socket.CANTest.testSendFrame #19548

Merged
merged 1 commit into from May 4, 2021
Merged

bpo-40297: Fix test_socket.CANTest.testSendFrame #19548

merged 1 commit into from May 4, 2021

Conversation

karlding
Copy link
Contributor

@karlding karlding commented Apr 16, 2020 •
edited by bedevere-bot

The address tuple for CAN_RAW no longer returns the address family
after the introduction of CAN ISO-TP support in a30f6d4. However,
updating test_socket.CANTest.testSendFrame was missed as part of the
change, so the test incorrectly attempts to index past the last tuple
item to retrieve the address family.

This removes the now-redundant check for equality against socket.AF_CAN,
as the tuple will not contain the address family.

https://bugs.python.org/issue40297

@karlding
bpo-40297: Fix test_socket.CANTest.testSendFrame
4e2bb15 
The address tuple for CAN_RAW no longer returns the address family
after the introduction of CAN ISO-TP support in a30f6d4. However,
updating test_socket.CANTest.testSendFrame was missed as part of the
change, so the test incorrectly attempts to index past the last tuple
item to retrieve the address family.

This removes the now-redundant check for equality against socket.AF_CAN,
as the tuple will not contain the address family.
@bedevere-bot bedevere-bot added tests Tests in the Lib/test dir awaiting review labels Apr 16, 2020
@nanjekyejoannah nanjekyejoannah requested review from tiran and ned-deily Jul 9, 2020
@karlding
Copy link
Contributor Author

karlding commented Sep 26, 2020

@nanjekyejoannah I see that you've assigned reviewers to this PR. Is there anything that I can do to help have this change reviewed?

nanjekyejoannah
nanjekyejoannah reviewed Sep 27, 2020
View changes
Copy link
Member

@nanjekyejoannah nanjekyejoannah left a comment

The change looks reasonable to me but I wanted an opinion from either @tiran or @ned-deily as dedicated experts to these parts of the project hence the ping.

@karlding
Copy link
Contributor Author

karlding commented Apr 17, 2021

Hi @tiran / @ned-deily,

Is there anything I can do to help get this merged?

@tiran tiran removed their request for review Apr 17, 2021
@ned-deily
Copy link
Member

ned-deily commented May 3, 2021

Sorry, I don't have any particular expertise in this area so I am removing myself as a reviewer.

@ned-deily ned-deily removed their request for review May 3, 2021
zware
zware approved these changes May 4, 2021
View changes
Copy link
Member

@zware zware left a comment

LGTM; I've reproduced the failure locally. I'll try to get my Gentoo bot set up to test this, and will merge as soon as it is ready (unless someone else agrees with the change without a buildbot to test it and merges before I do :))

@zware
Copy link
Member

zware commented May 4, 2021

My x86 Gentoo buildbot worker now has the vcan0 interface set up and reproduced the failure: https://buildbot.python.org/all/#/builders/464/builds/155

@zware zware merged commit 355bae8 into python:main May 4, 2021
1 check passed
@miss-islington
Copy link
Contributor

miss-islington commented May 4, 2021

Thanks @karlding for the PR, and @zware for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.9.
🐍🍒⛏🤖

@bedevere-bot
Copy link

bedevere-bot commented May 4, 2021

GH-25902 is a backport of this pull request to the 3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 4, 2021
@karlding @miss-islington
bpo-40297: Fix test_socket.CANTest.testSendFrame (pythonGH-19548)
3f14824 
The address tuple for CAN_RAW no longer returns the address family
after the introduction of CAN ISO-TP support in a30f6d4. However,
updating test_socket.CANTest.testSendFrame was missed as part of the
change, so the test incorrectly attempts to index past the last tuple
item to retrieve the address family.

This removes the now-redundant check for equality against socket.AF_CAN,
as the tuple will not contain the address family.
(cherry picked from commit 355bae8)

Co-authored-by: karl ding <karlding@users.noreply.github.com>
@bedevere-bot
Copy link

bedevere-bot commented May 4, 2021

GH-25903 is a backport of this pull request to the 3.9 branch.

miss-islington added a commit that referenced this pull request May 4, 2021
@miss-islington @karlding
bpo-40297: Fix test_socket.CANTest.testSendFrame (GH-19548)
7b4725a 
The address tuple for CAN_RAW no longer returns the address family
after the introduction of CAN ISO-TP support in a30f6d4. However,
updating test_socket.CANTest.testSendFrame was missed as part of the
change, so the test incorrectly attempts to index past the last tuple
item to retrieve the address family.

This removes the now-redundant check for equality against socket.AF_CAN,
as the tuple will not contain the address family.
(cherry picked from commit 355bae8)

Co-authored-by: karl ding <karlding@users.noreply.github.com>
miss-islington added a commit that referenced this pull request May 4, 2021
@miss-islington @karlding
bpo-40297: Fix test_socket.CANTest.testSendFrame (GH-19548)
df99532 
The address tuple for CAN_RAW no longer returns the address family
after the introduction of CAN ISO-TP support in a30f6d4. However,
updating test_socket.CANTest.testSendFrame was missed as part of the
change, so the test incorrectly attempts to index past the last tuple
item to retrieve the address family.

This removes the now-redundant check for equality against socket.AF_CAN,
as the tuple will not contain the address family.
(cherry picked from commit 355bae8)

Co-authored-by: karl ding <karlding@users.noreply.github.com>
@karlding
Copy link
Contributor Author

karlding commented May 5, 2021

Thank you for your help reviewing, and for setting up your buildbot for the additional coverage! 🙂

@karlding karlding deleted the bpo_40297_fix_can_test branch May 5, 2021
@miss-islington
Copy link
Contributor

miss-islington commented May 6, 2021

Thanks @karlding for the PR, and @zware for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@bedevere-bot
Copy link

bedevere-bot commented May 6, 2021

GH-25957 is a backport of this pull request to the 3.8 branch.

ambv pushed a commit that referenced this pull request May 7, 2021
@miss-islington @karlding
bpo-40297: Fix test_socket.CANTest.testSendFrame (GH-19548) (#25957)
8a12f46 
The address tuple for CAN_RAW no longer returns the address family
after the introduction of CAN ISO-TP support in a30f6d4. However,
updating test_socket.CANTest.testSendFrame was missed as part of the
change, so the test incorrectly attempts to index past the last tuple
item to retrieve the address family.

This removes the now-redundant check for equality against socket.AF_CAN,
as the tuple will not contain the address family.
(cherry picked from commit 355bae8)

Co-authored-by: karl ding <karlding@users.noreply.github.com>
@pablogsal pablogsal mentioned this pull request Jul 18, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nanjekyejoannah nanjekyejoannah

@zware zware

Assignees
No one assigned
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@karlding @ned-deily @zware @miss-islington @bedevere-bot @nanjekyejoannah @the-knights-who-say-ni