search

jchambers / java-otp

A one-time password (HOTP/TOTP) library for Java
MIT License
455 stars 122 forks source link

Use `assumeTrue` when running SHA-512-based tests #33

Closed jchambers closed 2 years ago

jchambers commented 2 years ago

Most JREs support SHA-512, but not all do. This change adds an assumeTrue call to tests that makes sure the algorithm in question is supported by the JRE. If the JRE does not support SHA-512, the test will abort rather than failing. This should allow folks to build/test java-otp even if their immediate JDK doesn't support SHA-512.

As always, feedback is very welcome!

jchambers commented 2 years ago

I've gone back and forth on this a few times now, but at the end of the day, I think it's the right move. I figured I'd share my thought process here for posterity.

My main hesitation has been that, if somebody tries to run these tests on a system that doesn't support HMAC-SHA-512, we might be creating a false sense of "correctness" by preventing those tests from failing loudly. That could be doubly bad if some change actually breaks functionality with SHA-512 and then tests still semi-pass because the SHA-512 tests didn't run at all.

…at the same time, that's a little bit of a Michael Bay scenario. If we're being real, I'm the only one who's actually working on java-otp right now, it's very unlikely that I'm going to somehow break things with SHA-512 without also breaking things with the other HMAC algorithms, and—perhaps most to the point—every system I have access to does support SHA-512.

And then moving into the abstract/philosophical space, what does it mean for a test to fail? Reasonable minds can (and do) disagree on this point, but I think it's helpful to have test failures indicate that something is actually wrong with the code being tested rather than the system on which the code is running. With these changes, if somebody runs the tests on a system that doesn't support SHA-512, tests will have three possible outcomes: definitively passing, definitively failing, or "didn't run and don't know." The humans running these tests can interpret those results and make judgements accordingly.

At the end of the day, this works out to be a courtesy for folks who might poke at java-otp on systems unlike my own without introducing any real downsides for mainline development.