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

Add unit tests for RandomNumberGenerator #44560

Merged
merged 1 commit into from
Dec 21, 2020
Merged

Add unit tests for RandomNumberGenerator #44560

merged 1 commit into from
Dec 21, 2020

Conversation

Xrayez
Copy link
Contributor

@Xrayez Xrayez commented Dec 21, 2020

Salvages #44350 (I added @vinayakmtiwari as co-author but apparently the account is deleted so I don't know... The author is encouraged to get back!)

This covers RNG functionality completely (added randf and randi tests as well).

"Integer 32 bit" test is a sanity check to make sure that we do generate 32 unsigned integers given the same test seed.

The problem whether some tests may fail at some point (such as "Normal distribution" case) is handled by doctest's may_fail decorator, for which I added TEST_CASE_MAY_FAIL test macro in tests/test_macros.h to be used by other test suites abstracting away implementation detail behind doctest, so such test cases won't block CI, and if they do fail (I believe this won't happen as long as we don't change RNG implementation often, I hope), it's a matter of updating the seed, because current results are deterministic (but only per platform, not necessarily across platforms).

This covers RNG functionality completely.

Co-authored-by: @vinayakmtiwari.
@Xrayez Xrayez changed the title Add unit test for RandomNumberGenerator Add unit tests for RandomNumberGenerator Dec 21, 2020
@Calinou Calinou added this to the 4.0 milestone Dec 21, 2020
@akien-mga akien-mga merged commit 0f4a23b into godotengine:master Dec 21, 2020
@akien-mga
Copy link
Member

Thanks!

@Xrayez Xrayez deleted the rng-tests branch December 21, 2020 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants