add method to reset the histogram #10
Conversation
|
|
||
| test('reset histogram', (t) => { | ||
| const instance = new Histogram(1, 100) | ||
| t.equal(instance.min(), 9223372036854776000, 'min is setup') |
There was a problem hiding this comment.
@mcollina I discovered this tidbit when writing my tests, I have recreated it several times. should this be acceptable? its an empty instance.
There was a problem hiding this comment.
Not really.
However it is a bug in the C library. We might open a separate bug there.
There was a problem hiding this comment.
I've done some research on this, I don't believe it the issue we thought it. The min value should be initialised to positive infinity, and max value to negative infinity. It means the check for greater than or less than values are not complex logic trees, and you just use the single variables from initialisation in implementation.
There was a problem hiding this comment.
& relevant issue: HdrHistogram/HdrHistogram#54
| @@ -7,7 +7,7 @@ | |||
| "scripts": { | |||
| "prepublish": "npm ls", | |||
| "install": "node-pre-gyp install --fallback-to-build", | |||
| "test": "node-pre-gyp install --fallback-to-build && standard && tap test.js" | |||
| "test": "node-pre-gyp install --build-from-source && standard && tap test.js" | |||
There was a problem hiding this comment.
Changed this command to ensure when running tests it always rebuilds. Previously it used any existing build, which wasn't optimal in dev.
| NAN_METHOD(HdrHistogramWrap::Reset) { | ||
| HdrHistogramWrap* obj = Nan::ObjectWrap::Unwrap<HdrHistogramWrap>(info.This()); | ||
| hdr_reset(obj->histogram); | ||
| info.GetReturnValue().Set(true); |
There was a problem hiding this comment.
@mcollina Wasn't sure if I should or should not return here.
There was a problem hiding this comment.
IMHO we should return the histogram, or undefined.
There was a problem hiding this comment.
I'll return the histogram.
|
|
||
| test('reset histogram', (t) => { | ||
| const instance = new Histogram(1, 100) | ||
| t.equal(instance.min(), 9223372036854776000, 'min is setup') |
There was a problem hiding this comment.
Not really.
However it is a bug in the C library. We might open a separate bug there.
| NAN_METHOD(HdrHistogramWrap::Reset) { | ||
| HdrHistogramWrap* obj = Nan::ObjectWrap::Unwrap<HdrHistogramWrap>(info.This()); | ||
| hdr_reset(obj->histogram); | ||
| info.GetReturnValue().Set(true); |
There was a problem hiding this comment.
IMHO we should return the histogram, or undefined.
This is so I can work on something for mcollina/autocannon#92