-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Begin testing sysfs port recognition #838
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
base: master
Are you sure you want to change the base?
Conversation
This introduces the following changes: * Add a `pytest-generate-test-asset` command. The command produces a detailed JSON output to capture the state of a device on a system that can be used to diagnose issues with device recognition and can be added directly to the test suite for permanence. This command only works on sysfs systems so far. * Add test assets generated from an Ubuntu 24.04 system running kernel 6.14.0 with real devices plugged in. These assets demonstrate the following subsystems: - `serial-base` - `usb` - `usb-serial` and demonstrate globbing of the following `/dev` patterns: - `/dev/ttyACM*` - `/dev/ttyS*` - `/dev/ttyUSB*` * Increase the code coverage `fail-under` value to 30%. Previously the value was 20%. * mypy was run against the new code and passes. However, type annotations are not yet guaranteed until more of the project is type annotated and mypy is passing in CI.
This introduces the following changes: * Type annotations have been added. * The code has been run through mypy. Some issues were found and fixed, but others will require additional work. * The code has been run through pyupgrade and isort. * Device patterns (like `/dev/ttyS*`) have been moved, and have been reordered to reflect which patterns have tests and which are currently untested. * The `if __name__ == "__main__"` condition has been removed. If seeing the device subsystem is valuable, the `pyserial-ports` command can be updated for that purpose. * Some typos were fixed.
andrewleech
left a comment
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.
This generated mocking does feel like a fairly pragmatic way to get closer-to-real-world test coverage.
It think it really needs a bit more documentation than the comment looking for more sample devices, it took me a bit to figure out how it's all fitting together.
I'm also not sure if it really needs the generate function exposed as a cli tool, surely a documented python -c 'import serial; serial.tools.generate... sort of line is more appropriate for the small number of people who might run it once-off to capture a device? Then some more description of how the test itself should be written to use the new asset.
|
|
||
| # Truncate if necessary. | ||
| is_truncated = False | ||
| if len(raw_contents) > 1000: |
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.
Have you seen a case where this happens? Is it neccesary?
How should the test cases handle this?
| # This file is part of pySerial. https://github.com/pyserial/pyserial | ||
| # (C) 2011-2015 Chris Liechti <cliechti@gmx.net> | ||
| # | ||
| # SPDX-License-Identifier: BSD-3-Clause |
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.
New files should have a SPDX header too
| # test | ||
| if __name__ == '__main__': | ||
| for info in sorted(comports()): | ||
| print("{0}: {0.subsystem}".format(info)) |
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.
Was it neccesary to remove this? As much as formal testing is done with pytest I often like little test harnesses like this myself.
Note
This PR is broken into two commits, and should likely be reviewed one at-a-time.
f5cb780 introduces a new command,
pyserial-generate-test-asset, which can generate a JSON output of a user's sysfs-based device information, and introduces a test framework for recreating the device's sysfs filesystem entry in-memory as it existed on the user's system.This will help with debugging sysfs-based device recognition, as well as locking valuable information into the test suite to help evaluate future code changes.
Three devices from my own testing, representing the
usb,usb-serial, andserial-basesysfs subsystems, are now tested.323c81f updates/modernizes the code in
list_ports_linux.py, which is where the sysfs-based device recognition code lives. The code was run through pyupgrade, isort, and has had type annotations added. A cursory run of mypy was made and some issues were resolved, but type annotations are not guaranteed to be correct until more of the code has been type-annotated.This introduces the following changes:
Add a
pytest-generate-test-assetcommand.The command produces a detailed JSON output
to capture the state of a device on a system
that can be used to diagnose issues with device recognition
and can be added directly to the test suite for permanence.
This command only works on sysfs systems so far.
Add test assets generated from an Ubuntu 24.04 system
running kernel 6.14.0 with real devices plugged in.
These assets demonstrate the following subsystems:
serial-baseusbusb-serialand demonstrate globbing of the following
/devpatterns:/dev/ttyACM*/dev/ttyS*/dev/ttyUSB*Increase the code coverage
fail-undervalue to 30%.Previously the value was 20%.
mypy was run against the new CLI command and passes.
However, type annotations are not yet guaranteed
until more of the project is type annotated
and mypy is passing in CI.
Type annotations have been added to
list_ports_linux.py.The
list_ports_linux.pycode has been run through mypy.Some issues were found and fixed,
but others will require additional work.
The code has been run through pyupgrade and isort.
Device patterns (like
/dev/ttyS*) have been moved,and have been reordered to reflect which patterns have tests
and which are currently untested.
The
if __name__ == "__main__"condition has been removed.If seeing the device subsystem is valuable,
the
pyserial-portscommand can be updated for that purpose.Some typos were fixed.