[Go] Add test coverage for the rand package#427
Open
tauptlab wants to merge 1 commit into
Open
Conversation
Closes the TODO that asked for tests of the exported noise-generating functions. Includes deterministic byte-level tests, range invariants, 5-sigma moment checks, and a concurrent stress test. Also surfaces two pre-existing edge cases while writing tests. I63n doesn't validate non-positive n - will fix in a follow-up. randSource.Int63 returns a negative value once per 2^64 calls on byte stream 0x8000_0000_0000_0000 - documented as a Note since any fix changes the output mapping for many byte sequences.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the TODO at the top of
rand.goasking for test coverage of the exported noise-generating functions. The package only had a test forBoolean's bit-shifting before this.The tests are in three layers:
randBuffor abytes.NewReaderand check exact outputs. The targeted edge cases live here:I63n's rejection loop at(MaxInt64/n)*n, the sign-bit mask,Geometric's leading-zero counting, andUniform'sr==0fallback. The fallback can't be reached fromcrypto/randat any realistic call count, but it is reachable by feeding 128 zero bytes (forcesGeometric() >= 1024somath.Pow(2,k)overflows to+Inf).mu_4 == sigma^4.TestNormal_FourthMomentis a separate explicit check becauseNormalandLaplaceat the same variance both pass the regular moment test - they only differ atmu_4(3 vs 6). The library uses both for the Gaussian and Laplace mechanisms.TestConcurrentstresses the package locks with 8 goroutines, plus a Boolean mean check so it has signal even without-race.testing.Short()skips the moment tests.rand.gorandBufsaying it must wrapcrypto/rand.Two pre-existing edge cases noticed while writing the tests:
I63ndoesn't validaten.n=0panics with divide-by-zero,n<0returns values in[0, |n|)instead of failing. Added a TODO; I'll send a follow-up PR.randSource.Int63returnsmath.MinInt64once per 2^64 calls on the byte stream0x8000000000000000(since-math.MinInt64 == math.MinInt64in two's complement). Added aNotein code rather than fixing - any fix would change the output mapping for ~half of all byte sequences, which seems disproportionate for a 2^-64 contract violation that's never been observed.Testing
go test ./go/...clean.-count=10clean.-shortcorrectly skips the moment tests.-racelocally - nogccon this Windows machine.