Repository: pypa/twine

Issue:
Make --skip-existing less brittle
`twine` checks for message starting with `File already exists`:
https://github.com/pypa/twine/blob/bcb73e025102565c395e0bf322faebd7f72e76f7/twine/commands/upload.py#L58-L61
Message has been changed to one starting with `The filename or contents already exist` in pypa/warehouse#3310:
https://github.com/pypa/warehouse/blob/969695d030aef8b11d5b052e8731a80746c404c8/warehouse/forklift/legacy.py#L1083-L1094

c.f. failing deployment: https://travis-ci.org/mayeut/gcovr/builds/360424325

Discussion:

Patch:
diff --git a/README.rst b/README.rst
index 90317fe..99d40ff 100644
--- a/README.rst
+++ b/README.rst
@@ -106,7 +106,7 @@ Once keyring is installed you can use the ``keyring`` program to set your
 username and password to use for each package index (repository) you want to
 upload to using Twine.
 
-To set your username and password for Test PyPI run the following command.
+To set your username and password for test PyPI run the following command.
 ``keyring`` will prompt you for your password:
 
 .. code-block:: console
@@ -215,7 +215,7 @@ Uploads one or more distributions to a repository.
 ``twine check``
 ^^^^^^^^^^^^^^^
 
-Checks whether your distribution's long description will render correctly on PyPI.
+Checks whether your distributions long description will render correctly on PyPI.
 
 .. code-block:: console
 
diff --git a/docs/changelog.rst b/docs/changelog.rst
index fc543da..a99f9d8 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -3,8 +3,9 @@
 =========
 Changelog
 =========
+* :bug:`332` More robust handling of server response in ``--skip-existing``
 * :release:`2.0.0 <2019-09-24>`
-* :feature:`437`: Twine now requires Python 3.6 or later. Use pip
+* :feature:`437` Twine now requires Python 3.6 or later. Use pip
   9 or pin to "twine<2" to install twine on older Python versions.
 * :bug:`491` Require requests 2.20 or later to avoid reported security
   vulnerabilities in earlier releases.
diff --git a/docs/conf.py b/docs/conf.py
index bbb709f..81d78c0 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -57,7 +57,7 @@ master_doc = "index"
 
 # General information about the project.
 project = "twine"
-copyright = "2019, Donald Stufft and individual contributors"
+copyright = "2018, Donald Stufft and individual contributors"
 
 # The version info for the project you're documenting, acts as replacement for
 # |version| and |release|, also used in various other places throughout the
diff --git a/twine/__init__.py b/twine/__init__.py
index 87eb2ea..9212c01 100644
--- a/twine/__init__.py
+++ b/twine/__init__.py
@@ -26,4 +26,4 @@ __author__ = "Donald Stufft and individual contributors"
 __email__ = "donald@stufft.io"
 
 __license__ = "Apache License, Version 2.0"
-__copyright__ = "Copyright 2019 Donald Stufft and individual contributors"
+__copyright__ = "Copyright 2018 Donald Stufft and individual contributors"
diff --git a/twine/commands/upload.py b/twine/commands/upload.py
index 8057dca..43b3b4f 100644
--- a/twine/commands/upload.py
+++ b/twine/commands/upload.py
@@ -22,30 +22,26 @@ from twine import utils
 
 
 def skip_upload(response, skip_existing, package):
-    filename = package.basefilename
-    msg_400 = (
-        # Old PyPI message:
-        f'A file named "{filename}" already exists for',
-        # Warehouse message:
-        'File already exists',
-        # Nexus Repository OSS message:
-        'Repository does not allow updating assets',
-    )
-    msg_403 = 'Not enough permissions to overwrite artifact'
+    if not skip_existing:
+        return False
+
+    status = response.status_code
+    reason = getattr(response, 'reason', '').lower()
+    text = getattr(response, 'text', '').lower()
+
     # NOTE(sigmavirus24): PyPI presently returns a 400 status code with the
     # error message in the reason attribute. Other implementations return a
-    # 409 or 403 status code. We only want to skip an upload if:
-    # 1. The user has told us to skip existing packages (skip_existing is
-    #    True) AND
-    # 2. a) The response status code is 409 OR
-    # 2. b) The response status code is 400 AND it has a reason that matches
-    #       what we expect PyPI or Nexus OSS to return to us. OR
-    # 2. c) The response status code is 403 AND the text matches what we
-    #       expect Artifactory to return to us.
-    return (skip_existing and (response.status_code == 409 or
-            (response.status_code == 400 and
-             response.reason.startswith(msg_400)) or
-            (response.status_code == 403 and msg_403 in response.text)))
+    # 403 or 409 status code.
+    return (
+        # pypiserver (https://pypi.org/project/pypiserver)
+        status == 409
+        # PyPI / TestPyPI
+        or (status == 400 and 'already exist' in reason)
+        # Nexus Repository OSS (https://www.sonatype.com/nexus-repository-oss)
+        or (status == 400 and 'updating asset' in reason)
+        # Artifactory (https://jfrog.com/artifactory/)
+        or (status == 403 and 'overwrite artifact' in text)
+    )
 
 
 def upload(upload_settings, dists):


Test patch:
diff --git a/tests/test_upload.py b/tests/test_upload.py
index 396a4aa..6885b65 100644
--- a/tests/test_upload.py
+++ b/tests/test_upload.py
@@ -203,71 +203,77 @@ def test_prints_skip_message_for_response(make_settings, capsys):
     assert RELEASE_URL not in captured.out
 
 
-def test_skip_existing_skips_files_already_on_PyPI(monkeypatch):
-    response = pretend.stub(
-        status_code=400,
-        reason='A file named "twine-1.5.0-py2.py3-none-any.whl" already '
-               'exists for twine-1.5.0.')
-
-    pkg = package.PackageFile.from_filename(WHEEL_FIXTURE, None)
-    assert upload.skip_upload(response=response,
-                              skip_existing=True,
-                              package=pkg) is True
-
-
-def test_skip_existing_skips_files_already_on_nexus(monkeypatch):
-    # Nexus Repository Manager (https://www.sonatype.com/nexus-repository-oss)
-    # responds with 400 when the file already exists
-    response = pretend.stub(
-        status_code=400,
-        reason="Repository does not allow updating assets: pypi for url: "
-               "http://www.foo.bar")
-
-    pkg = package.PackageFile.from_filename(WHEEL_FIXTURE, None)
-    assert upload.skip_upload(response=response,
-                              skip_existing=True,
-                              package=pkg) is True
-
-
-def test_skip_existing_skips_files_already_on_pypiserver(monkeypatch):
-    # pypiserver (https://pypi.org/project/pypiserver) responds with a
-    # 409 when the file already exists.
-    response = pretend.stub(
-        status_code=409,
-        reason='A file named "twine-1.5.0-py2.py3-none-any.whl" already '
-               'exists for twine-1.5.0.')
+@pytest.mark.parametrize('response_kwargs', [
+    pytest.param(
+        dict(status_code=400, reason=(
+            'A file named "twine-1.5.0-py2.py3-none-any.whl" already '
+            'exists for twine-1.5.0.'
+        )),
+        id='pypi'
+    ),
+    pytest.param(
+        dict(status_code=400, reason=(
+            'Repository does not allow updating assets: pypi for url: '
+            'http://www.foo.bar'
+        )),
+        id='nexus'
+    ),
+    pytest.param(
+        dict(status_code=409, reason=(
+            'A file named "twine-1.5.0-py2.py3-none-any.whl" already '
+            'exists for twine-1.5.0.'
+        )),
+        id='pypiserver'
+    ),
+    pytest.param(
+        dict(status_code=403, text=(
+            "Not enough permissions to overwrite artifact "
+            "'pypi-local:twine/1.5.0/twine-1.5.0-py2.py3-none-any.whl'"
+            "(user 'twine-deployer' needs DELETE permission)."
+        )),
+        id='artifactory_old'
+    ),
+    pytest.param(
+        dict(status_code=403, text=(
+            "Not enough permissions to delete/overwrite artifact "
+            "'pypi-local:twine/1.5.0/twine-1.5.0-py2.py3-none-any.whl'"
+            "(user 'twine-deployer' needs DELETE permission)."
+        )),
+        id='artifactory_new'
+    ),
+])
+def test_skip_existing_skips_files_on_repository(response_kwargs):
+    assert upload.skip_upload(
+        response=pretend.stub(**response_kwargs),
+        skip_existing=True,
+        package=package.PackageFile.from_filename(WHEEL_FIXTURE, None),
+    )
+
 
-    pkg = package.PackageFile.from_filename(WHEEL_FIXTURE, None)
-    assert upload.skip_upload(response=response,
-                              skip_existing=True,
-                              package=pkg) is True
+@pytest.mark.parametrize('response_kwargs', [
+    pytest.param(
+        dict(status_code=400, reason='Invalid credentials'),
+        id='wrong_reason'
+    ),
+    pytest.param(
+        dict(status_code=404),
+        id='wrong_code'
+    ),
+])
+def test_skip_upload_doesnt_match(response_kwargs):
+    assert not upload.skip_upload(
+        response=pretend.stub(**response_kwargs),
+        skip_existing=True,
+        package=package.PackageFile.from_filename(WHEEL_FIXTURE, None)
+    )
 
 
-def test_skip_existing_skips_files_already_on_artifactory(monkeypatch):
-    # Artifactory (https://jfrog.com/artifactory/) responds with 403
-    # when the file already exists.
-    response = pretend.stub(
-        status_code=403,
-        text="Not enough permissions to overwrite artifact "
-             "'pypi-local:twine/1.5.0/twine-1.5.0-py2.py3-none-any.whl'"
-             "(user 'twine-deployer' needs DELETE permission).")
-
-    pkg = package.PackageFile.from_filename(WHEEL_FIXTURE, None)
-    assert upload.skip_upload(response=response,
-                              skip_existing=True,
-                              package=pkg) is True
-
-
-def test_skip_upload_respects_skip_existing(monkeypatch):
-    response = pretend.stub(
-        status_code=400,
-        reason='A file named "twine-1.5.0-py2.py3-none-any.whl" already '
-               'exists for twine-1.5.0.')
-
-    pkg = package.PackageFile.from_filename(WHEEL_FIXTURE, None)
-    assert upload.skip_upload(response=response,
-                              skip_existing=False,
-                              package=pkg) is False
+def test_skip_upload_respects_skip_existing():
+    assert not upload.skip_upload(
+        response=pretend.stub(),
+        skip_existing=False,
+        package=package.PackageFile.from_filename(WHEEL_FIXTURE, None),
+    )
 
 
 def test_values_from_env(monkeypatch):