Fix the panic in the TestGenerateRandomBytes test #183 #184
Conversation
helpers_test.go
Outdated
| // panic here to simulate the panic in the rand.Read method. | ||
| // The rand.Read method returns a rand_fatal which cannot be recovered from. | ||
| panic("crypto/rand: failed to read random data (see https://go.dev/issue/66821)") |
There was a problem hiding this comment.
crypo/rand.Read doesn't panic, as you note a failure there "...cannot be recovered from" (the difference being panic-ing is recoverable). Replacing that with a panic here is not accurate.
There was a problem hiding this comment.
Agreed, I changed the logic to run the test that throws a fatal error inside a sub-process, make sure we get an error. Let me know what you think about that? Thanks!
There was a problem hiding this comment.
Agreed, I changed the logic to run the test that throws a fatal error inside a sub-process, make sure we get an error. Let me know what you think about that? Thanks!
I'm not sure it makes sense to run this test for Go 1.24+: the description of the test is:
TestGenerateRandomBytes tests the (extremely rare) case that crypto/rand does not return the expected number of bytes.
However, crypto/rand.Read from Go 1.24 onwards will:
Read fills b with cryptographically secure random bytes. It never returns an error, and always fills b entirely.
So the test case isn't really valid any more.
sudhanvaghebbale
force-pushed
the
shebbale/bug-183-fix-test
branch
from
a32a244 to
6b93cd7
Compare
sudhanvaghebbale
force-pushed
the
shebbale/bug-183-fix-test
branch
from
6b93cd7 to
15cd090
Compare
Issue:
From Go 1.24 onwards, crypto/rand.Read will do the following:
So the test case isn't really valid any more.
Fix:
Remove the test from the code.