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

fix: update tests.py to pass, add to github workflow #576

Merged
merged 2 commits into from
Jan 6, 2025

Conversation

tringenbach
Copy link
Contributor

As I mentioned on my other PR, I noticed that tests.py was failing, looks like the code under test changed out from under it over time. This gets it all passing again. I added it to the github workflow, so that hopefully they can be more useful.

For the colour and white light tests, the values changed. I am assuming the new ones are correct, just for a different variant. I left the old ones commented out.

I reworked how the tests use mocks. I change them to use return_value instead of side_effect, and to grab the "results" (really, what the library wanted to send) from .call_args. That allowed me to eliminate a separate mock function for each test.

The also now mostly deals with Python dicts instead of decoding the binary format. That is mainly because of how _send_receive works. (I assume it used to not do that decoding, but I didn't really dig into the history).

I also added section comments, the standard-ish "arrange" / "act" / "assert" comments plus "gather results" and "expectations". I can remove those if you don't like them of course. Personally I like to at least have an "act" or "execute" comment in my unit tests, because I find that line, which is the important thing-you're-actually-testing line, often gets lost among all the setup code.

@jasonacox jasonacox merged commit 0f9ad3f into jasonacox:master Jan 6, 2025
16 checks passed
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