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

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

Merged
merged 1 commit into from
May 4, 2021

Conversation

karlding
Copy link
Contributor

@karlding karlding commented Apr 16, 2020

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

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.
@karlding
Copy link
Contributor Author

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

Copy link
Contributor

@nanjekyejoannah nanjekyejoannah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Hi @tiran / @ned-deily,

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

@tiran tiran removed their request for review April 17, 2021 21:22
@ned-deily
Copy link
Member

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 18:11
Copy link
Member

@zware zware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels May 4, 2021
@zware zware merged commit 355bae8 into python:main May 4, 2021
@miss-islington
Copy link
Contributor

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

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
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 <[email protected]>
@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label May 4, 2021
@bedevere-bot
Copy link

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
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 <[email protected]>
miss-islington added a commit that referenced this pull request May 4, 2021
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 <[email protected]>
@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 04:51
@zware zware added the needs backport to 3.8 only security fixes label May 6, 2021
@miss-islington
Copy link
Contributor

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

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

@bedevere-bot bedevere-bot removed the needs backport to 3.8 only security fixes label May 6, 2021
ambv pushed a commit that referenced this pull request May 7, 2021
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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants