-
Notifications
You must be signed in to change notification settings - Fork 301
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.
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/mitigate flaky integration tests #684
Comments
Following up on questions from @sigmavirus24 in #703 (comment):
Wait, do we not all share the same definitions of "acceptance, "integration", and "unit" tests? Shocking! 😉 That said, these do feel like "acceptance"/"end-to-end"/"smoke" tests to me, and seem valuable in that capacity, as a way to simulate the user's experience and detect upstream issues.
I'm definitely open ideas, and am not married to these tests. They were originally added by @jaraco, and my effort has been focused on keeping their value while reducing their costs, which I think manifest primarily as PR noise due to flakiness of the test servers. At best, that requires a contributor or maintainer to dive into the test results to discover what failed. At worst, it blocks a merge or release. I'm keen to reduce the PR noise as soon as possible. Assuming we want some amount of testing against real servers, I'd rather not entirely disable these tests. If we do end up rewriting them, I think we'll still want them in place until that's done, which means they'll still be noisy. With that in mind, what do folks think about the "TODO" that I added to the issue description:
I think that'd be running |
👍 Let's do that. I like that option as an interim step while we think a bit harder about this |
The tests against I can reproduce this locally by running |
You’re welcome to proceed that way or assign to me and I can investigate the issue. Let me know. |
@jaraco It might be worth verifying that That said, it looks like it might be relatively straightforward to use As of #956, the |
It's working fine for me:
Moreover, I ran
My motivation for using tox was because it provides the following functionality:
I'm not super-happy with the interfaces exposed by |
Yep; I have a branch that's passing after replacing |
The issue seems to be that under some circumstances, the env used to install pypiserver is
|
I'm able to replicate the issue without
Edit: I'm not sure that shows what I think it shows. The fact that |
I upgraded my copy of Then, I ran |
I am able to work around the issue by pinning to
You have to pin in the |
Since my last comment, I root-caused and Gabor fixed the issue in tox that was leading to failures with pypiserver. Moreover, while investigating recent failures as part of #1120, I noticed that the failures tend to be 503 Service Unavailable from test.pypi.org (https://github.com/pypa/twine/actions/runs/9294853403). So it seems as if the flaky tests are due primarily to a remote service that's under-provisioned for our use-case. I'm reluctant to switch to production pypi. I don't want to be polluting that repository with lots of artifacts from tests. I also don't want to expose a token for a project in pypi to the world. Maybe we should just drop the integration test for PyPI and rely on twine's own self-upload during any release to catch any regressions. It's the other servers that are less likely to have visibility into regressions. Maybe the best course of action is to simply mark these PyPI tests as xfail until there's a better solution. |
As I write this, the checks on
master
are currently failing on Travis and GitHub actions due totest_integration.py
.https://travis-ci.org/github/pypa/twine/jobs/718433234
https://github.com/pypa/twine/runs/991209553
I've noticed this intermittently since the tests were added, but it seems to be more of a problem lately.
TODO:
testenv
) for integration tests to clearly distinguish them from the "fast" tests Use separate environmment for integration tests #704The text was updated successfully, but these errors were encountered: