chore: add typing hints to pytest components #1478
Conversation
rpoisel
force-pushed
the
dev-add-pytest-plugin-typing-hints
branch
3 times, most recently
from
b9f455c to
200d915
Compare
labgrid/config.py
Outdated
| """ | ||
| import os | ||
| import warnings | ||
| from typing import Dict |
There was a problem hiding this comment.
Does Labgrid sort its imports?
There was a problem hiding this comment.
That's the way ruff sorts imports with the current configuration in this repo.
Would you prefer them to be in a different order?
There was a problem hiding this comment.
No, I suppose I just don't understand how the ordering works
Perhaps 'warnings' is consider an internal module and 'typing' is not?
There was a problem hiding this comment.
Ah, I see. Ruff puts from imports after imports without the from keyword.
| "pytest_configure", | ||
| "pytest_collection_modifyitems", | ||
| "pytest_cmdline_main", | ||
| ] |
There was a problem hiding this comment.
Should this be in a separate commit?
| target = env.get_target(target_name) | ||
| except UserError as e: | ||
| pytest.exit(e) | ||
| pytest.exit(str(e)) |
There was a problem hiding this comment.
This change seems to be unrelated to typing? Or does it fix a bug? If so, best to put the bug-fixes first, in one or more separate commits.
There was a problem hiding this comment.
Actually, this fixes that the wrong type of the reason parameter is passed to the pytest.exit() function: pytest.exit.
tests/test_fixtures.py
Outdated
|
|
||
| def test_config(short_test): | ||
| with pexpect.spawn(f'pytest --traceconfig {short_test}') as spawn: | ||
| with pexpect.spawn(f"pytest --traceconfig {short_test}") as spawn: |
There was a problem hiding this comment.
Is Labgrid supposed to use " instead of ' ? I see both, I have been wondering
There was a problem hiding this comment.
Good point, I'll make sure these things remain untouched in this PR.
Personally, I'd prefer them to be consistent (whether it's ' or ").
There was a problem hiding this comment.
I agree. I much prefer ' since it is easier to distinguish as small font sizes, but Labgrid seems to use both
There was a problem hiding this comment.
I just updated the pyproject.toml file to contain:
[tool.ruff.format]
quote-style = "single"Actually, I'd suggest to re-format the whole code base. But this should go into a dedicated PR in my view.
In this PR, however, I already re-formatted the tests/test_fixtures.py file to this new formatting rule anyways.
There was a problem hiding this comment.
Okay, this does not work yet. All sources would have to be re-formatted this way before passing CI checks.
As a quick workaround I applied this setting to the tests/test_fixtures.py file, but removed it from the pyproject.toml file.
CHANGES.rst
Outdated
| New Features in 24.1 | ||
| ~~~~~~~~~~~~~~~~~~~~ | ||
| - All components can be installed into the same virtualenv again. | ||
| - The ``pytest`` specific parts of the labgrid framework now have typing hints. |
rpoisel
force-pushed
the
dev-add-pytest-plugin-typing-hints
branch
2 times, most recently
from
79fb518 to
7df770f
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1478 +/- ##
========================================
+ Coverage 45.4% 46.8% +1.4%
========================================
Files 172 172
Lines 13501 13519 +18
========================================
+ Hits 6131 6340 +209
+ Misses 7370 7179 -191
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@sjg20, could you please review my PR again? All your comments should be implemented/resolved now. To be honest, I'd love to see this PR merged into labgrid as it improves the code navigation and static code analysis experience quite a lot. |
rpoisel
force-pushed
the
dev-add-pytest-plugin-typing-hints
branch
from
7df770f to
a8f887c
Compare
rpoisel
force-pushed
the
dev-add-pytest-plugin-typing-hints
branch
7 times, most recently
from
854f9ac to
629c116
Compare
|
@sjg20, could you please have a look? I just made sure this PR can be merged without conflicts. |
Add this logging level without changing Python’s internal logging module. This maintains architectural integrity and clarifies that the level is labgrid-specific. Signed-off-by: Rainer Poisel <[email protected]>
Signed-off-by: Rainer Poisel <[email protected]>
Signed-off-by: Rainer Poisel <[email protected]>
Signed-off-by: Rainer Poisel <[email protected]>
rpoisel
force-pushed
the
dev-add-pytest-plugin-typing-hints
branch
from
629c116 to
e64359a
Compare
|
The Python 3.14 errors should be fixed when rebasing on latest master. |
Thanks for the hint! Just found out that Python 3.14 support has been added recently. I'd be very happy to get this PR merged asap. It is already over 1 year old and rebasing it always takes some effort. Hopefully, I get the errors fixed today evening (CEST) or tomorrow. |
Signed-off-by: Rainer Poisel <[email protected]>
…nents Signed-off-by: Rainer Poisel <[email protected]>
Signed-off-by: Rainer Poisel <[email protected]>
Signed-off-by: Rainer Poisel <[email protected]>
Signed-off-by: Rainer Poisel <[email protected]>
rpoisel
force-pushed
the
dev-add-pytest-plugin-typing-hints
branch
from
e64359a to
83dfee8
Compare
3 participants
Description
This PR introduces Python typing hints to the
pytestspecific components of the labgrid framework. These additions can be seen as initial steps towards enhancing the overall code navigation experience and improving the effectiveness of static code analysis tools.Checklist