test: fix flakiness of stringbytes-external#6039
test: fix flakiness of stringbytes-external#6039ofrobots wants to merge 1 commit intonodejs:masterfrom
Conversation
9f7fe64 to
7ae1e9c
Compare
The tests used to rely on precise timing of when a JavaScript object would be garbage collected to ensure that there is enough memory available on the system. Switch the test to use a malloc/free pair instead. Ref: nodejs#5945
7ae1e9c to
4e6f03c
Compare
|
Interesting. I believe all the other tests in |
|
Indeed, although my interpretation of An uglier alternative would be to expose the |
|
This is certainly an interesting case. Overall it LGTM it's just definitely quite a bit different from our other tests. |
|
LGTM |
|
Would perhaps the cctest suite be a better fit? |
|
Adding a new suite for this is probably overkill. I don't think this pattern is going to be very common to require a suite. I would like to land this PR today as it is blocking #5945 which needs to land very soon in order to get ready for the Node v6 RC schedule. |
|
LGTM. After it lands, we can update the README in the test directory to call out this particular test. |
The tests used to rely on precise timing of when a JavaScript object would be garbage collected to ensure that there is enough memory available on the system. Switch the test to use a malloc/free pair instead. Ref: nodejs#5945 PR-URL: nodejs#6039 Reviewed-By: jasnell - James M Snell <jasnell@gmail.com> Reviewed-By: evanlucas - Evan Lucas <evanlucas@me.com> Reviewed-By: Trott - Rich Trott <rtrott@gmail.com>
|
Thanks. Landed as f4ebd59. I will update the README in a follow-on. |
|
Closing, as the changes have landed already. |
|
Follow-on: #6073. |
Avoid depending on precise timing of when an object will be collected by GC. This test was missed by nodejs#6039 as it happened to be in a different directory than the rest. Ref: nodejs#6039 PR-URL: nodejs#6073 Reviewed-By: Trott - Rich Trott <rtrott@gmail.com>
|
@ofrobots this is not merging cleanly onto v5.x, would you be able to backport? |
|
since this is not landing on v5.x I'm marking as We will still want this backported to |
The tests used to rely on precise timing of when a JavaScript object would be garbage collected to ensure that there is enough memory available on the system. Switch the test to use a malloc/free pair instead. Ref: nodejs#5945 Ref: nodejs#6039 Ref: nodejs#6073
The tests used to rely on precise timing of when a JavaScript object would be garbage collected to ensure that there is enough memory available on the system. Switch the test to use a malloc/free pair instead. Ref: #5945 Ref: #6039 Ref: #6073 PR-URL: #6705 Reviewed-By: James M Snell <jasnell@gmail.com>
The tests used to rely on precise timing of when a JavaScript object would be garbage collected to ensure that there is enough memory available on the system. Switch the test to use a malloc/free pair instead. Ref: #5945 Ref: #6039 Ref: #6073 PR-URL: #6705 Reviewed-By: James M Snell <jasnell@gmail.com>
Pull Request check-list
make -j8 test(UNIX) orvcbuild test nosign(Windows) pass withthis change (including linting)?
test (or a benchmark) included?
Affected core subsystem(s)
test
Description of change
The tests used to rely on precise timing of when a JavaScript object
would be garbage collected to ensure that there is enough memory
available on the system. Switch the test to use a malloc/free pair
instead.
Ref: #5945
R=@bnoordhuis, @Trott
CI: https://ci.nodejs.org/job/node-test-pull-request/2146/CI: https://ci.nodejs.org/job/node-test-pull-request/2147/CI: https://ci.nodejs.org/job/node-test-pull-request/2148/