Skip to content

Test of uniformity of BN_rand_range output.#8830

Closed
paulidale wants to merge 3 commits intoopenssl:masterfrom
paulidale:bnrand_chisq
Closed

Test of uniformity of BN_rand_range output.#8830
paulidale wants to merge 3 commits intoopenssl:masterfrom
paulidale:bnrand_chisq

Conversation

@paulidale
Copy link
Contributor

Rework the test so that it fails far less often.

A number of independent tests are executed and 5% are expected to fail.
The number of such failures follows a binomial distribution which permits
a statistical test a 0.01% expected failure rate.

An alternative fix to #8826

  • tests are added or updated

@paulidale
Copy link
Contributor Author

Thoughts @kroeckx ?

@paulidale paulidale mentioned this pull request Apr 26, 2019
@kroeckx
Copy link
Member

kroeckx commented Apr 26, 2019

I think such tests can be useful, but since they can fail when ran enough time, and it will be ran enough times, we should not run them by default.

@paulidale
Copy link
Contributor Author

Okay, a command line option to the test program to turn them on seems prudent.

@paulidale
Copy link
Contributor Author

paulidale commented Apr 27, 2019

The bntest program now takes the argument -stochastic which enables the extra testing.
It is disabled by default.

@paulidale
Copy link
Contributor Author

Review @openssl/committers ?

@paulidale
Copy link
Contributor Author

Could someone please review this? Thanks.

Rework the test so that it fails far less often.

A number of independent tests are executed and 5% are expected to fail.
The number of such failures follows a binomial distribution which permits
a statistical test a 0.01% expected failure rate.

There is a command line option to enable the stochastic range checking.
It is off by default.
@paulidale
Copy link
Contributor Author

AppVeyor failure isn't relevant.

@paulidale
Copy link
Contributor Author

Ping @mattcaswell

@paulidale
Copy link
Contributor Author

Ping @openssl/committers

@paulidale
Copy link
Contributor Author

Still waiting review/confirmation.

@mattcaswell mattcaswell added the approval: done This pull request has the required number of approvals label May 28, 2019
@paulidale
Copy link
Contributor Author

Merged, thanks.

@paulidale paulidale closed this May 28, 2019
@paulidale paulidale deleted the bnrand_chisq branch May 28, 2019 23:55
levitte pushed a commit that referenced this pull request May 28, 2019
Rework the test so that it fails far less often.

A number of independent tests are executed and 5% are expected to fail.
The number of such failures follows a binomial distribution which permits
a statistical test a 0.01% expected failure rate.

There is a command line option to enable the stochastic range checking.
It is off by default.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #8830)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: done This pull request has the required number of approvals

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants