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.

Sign up for GitHub

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

Fix tests with Python 3.11.4+ #213

Merged
merged 3 commits into from
Aug 2, 2023
Merged

Fix tests with Python 3.11.4+ #213

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6c6c29a
Fix tests with Python 3.11.4+
shadchin Jun 21, 2023
8b3ec31
Aplay black
shadchin Jul 3, 2023
6a5235a
Rework fix test for USERNAME_TO_ENCODE
shadchin Jul 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion tests/test_url.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import sys
import os
import unittest
from inspect import isclass
Expand Down Expand Up @@ -386,7 +387,6 @@ def test_safe_url_string_encoding(
"http://192.168.0.256", # Invalid IP address
"http://192.168.0.0.0", # Invalid IP address / domain name
"http://[2a01:5cc0:1:2::4]", # https://github.com/scrapy/w3lib/issues/193
"http://[2a01:5cc0:1:2:3:4]", # Invalid IPv6
"https://example.com:", # Removes the :
# Does not convert \ to /
"https://example.com\\a",
Expand Down Expand Up @@ -418,6 +418,12 @@ def test_safe_url_string_encoding(
# (%) are not escaped.
f"a://example.com#{FRAGMENT_TO_ENCODE}",
}
if sys.version_info < (3, 11, 4):
KNOWN_SAFE_URL_STRING_URL_ISSUES.add("http://[2a01:5cc0:1:2:3:4]") # Invalid IPv6
else:
KNOWN_SAFE_URL_STRING_URL_ISSUES.add(
f"https://{USERNAME_TO_ENCODE}:{PASSWORD_TO_ENCODE}@example.com"
)
Copy link
Member

Choose a reason for hiding this comment

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

So, to make sure I understand this correctly:

  1. Both safe_url_string("http://[2a01:5cc0:1:2:3:4]") and safe_url_string('https://"%;<=>@[]^`{|}\x7f:"%;<=>@[]^`{|}\x7f:@example.com') work on older Python versions but raise a ValueError on 3.11.4.
  2. test_safe_url_string_url is a test that takes input and output of safe_url_string() from SAFE_URL_URL_CASES and expects that output unless the input is in KNOWN_SAFE_URL_STRING_URL_ISSUES.
  3. Hence, KNOWN_SAFE_URL_STRING_URL_ISSUES is basically a list of cases that should xfail.
  4. SAFE_URL_URL_CASES says that "http://[2a01:5cc0:1:2:3:4]" should give ValueError and this is true only on the new Python, so this change removes xfail for it on the new Python.
  5. SAFE_URL_URL_CASES says that safe_url_string('https://"%;<=>@[]^`{|}\x7f:"%;<=>@[]^`{|}\x7f:@example.com') should be encoded correctly and that works only on older Python, so this change adds xfail for it on the new Python.

With this in mind I feel like we should instead change the USERNAME_TO_ENCODE and/or PASSWORD_TO_ENCODE so that it works on all Python versions instead. It's probably enough to remove [] from there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, if remove [] from USERNAME_TO_ENCODE, then test pass

Copy link
Member

Choose a reason for hiding this comment

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

Cool, can you please make this change?



@pytest.mark.parametrize(
Expand Down