Running twine tests in dev environment fail and emit PyPI tokens

[jaraco]

Is there an existing issue for this?

• [X] I have searched the existing issues (open and closed), and could not find an existing issue

What keywords did you use to search existing issues?

tests isolation

Please describe why your using this option

Running tox in a clean checkout of twine produces 5 failures:

FAILED tests/test_package.py::test_pkginfo_returns_no_metadata[unsupported Metadata-Version] - Failed: DID NOT RAISE <class 'twine.exceptions.InvalidDistribution'>
FAILED tests/test_settings.py::test_password_is_required_if_no_client_cert[None] - AssertionError: assert 'pypi-AgEIcHl...<elided>' == 'entered pw'
FAILED tests/test_settings.py::test_password_is_required_if_no_client_cert[] - AssertionError: assert 'pypi-AgEIcHl...<elided> == 'entered pw'
FAILED tests/test_settings.py::test_password_required_if_no_client_cert_and_non_interactive - Failed: DID NOT RAISE <class 'twine.exceptions.NonInteractive'>
FAILED tests/test_settings.py::test_no_password_prompt_if_client_cert_and_non_interactive - AssertionError: assert not 'pypi-AgEIcHl<elided>...

It appears as if the presence of credentials in my keyring is causing tests to fail (and emit my credentials).

Anything else you'd like to mention?

No response

[jaraco]

Running the tests on 4.0.2 produces a mostly different set of failures, seemingly related to build backend issues. Beginning with 5.0.0, the current set of failures is observed.

[sigmavirus24]

Integration tests broke nearly as soon as you added them over a year ago, but I knew you'd complain if I removed them and you didn't notice any of the pings. Brian ignored them before me and since they very occasionally pass, I've left them.

Master is failing now because Tres released an API breaking change in a minor version of pkginfo. I just haven't had time to fix it.

Edit Just realized this was a different issue you opened from the one about master tests being broken.

[jaraco]

Master is failing now because Tres released an API breaking change in a minor version of pkginfo. I just haven't had time to fix it.

There appear to be two failure modes here. The first is in test_pkginfo_returns_no_metadata, which also fails in an environment without my credentials. That's probably the issue with pkginfo. It was reported in #1116. Let's disregard that one for the sake of this issue, which is about the other failures which happen in my dev environment due to the presence of my PyPI credentials.

[jaraco]

It looks like when 2afda476ef7edb78c216663273a449966549acfd was introduced (#1040), it supersedes the username="fakeuser" in the tests, allowing keyring to resolve the user's PyPI token instead of the entered_password.

[jaraco]

I'm unsure the best way forward here. An easy fix might be to disable keyring in tests - mock it out so it always behaves as if there's no password. Another option could be to make __token__ the default username when accessing PyPI or TestPyPI, but have it honor the specified username if supplied. @woodruffw do you have an opinion on the matter?

[woodruffw]

Another option could be to make __token__ the default username when accessing PyPI or TestPyPI, but have it honor the specified username if supplied. @woodruffw do you have an opinion on the matter?

No strong opinion on the mocking idea, but I think this is likely to cause user confusion (since PyPI and TestPyPI will no longer accept a username at all for package upload, and will always fail hard.)

[jaraco]

@sigmavirus24 or @bhrutledge : Do you have an opinion on how to deal with this scenario?

In ddd4ecb, I've drafted "token" as default if unspecified. It addresses the issue, but creates four new test failures (exactly the concern woodruffw raised):

FAILED tests/test_register.py::test_values_from_env_pypi[pypi] - AssertionError: assert '__token__' == 'this-is-ignored'
FAILED tests/test_register.py::test_values_from_env_pypi[testpypi] - AssertionError: assert '__token__' == 'this-is-ignored'
FAILED tests/test_upload.py::test_values_from_env_pypi[pypi] - AssertionError: assert '__token__' == 'this-is-ignored'
FAILED tests/test_upload.py::test_values_from_env_pypi[testpypi] - AssertionError: assert '__token__' == 'this-is-ignored'

It would be a breaking change to drop that expectation, but it feels like the cleanest approach. It also means that users have more control over their environment, but it means that those reliant on the current hard-coded behavior will need to remove any configured username to retain compatibility. Honestly, that feels best to me because the current approach is masking an unused configuration. It feels wrong for a user (or test) to be explicitly supplying a configuration but it's silently suppressed.

I'm also trying to avoid test-only behaviors. I'd like for the tests to exercise the system as closely as possible to how it operates in production, but since it's not possible to override the username or otherwise select a non-default username, the tests are hitting real credentials. It feels like the current design, overriding with a username="fakeuser" is the most genuine way to avoid hitting real credentials.

[jaraco]

In 2c4e4a1, I've committed the changes that allow the tests to pass. Interestingly, not all of 4a2dfb0d44c5f0d195bfe33475e7dd2a26feeeaf had to be reverted. The fixtures in test_settings still retain the behavior that __token__ takes precedence (because the default value takes precedence over config), so only users setting TWINE_USERNAME or specifying the username on the command line will see a change in behavior.