numbers / numbers.js

Advanced Mathematics Library for Node.js and JavaScript
Apache License 2.0
1.77k stars 167 forks source link

Rewrite tests from random.distribution.* #116

Closed LarryBattle closed 9 years ago

LarryBattle commented 9 years ago

The test cases for random.distribution.* fail 15% of the time. This is causing working builds to randomly fail in Travis CI. Thus a rewrite of the test cases is needed. Unfortunately I need to read up on this subject matter before I can proceed with a rewrite.

Example of a failed Travis CI build

Error Message:

  ✖ 1 of 128 tests failed:
  1) numbers random.distribution.irwinHall should return a normal distribution of length n within bounds of (m/2 - sub, m/2):
     AssertionError: Math.abs(49.31523224141216 - 50) < 0.5
      at Object.testing.approxEquals (/home/travis/build/sjkaliski/numbers.js/test/testing.js:14:10)
      at Context.<anonymous> (/home/travis/build/sjkaliski/numbers.js/test/random.test.js:156:13)
      at Test.Runnable.run (/home/travis/build/sjkaliski/numbers.js/node_modules/mocha/lib/runnable.js:196:15)
      at Runner.runTest (/home/travis/build/sjkaliski/numbers.js/node_modules/mocha/lib/runner.js:344:10)
      at /home/travis/build/sjkaliski/numbers.js/node_modules/mocha/lib/runner.js:390:12
      at next (/home/travis/build/sjkaliski/numbers.js/node_modules/mocha/lib/runner.js:270:14)
      at /home/travis/build/sjkaliski/numbers.js/node_modules/mocha/lib/runner.js:279:7
      at next (/home/travis/build/sjkaliski/numbers.js/node_modules/mocha/lib/runner.js:227:23)
      at Object._onImmediate (/home/travis/build/sjkaliski/numbers.js/node_modules/mocha/lib/runner.js:247:5)
      at processImmediate [as _immediateCallback] (timers.js:345:15)
make: *** [test] Error 1
npm ERR! Test failed.  See above for more details.

Source: https://travis-ci.org/sjkaliski/numbers.js/jobs/35846620

Dakkers commented 9 years ago

I'll hopefully be working on this in the next while

Dakkers commented 9 years ago

so many other tests are failing. I thought it was just that one, it seemed to have been giving us the most problems but apparently that's not the case. I'm just going to skew the error bounds on the tests so that it has a better chance of passing. it may be hacky but random numbers are wonky so whatever.

LarryBattle commented 9 years ago

I tried the latest code from master with your changes but it didn't really help to lower the error rate.

I think this is the real problem. In order for the random functions to be testable, a predetermined seed value for the pseudo random number generator must be used. Currently, there's no way to achieve this.

Here's one possible solution. In the random module, add a setter for the random number generator instead of hard coding to Math.random().

I see two benefits here. 1) Users could replace Math.random() with a cryptographic-grade random number generator. 2) Test cases would become predictable. During testing, we could use an alternative to the plain old Math.random(). Possible options:

I'll try to submit a pull request soon.