-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Deduplicate testing dependencies by dropping [testing-integration]
#4283
Deduplicate testing dependencies by dropping [testing-integration]
#4283
Conversation
548da66
to
ff4b87c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much, @Avasam .
I think this is the simplest approach...
The next 2 weeks I don't know if I will have the time to contribute here, but my plan is to download the PR and run the integration tests manually to see if everything works as expected (I think the CI do not run them on PR).
@Avasam, when running the tests I was having an error1:
As a workaround, I added d98a9d9. Line 16 in 6ee23bf
Footnotes
|
@abravalheri I remember having trove_classifier be picked up as an import previously, but I don't remember if/how I fixed it. Or if I just fixed it locally by installing trove_classifier then the CI did something different that didn't need it explicitly installed. Either way, the difference that I notice now is that integration tests will also run checkdocks, cov, mypy, ruff and perf whereas previously it didn't. |
d98a9d9
to
3ca45b7
Compare
@Avasam 6c118be is a way of disabling those tests. Does it make sense for you, or should I revert that commit? I believe that |
ded2634
to
6c118be
Compare
Hmm, in a way, I wonder if we're just moving the duplication from one file to another. Duplicating the declaration of dependencies/tests not to be repeated, where before we were duplicating shared dependencies. Whilst leaving the door open to forgetting to disable a test. Not that I see new tooling being added too often, so it'd probably be caught eventually. Other than that, it does bring setup cfg closer to the skeleton and dedupes dependencies in that file. |
Yeah, that is true... well, it is an optimisation thing, we can drop it at some point. I think that if the |
Summary of changes
In #4257 (comment), @abravalheri mentioned that:
This PR is doing just that.
Accepting this closes #4282 , which was my original idea.
Pull Request Checklist
newsfragments/
.(See documentation for details)