inky icon indicating copy to clipboard operation
inky copied to clipboard

Clean up IO tests to use hex or constants in comparisons

Open lawik opened this issue 5 years ago • 4 comments

Currently we have things like:

assert get_border_command() == [send_command: {60, 51}]

While the command being sent is provided in hex:

write_command(state, 0x3C, border_data)

0x3C is output as 60 and that's why things are this way right now. We should probably change it so that the connection remains clear.

lawik avatar Jul 26 '19 11:07 lawik

This would benefit from #30

lawik avatar Jul 26 '19 11:07 lawik

How would this benefit from #30?

I'd say we want numbers to remain literals and not references in our tests. If our tests and production code refer to the same values, we are not testing that the right values are being passed to the devices, making the tests weaker.

Being consistent in how we represent our integers is all fine and well, but going beyond that doesn't have clear benefits to me.

nyaray avatar Jul 26 '19 11:07 nyaray

Okay, that's fair. I agree that literals are probably the way to go. But I think module attributes in the test suite would be beneficial to clarity and reduce the risk of a hexadecimal number being replaced because it is identical but used for a different purpose in a different context. I think that risk is reduced with using module attributes in the test module as opposed to repeating the literals in every test.

What do you think about that approach?

lawik avatar Jul 26 '19 11:07 lawik

Sounds good!

nyaray avatar Jul 26 '19 12:07 nyaray