Skip to content
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

Add tests for core functionality #93

Merged
merged 3 commits into from
Apr 6, 2024

Conversation

robberwick
Copy link
Contributor

@robberwick robberwick commented Apr 6, 2024

Adds first cut at a test suite for the SerialTransfer and CRC classes. It only provides ~85% coverage in total (~90% for CRC.py and 84% for pySerialTransfer.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 as msb, 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, and pytest-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 just pytest, or with coverage enabled with pytest --cov .\pySerialTransfer

Addresses #89

@PowerBroker2
Copy link
Owner

This is a dumb question, but did you run the tests and everything checked out?

@robberwick
Copy link
Contributor Author

robberwick commented Apr 6, 2024

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

  • they're both skipped as the 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.

@PowerBroker2
Copy link
Owner

Ok, thanks!

@PowerBroker2 PowerBroker2 merged commit d29d5ed into PowerBroker2:master Apr 6, 2024
@robberwick robberwick deleted the pytest branch April 15, 2024 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants