Skip to content

added cov-fail-under=100 for pytest #206

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

Merged
sethmlarson merged 2 commits into from
Aug 15, 2019
Merged

added cov-fail-under=100 for pytest #206

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b4b08e
added cov-fail-under=100 for pytest
Aug 14, 2019
0f2cf98
Merge branch 'master' into fail-under
sethmlarson Aug 15, 2019
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
2 changes: 1 addition & 1 deletion httpx/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def __init__(
if idna_authority != self._uri_reference.authority:
self._uri_reference = self._uri_reference.copy_with(
authority=idna_authority
)
) # pragma: nocover
Copy link
Copy Markdown
Contributor

@sethmlarson sethmlarson Aug 13, 2019

Choose a reason for hiding this comment

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

Instead of disabling coverage here let's create a simple unit test for the branch.

Copy link
Copy Markdown
Contributor Author

@cansarigol cansarigol Aug 14, 2019

Choose a reason for hiding this comment

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

I couldn't find a test case because if I try to Unicode domain, I get this error: rfc3986.exceptions.InvalidAuthority: The authority (😁.de) is not valid.

Should we change this part if there is no case or could you help me with this?

Copy link
Copy Markdown
Contributor

@sethmlarson sethmlarson Aug 14, 2019

Choose a reason for hiding this comment

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

What about ουτοπία.δπθ.gr? Emojis aren't valid for IDNs for IDNA 2008.

Copy link
Copy Markdown
Contributor Author

@cansarigol cansarigol Aug 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

idna encode raises an error if there is any char that greater then 128. (it's not just about emoji link) so this if clause is never used. If you want we can delete this part with another PR and check InvalidAuthority error?

Copy link
Copy Markdown
Contributor

@sethmlarson sethmlarson Aug 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I think we can literally just get rid of that whole if statement branch. IRIReference.encode() does everything for us. Tested it locally and it seems to work for me.

Copy link
Copy Markdown
Contributor Author

@cansarigol cansarigol Aug 14, 2019

Choose a reason for hiding this comment

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

added a pr #210


# Normalize scheme and domain name.
if self.is_absolute_url:
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ line_length = 88
multi_line_output = 3

[tool:pytest]
addopts = --cov=httpx --cov=tests --cov-report=term-missing
addopts = --cov=httpx --cov=tests --cov-report=term-missing --cov-fail-under=100
6 changes: 6 additions & 0 deletions tests/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ def test_bad_get_netrc_login():
os.environ["NETRC"] = "wrongpath"
assert get_netrc_login("url") is None

from httpx import utils

utils.NETRC_STATIC_FILES = ()
os.environ["NETRC"] = ""
assert utils.get_netrc_login("url") is None


def test_get_netrc_login():
os.environ["NETRC"] = "tests/.netrc"
Expand Down