Closed robberwick closed 5 months ago
This is a dumb question, but did you run the tests and everything checked out?
that's not a dumb question, that's a very good question. I should have included the output of a test run in the pr description.
========================================== test session starts ===========================================
platform win32 -- Python 3.11.0, pytest-8.1.1, pluggy-1.4.0
plugins: cov-5.0.0, mock-3.14.0
collected 80 items
tests\test_crc.py .............x..x.. [ 23%]
tests\test_py_serial_transfer.py .....................................x................x...... [100%]
---------- coverage: platform win32, python 3.11.0-final-0 -----------
Name Stmts Miss Cover
----------------------------------------------------------
pySerialTransfer\CRC.py 42 4 90%
pySerialTransfer\__init__.py 1 0 100%
pySerialTransfer\pySerialTransfer.py 303 42 86%
----------------------------------------------------------
TOTAL 346 46 87%
===================================== 76 passed, 4 xfailed in 0.73s ======================================
x
denotes an expected fail:
test_calculate_with_dist_greater_than_list_length
test_calculate_with_negative_dist
calculate
method currently fails those tests. once the tests are merged, i'll raise an issue (or just a pr) to enable the tests and fix the edge cases.test_tx_obj_known_types
with a val_type_override
of 'c' doesn't gracefully handle exceptions. not sure what to do here, so marked the test as xfail
test_tick_with_valid_data_and_non_callable_callback
- I raised an issue for this, and have a pr ready to go, but it of course relies on having the test suite, and i wanted to keep this pr focused on only introducing tests.
Ok, thanks!
Adds first cut at a test suite for the
SerialTransfer
andCRC
classes. It only provides ~85% coverage in total (~90% forCRC.py
and 84% forpySerialTransfer.py
). In part this is due to not covering the non core library functionality i.e. the crc__name__ == "__main__"
code and the helper functions such asmsb
,lsb
etc.The coverage is not the full story though, obviously. There are definitely untested code paths here also (which can be seen if pytest is run with coverage turned on), so there are definitely areas for improvement. This feels like a good start however, and I think will go some way to providing a defence against regressions when extending or refactoring the functionality of the library.
It's also worth noting that the content of the tests themselves probably wants reviewing to double check that the things i am asserting in the tests are actually correct, as the'll be treated as the source of truth. As you did all the excellent work in the first plaec, you'll be well placed to point out where i've made stupid mistakes :-).
Note: I have added
pytest
,pytest-mock
, andpytest-cov
as dev dependencies. In order to run the tests, it is necessary to perform an editable install of the package, passing the extra[dev]
qualifier e.g.pip install -e .[dev]
. Tests can then be run with justpytest
, or with coverage enabled withpytest --cov .\pySerialTransfer
Addresses #89