-
Notifications
You must be signed in to change notification settings - Fork 4
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
Use different dev dependencies for different fedora versions #219
base: main
Are you sure you want to change the base?
Conversation
It is not sufficient to use one set of dev dependencies. In particular, pytest-asyncio has introduced a breaking change in f39.
Use a later version of `flake8`, and resolve a deprecation warning.
Due to changes in pytest-asyncio 0.23, the event loop used by fixtures/tests with different scopes is not the same. For example, a fixture declared with "module" scope will use a different event loop to tests, which run with the "function" scope's event loop. This causes many tests to fail. Despite being less efficient, the prudent way forward for now is to modify the fixtures to be recreated for each test. Astacus' tests are very fast, so this does not slow down CI runs very much at all.
@@ -90,7 +90,7 @@ f39 = [ | |||
"wcmatch == 8.4.1", | |||
"zstandard == 0.21.0", | |||
] | |||
dev = [ | |||
f38-dev = [ |
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.
pytest-cov f38 version is 4.0.0
"pytest==7.3.2", | ||
"mypy==1.8.0", | ||
# Types for things that don't seem to have them | ||
"types-botocore>=1.0.2", |
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.
I couldn't find python3-types-botocore in F38 or F39, this shouldn't be in sections named fX-dev.
"pytest-asyncio==0.23.5", | ||
"pytest-cov==4.0.0", | ||
"pytest-mock==3.11.1", | ||
"pytest-order", |
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.
I couldn't find this package in F38 or F39, this shouldn't be in sections named fX-dev.
"mypy==1.8.0", | ||
# Types for things that don't seem to have them | ||
"types-botocore>=1.0.2", | ||
"types-PyYAML>=6.0.12.2", |
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.
The version of python3-types-pyyaml is 6.0.1, both for F38 and F39.
# Types for things that don't seem to have them | ||
"types-botocore>=1.0.2", | ||
"types-PyYAML>=6.0.12.2", | ||
"types-requests>=2.28.11.5", |
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.
The version of python3-types-requests is 2.26.1, both for F38 and F39.
"types-PyYAML>=6.0.12.2", | ||
"types-requests>=2.28.11.5", | ||
"types-tabulate>=0.9.0.0", | ||
"types-ujson>=5.9.0.0", |
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.
I couldn't find python3-types-ujson in F38 or F39, but thankfully we're not using ujson at all, so this can be removed.
"types-requests>=2.28.11.5", | ||
"types-tabulate>=0.9.0.0", | ||
"types-ujson>=5.9.0.0", | ||
"types-urllib3>=1.26.25.4", |
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.
I couldn't find python3-types-urllib3 in F38 or F39, this shouldn't be in sections named fX-dev.
@@ -116,6 +116,31 @@ dev = [ | |||
"respx==0.20.1", | |||
] | |||
|
|||
f39-dev = [ |
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.
Given the large number of missing dev deps in Fedora, and to keep the project somewhat normal for non-Fedora users, we should probably have a dev section with reasonably open ranges (the actual requirements), and then fX-dev sections will fully pinned versions of the packages that can be pinned to Fedora matched versions.
|
||
- name: Execute lints and tests | ||
run: make tests | ||
run: make test |
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.
Oh ! Thanks for noticing that.
@@ -147,7 +137,7 @@ def fixture_ports() -> Ports: | |||
return Ports() | |||
|
|||
|
|||
@pytest.fixture(scope="module", name="zookeeper") | |||
@pytest.fixture(name="zookeeper") |
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.
We really can't drop all module scopes fixtures, the tests are already quite slow and it's not viable to move everything to function scope.
(Don't look at the GHA test times, many tests don't run in that context.)
Thankfully we already have a solution used in core, I've added a PR to do the same in astacus: #220
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.
oh good, I'd far prefer to move away from pytest-asyncio
. I'll amend this PR to not include those changes, and I'll review the versions to make sure they include only the versions included there.
It is not sufficient to use one set of dev dependencies. In particular, pytest-asyncio has introduced a breaking change in f39.