Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1758)

Issue 7306066: Add Random unit tests (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 10 months ago by JimVV
Modified:
11 years, 10 months ago
Reviewers:
Humper, bsalomon, reed1
CC:
skia-review_googlegroups.com
Base URL:
http://skia.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Add Random unit tests. This adds a variety of unit tests for randomness. Most are straightforward. The gorilla test is for checking how random the individual bit places are. The suite only tests SkMWCRandom because SkRandom fails a number of them, in particular the uniform byte test, the gorilla test and the range test. Suggestions for more tests are welcome.

Patch Set 1 #

Patch Set 2 : Faster normal CDF function; added assert for range check #

Unified diffs Side-by-side diffs Delta from patch set Stats (+197 lines, -0 lines) Patch
M gyp/tests.gyp View 1 1 chunk +1 line, -0 lines 0 comments Download
A tests/RandomTest.cpp View 1 1 chunk +196 lines, -0 lines 0 comments Download

Messages

Total messages: 6
JimVV
11 years, 10 months ago (2013-02-07 22:04:07 UTC) #1
Humper
This looks great to me (lGtm?). Important stuff, thanks.
11 years, 10 months ago (2013-02-08 15:03:52 UTC) #2
bsalomon
On 2013/02/08 15:03:52, Humper wrote: > This looks great to me (lGtm?). Important stuff, thanks. ...
11 years, 10 months ago (2013-02-08 15:04:20 UTC) #3
JimVV
On 2013/02/08 15:04:20, bsalomon wrote: > On 2013/02/08 15:03:52, Humper wrote: > > This looks ...
11 years, 10 months ago (2013-02-08 16:38:29 UTC) #4
bsalomon
On 2013/02/08 16:38:29, JimVV wrote: > On 2013/02/08 15:04:20, bsalomon wrote: > > On 2013/02/08 ...
11 years, 10 months ago (2013-02-08 17:09:11 UTC) #5
JimVV
11 years, 10 months ago (2013-02-08 17:13:58 UTC) #6
On 2013/02/08 17:09:11, bsalomon wrote:
> On 2013/02/08 16:38:29, JimVV wrote:
> > On 2013/02/08 15:04:20, bsalomon wrote:
> > > On 2013/02/08 15:03:52, Humper wrote:
> > > > This looks great to me (lGtm?).  Important stuff, thanks.
> > > 
> > > lgtm, too.
> > 
> > Sorry, a couple of small updates. I changed the normal distribution CDF out
> for
> > a cheaper one and added an assert to ensure that our nextRangeU values are
> > actually in range.
> 
> lgtm

Committed as r7670.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b