Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use deterministic randomness in arbitrary tests #1120

Merged
merged 2 commits into from Oct 24, 2023

Conversation

brson
Copy link
Contributor

@brson brson commented Oct 24, 2023

What

Use a seeded StdRng instead of thread_rng in arbitrary tests.

Why

Per stellar/rs-soroban-env#810 randomized test cases should be deterministic.

Known limitations

This is different from the approach in soroban-env-host from stellar/rs-soroban-env#1124, where a test prng is attached to the env / host.

There are two more uses of thread_rng in this crate, but I don't know enough about that code to know how they should be removed, i.e. if they should follow a pattern similar to soroban-env-host. I am willing to replace all uses of thread_rng if given guidance about the preferred way to do it.

The implementation of StdRng is allowed to change in the future, so it's possible the exact numbers generated could change. I could import rand_chacha instead if desired.

@brson
Copy link
Contributor Author

brson commented Oct 24, 2023

The CI failure is strange.

It looks like a change in rust nightly linting behavior, and can be reproduced with

cargo +nightly check -p soroban-sdk --features testutils

Possibly rust-lang/rust#117120 and rust-lang/rust#116033.

Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@leighmcculloch
Copy link
Member

Thanks for sharing the command to minimally reproduce it. That helped. I've opened this to fix the lint warnings in nightly:

github-merge-queue bot pushed a commit that referenced this pull request Oct 24, 2023
### What
Fix nightly lints on unused internal exports

### Why

#1120 (comment)
@leighmcculloch leighmcculloch added this pull request to the merge queue Oct 24, 2023
Merged via the queue into stellar:main with commit 9850278 Oct 24, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants