fix: update go-test#1187
Conversation
Replace all use of `math/rand` with `math/rand/v2`
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #1187 +/- ##
==========================================
- Coverage 63.87% 63.84% -0.03%
==========================================
Files 269 269
Lines 27078 27073 -5
==========================================
- Hits 17297 17286 -11
- Misses 8073 8076 +3
- Partials 1708 1711 +3
... and 11 files with indirect coverage changes 🚀 New features to boost your workflow:
|
The bytehash table in chunker/buzhash.go was generated with math/rand seeded by NewSource(0). Switching gen to math/rand/v2 made it print a different table, so re-running it and pasting the output would have changed buzhash chunk boundaries, and with them the CID of anything chunked with buzhash. Pin gen back to the old source and say why.
The remote blockstore, CAR fetcher, and value store each held their own *rand.Rand and used it to pick a URL on every request. A *rand.Rand cannot be used from more than one goroutine, so a gateway serving requests in parallel was racing. Use the top-level math/rand/v2 functions instead, which are safe to share, and drop the seeding code.
Record the removal of util.NewSeededRand and NewTimeSeededRand, which were exported, and the gateway race fix. Fix two typos in the go-test entry and say what dropping the global seed is good for. Note on sharedRNG that a random.Random cannot be shared between goroutines.
lidel
left a comment
lidel
left a comment
There was a problem hiding this comment.
Thanks for this, removes a whole class of cross-test interference. I pushed three commits on top, CI is green:
-
c7a2307 fix(chunker): keep gen reproducing the committed table
On rand/v2,chunker/genprinted a different table than thebytehashcommitted next to it, so (iiuc) re-running it would have moved buzhash chunk boundaries and changed CIDs. Pinned back toNewSource(0), just to avoid floating the boat. Lmk if I mistunderstood this. -
3b10577 fix(gateway): data race when picking a gateway url
The remote blockstore, CAR fetcher, and value store shared a*rand.Randacross request goroutines, which isn't safe. Top-levelmath/rand/v2is, and the seeding code goes away.
LGTM, once this lands, ipfs/kubo#11390 needs a re-pin from boxo main
2 participants
🧪 Tests use new
go-testandmath/rand/v2The new
go-testis upgraded to usemath/rand/v2in all of its packages. Thego-test/randompackage now allows reuse of the random number generator for more efficiently generating sets of random values.Additionally, the
go-test/randompackage removes support for a global seed for the random number generator. This led to the possibility of multiple tests interferring with each other when generating deterministic values. For the same reason, support for a global sequence was removed. This breakage could be intermittent and difficult to debug, depending on the how/which tests ran at the same time.Since the underlying pseudo-random number generator was changed in the go-test module, the data generated for a given seed also changed. This required updating tests that relied on seeding the generator and getting expected values.
go-test/randomAPI/math/rand' to/math/rand/v2`[32]byteand notuint64StringToSeedorUint64ToSeedto create random seeds for deterministic output