From 6e681909634ac06d3435430ac16ff063b25a8026 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Thu, 18 Jun 2020 05:57:38 -0400 Subject: [PATCH 01/10] Separate exception handling --- twine/__main__.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/twine/__main__.py b/twine/__main__.py index 3a19f9d1..4569c530 100644 --- a/twine/__main__.py +++ b/twine/__main__.py @@ -25,10 +25,16 @@ def main() -> Any: try: return cli.dispatch(sys.argv[1:]) - except (exceptions.TwineException, requests.HTTPError) as exc: + except requests.HTTPError as exc: + return _format_error(_format_http_error(exc)) + except exceptions.TwineException as exc: return _format_error(f"{exc.__class__.__name__}: {exc.args[0]}") +def _format_http_error(exc: requests.HTTPError) -> str: + return f"{exc.__class__.__name__}: {exc.args[0]}" + + def _format_error(message: str) -> str: pre_style, post_style = "", "" if not cli.args.no_color: From 9bb2d93fa5d3e57cc24159631b170ab5e1d2bd56 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Thu, 18 Jun 2020 06:07:33 -0400 Subject: [PATCH 02/10] Reformat HTTP error --- twine/__main__.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/twine/__main__.py b/twine/__main__.py index 4569c530..4ab69b17 100644 --- a/twine/__main__.py +++ b/twine/__main__.py @@ -12,6 +12,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import re import sys from typing import Any @@ -26,12 +27,25 @@ def main() -> Any: try: return cli.dispatch(sys.argv[1:]) except requests.HTTPError as exc: - return _format_error(_format_http_error(exc)) + return _format_error(_format_http_exception(exc)) except exceptions.TwineException as exc: - return _format_error(f"{exc.__class__.__name__}: {exc.args[0]}") + return _format_error(_format_exception(exc)) -def _format_http_error(exc: requests.HTTPError) -> str: +def _format_http_exception(exc: requests.HTTPError) -> str: + # '%s Client Error: %s for url: %s' + # '%s Server Error: %s for url: %s' + pattern = r"(?P.*Error): (?P.*) for url: (?P.*)" + match = re.match(pattern, exc.args[0]) + if match: + return ( + f"{exc.__class__.__name__} from {match['url']}: {match['status']}\n" + f"{match['reason']}" + ) + return _format_exception(exc) + + +def _format_exception(exc: Exception) -> str: return f"{exc.__class__.__name__}: {exc.args[0]}" From 9414ad85c1b7418721662ccb9bd6d3527b9fa1e8 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Thu, 18 Jun 2020 06:33:21 -0400 Subject: [PATCH 03/10] Add integration test --- tests/conftest.py | 2 +- tests/test_integration.py | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 630609a8..f497e9d6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -128,7 +128,7 @@ def entered_password(monkeypatch): monkeypatch.setattr(getpass, "getpass", lambda prompt: "entered pw") -@pytest.fixture +@pytest.fixture(scope="session") def sampleproject_dist(tmp_path_factory): checkout = tmp_path_factory.mktemp("sampleproject", numbered=False) subprocess.run( diff --git a/tests/test_integration.py b/tests/test_integration.py index c39734ae..a66542b2 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -1,7 +1,9 @@ import sys +import colorama import pytest +from twine import __main__ as dunder_main from twine import cli @@ -46,6 +48,29 @@ def test_pypi_upload(sampleproject_dist): cli.dispatch(command) +def test_pypi_error(sampleproject_dist, monkeypatch): + command = [ + "twine", + "upload", + "--repository-url", + "https://test.pypi.org/legacy/", + "--username", + "foo", + "--password", + "bar", + str(sampleproject_dist), + ] + monkeypatch.setattr(sys, "argv", command) + + message = ( + "HTTPError from https://test.pypi.org/legacy/: 403 Client Error\n" + "Invalid or non-existent authentication information. " + "See https://test.pypi.org/help/#invalid-auth for details" + ) + + assert dunder_main.main() == colorama.Fore.RED + message + colorama.Style.RESET_ALL + + @pytest.mark.xfail( sys.platform == "win32", reason="pytest-services watcher_getter fixture does not support Windows", From 5796a2206332615f29b21fee06d4259b3b23a057 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Fri, 19 Jun 2020 05:42:25 -0400 Subject: [PATCH 04/10] Add __main__ to coverage --- .coveragerc | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.coveragerc b/.coveragerc index 89e464b4..5ac2d2f4 100644 --- a/.coveragerc +++ b/.coveragerc @@ -10,13 +10,5 @@ exclude_lines = # Don't complain if non-runnable code isn't run if __name__ == .__main__.: - -# Paths to omit from consideration -omit = - # __main__.py exists only as a very basic wrapper around warehouse.cli - # and exists only to provide setuptools and python -m a place to point - # at. - */twine/__main__.py - [html] show_contexts = True From 746eaaa6c429325059dd11a3fc433dfc72eb74bb Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Fri, 19 Jun 2020 05:50:21 -0400 Subject: [PATCH 05/10] Add test of HTTPError fallback --- tests/test_integration.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_integration.py b/tests/test_integration.py index a66542b2..e0704de7 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -1,7 +1,9 @@ import sys import colorama +import pretend import pytest +import requests from twine import __main__ as dunder_main from twine import cli @@ -71,6 +73,26 @@ def test_pypi_error(sampleproject_dist, monkeypatch): assert dunder_main.main() == colorama.Fore.RED + message + colorama.Style.RESET_ALL +def test_pypi_error_fallback(sampleproject_dist, monkeypatch): + command = [ + "twine", + "upload", + "--repository-url", + "https://test.pypi.org/legacy/", + "--username", + "foo", + "--password", + "bar", + str(sampleproject_dist), + ] + monkeypatch.setattr(sys, "argv", command) + + monkeypatch.setattr(cli, "dispatch", pretend.raiser(requests.HTTPError("403"))) + + message = "HTTPError: 403" + assert dunder_main.main() == colorama.Fore.RED + message + colorama.Style.RESET_ALL + + @pytest.mark.xfail( sys.platform == "win32", reason="pytest-services watcher_getter fixture does not support Windows", From 50bd6d844b8ef204590530bb5f078b9f9d1cd6e2 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Sat, 20 Jun 2020 05:58:14 -0400 Subject: [PATCH 06/10] Use exc.response instead of re --- tests/test_integration.py | 24 +----------------------- twine/__main__.py | 19 ++++++++----------- 2 files changed, 9 insertions(+), 34 deletions(-) diff --git a/tests/test_integration.py b/tests/test_integration.py index e0704de7..c71b1c23 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -1,9 +1,7 @@ import sys import colorama -import pretend import pytest -import requests from twine import __main__ as dunder_main from twine import cli @@ -65,7 +63,7 @@ def test_pypi_error(sampleproject_dist, monkeypatch): monkeypatch.setattr(sys, "argv", command) message = ( - "HTTPError from https://test.pypi.org/legacy/: 403 Client Error\n" + "HTTPError from https://test.pypi.org/legacy/: 403 Forbidden\n" "Invalid or non-existent authentication information. " "See https://test.pypi.org/help/#invalid-auth for details" ) @@ -73,26 +71,6 @@ def test_pypi_error(sampleproject_dist, monkeypatch): assert dunder_main.main() == colorama.Fore.RED + message + colorama.Style.RESET_ALL -def test_pypi_error_fallback(sampleproject_dist, monkeypatch): - command = [ - "twine", - "upload", - "--repository-url", - "https://test.pypi.org/legacy/", - "--username", - "foo", - "--password", - "bar", - str(sampleproject_dist), - ] - monkeypatch.setattr(sys, "argv", command) - - monkeypatch.setattr(cli, "dispatch", pretend.raiser(requests.HTTPError("403"))) - - message = "HTTPError: 403" - assert dunder_main.main() == colorama.Fore.RED + message + colorama.Style.RESET_ALL - - @pytest.mark.xfail( sys.platform == "win32", reason="pytest-services watcher_getter fixture does not support Windows", diff --git a/twine/__main__.py b/twine/__main__.py index 4ab69b17..3830f303 100644 --- a/twine/__main__.py +++ b/twine/__main__.py @@ -12,7 +12,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import re +import http import sys from typing import Any @@ -33,16 +33,13 @@ def main() -> Any: def _format_http_exception(exc: requests.HTTPError) -> str: - # '%s Client Error: %s for url: %s' - # '%s Server Error: %s for url: %s' - pattern = r"(?P.*Error): (?P.*) for url: (?P.*)" - match = re.match(pattern, exc.args[0]) - if match: - return ( - f"{exc.__class__.__name__} from {match['url']}: {match['status']}\n" - f"{match['reason']}" - ) - return _format_exception(exc) + status_code = exc.response.status_code + status_phrase = http.HTTPStatus(status_code).phrase + return ( + f"{exc.__class__.__name__} from {exc.response.url}: " + f"{status_code} {status_phrase}\n" + f"{exc.response.reason}" + ) def _format_exception(exc: Exception) -> str: From 54c753cbd4875ce16f1bc7a44bab60319134d9d3 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Sat, 20 Jun 2020 06:28:33 -0400 Subject: [PATCH 07/10] Use fuzzy assertion for error message --- tests/test_integration.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/test_integration.py b/tests/test_integration.py index c71b1c23..ba274d0b 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -1,3 +1,4 @@ +import re import sys import colorama @@ -63,12 +64,14 @@ def test_pypi_error(sampleproject_dist, monkeypatch): monkeypatch.setattr(sys, "argv", command) message = ( - "HTTPError from https://test.pypi.org/legacy/: 403 Forbidden\n" - "Invalid or non-existent authentication information. " - "See https://test.pypi.org/help/#invalid-auth for details" + re.escape(colorama.Fore.RED) + + r"HTTPError from https://test.pypi.org/legacy/: 403 Forbidden\n" + + r".*authentication" ) - assert dunder_main.main() == colorama.Fore.RED + message + colorama.Style.RESET_ALL + result = dunder_main.main() + + assert re.match(message, result) @pytest.mark.xfail( From 3fd259ae162c966c7a7c49dfb2ce880301773dd5 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Sat, 20 Jun 2020 08:48:25 -0400 Subject: [PATCH 08/10] Refactor error formatting --- twine/__main__.py | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/twine/__main__.py b/twine/__main__.py index 3830f303..126359a3 100644 --- a/twine/__main__.py +++ b/twine/__main__.py @@ -25,25 +25,19 @@ def main() -> Any: try: - return cli.dispatch(sys.argv[1:]) + result = cli.dispatch(sys.argv[1:]) except requests.HTTPError as exc: - return _format_error(_format_http_exception(exc)) + status_code = exc.response.status_code + status_phrase = http.HTTPStatus(status_code).phrase + result = ( + f"{exc.__class__.__name__} from {exc.response.url}: " + f"{status_code} {status_phrase}\n" + f"{exc.response.reason}" + ) except exceptions.TwineException as exc: - return _format_error(_format_exception(exc)) + result = f"{exc.__class__.__name__}: {exc.args[0]}" - -def _format_http_exception(exc: requests.HTTPError) -> str: - status_code = exc.response.status_code - status_phrase = http.HTTPStatus(status_code).phrase - return ( - f"{exc.__class__.__name__} from {exc.response.url}: " - f"{status_code} {status_phrase}\n" - f"{exc.response.reason}" - ) - - -def _format_exception(exc: Exception) -> str: - return f"{exc.__class__.__name__}: {exc.args[0]}" + return _format_error(result) if isinstance(result, str) else result def _format_error(message: str) -> str: From 9f91476608e050369cebe6907a066df5351e0b13 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Sun, 21 Jun 2020 05:51:11 -0400 Subject: [PATCH 09/10] Shuffle status code and reason --- tests/test_integration.py | 2 +- twine/__main__.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_integration.py b/tests/test_integration.py index ba274d0b..8280b881 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -65,7 +65,7 @@ def test_pypi_error(sampleproject_dist, monkeypatch): message = ( re.escape(colorama.Fore.RED) - + r"HTTPError from https://test.pypi.org/legacy/: 403 Forbidden\n" + + r"HTTPError: 403 Forbidden from https://test.pypi.org/legacy/\n" + r".*authentication" ) diff --git a/twine/__main__.py b/twine/__main__.py index 126359a3..fd0133e2 100644 --- a/twine/__main__.py +++ b/twine/__main__.py @@ -30,8 +30,8 @@ def main() -> Any: status_code = exc.response.status_code status_phrase = http.HTTPStatus(status_code).phrase result = ( - f"{exc.__class__.__name__} from {exc.response.url}: " - f"{status_code} {status_phrase}\n" + f"{exc.__class__.__name__}: {status_code} {status_phrase} " + f"from {exc.response.url}\n" f"{exc.response.reason}" ) except exceptions.TwineException as exc: From 6b7ed1e5f24f0ac227fac3c49bcb6c40fe7005b3 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Sun, 21 Jun 2020 14:41:57 -0400 Subject: [PATCH 10/10] Tweak test regex --- tests/test_integration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_integration.py b/tests/test_integration.py index 8280b881..c499ab9c 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -65,8 +65,8 @@ def test_pypi_error(sampleproject_dist, monkeypatch): message = ( re.escape(colorama.Fore.RED) - + r"HTTPError: 403 Forbidden from https://test.pypi.org/legacy/\n" - + r".*authentication" + + r"HTTPError: 403 Forbidden from https://test\.pypi\.org/legacy/\n" + + r".+?authentication" ) result = dunder_main.main()