test: improve test-cluster-disconnect-suicide-race by Trott · Pull Request #4739 · nodejs/node

@Trott

Previously, test-cluster-disconnect-suicide-race had two issues:

* Magic numbers: How many times to spawn a worker was determined through
empirical experimentation. This means that as new platforms and new
CPU/RAM configurations are tested, the magic numbers require more
and more refinement. This brings us to...

* Non-determinism: The test seems to fail all the time when the bug
it tests for is present, but it's really a judgment based on sampling.
"Oh, with 8 workers per CPU, it fails about 80% of the time. Let's try
16..."

This revised version of the test takes a different approach. The fix
for the bug that the test was written for means that the disconnect
event will fire on a subsequent tick. So we check for that and the test
still fails when the fix is not in the code base and succeeds when it
is.

Advantages of this approach include:

* The test runs much faster.
* The test should be reliable on any new platform regardless of CPU and
RAM.

Ref: nodejs#4674

cc @santigimeno @iwuzhere

@Trott

Trott added a commit that referenced this pull request

Jan 19, 2016
Previously, test-cluster-disconnect-suicide-race had two issues:

* Magic numbers: How many times to spawn a worker was determined through
empirical experimentation. This means that as new platforms and new
CPU/RAM configurations are tested, the magic numbers require more
and more refinement. This brings us to...

* Non-determinism: The test seems to fail all the time when the bug
it tests for is present, but it's really a judgment based on sampling.
"Oh, with 8 workers per CPU, it fails about 80% of the time. Let's try
16..."

This revised version of the test takes a different approach. The fix
for the bug that the test was written for means that the disconnect
event will fire on a subsequent tick. So we check for that and the test
still fails when the fix is not in the code base and succeeds when it
is.

Advantages of this approach include:

* The test runs much faster.
* The test should be reliable on any new platform regardless of CPU and
RAM.

PR-URL: #4739
Ref: #4674
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

@Trott Trott mentioned this pull request

Jan 20, 2016

Trott added a commit to Trott/io.js that referenced this pull request

Jan 20, 2016
Two cluster tests have recently changed so that they are no longer
resource intensive. Move them back to parallel.

Ref: nodejs#4736
Ref: nodejs#4739

Trott added a commit to Trott/io.js that referenced this pull request

Jan 21, 2016
Two cluster tests have recently changed so that they are no longer
resource intensive. Move them back to parallel.

Ref: nodejs#4736
Ref: nodejs#4739
PR-URL: nodejs#4774
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

rvagg pushed a commit that referenced this pull request

Jan 25, 2016
Previously, test-cluster-disconnect-suicide-race had two issues:

* Magic numbers: How many times to spawn a worker was determined through
empirical experimentation. This means that as new platforms and new
CPU/RAM configurations are tested, the magic numbers require more
and more refinement. This brings us to...

* Non-determinism: The test seems to fail all the time when the bug
it tests for is present, but it's really a judgment based on sampling.
"Oh, with 8 workers per CPU, it fails about 80% of the time. Let's try
16..."

This revised version of the test takes a different approach. The fix
for the bug that the test was written for means that the disconnect
event will fire on a subsequent tick. So we check for that and the test
still fails when the fix is not in the code base and succeeds when it
is.

Advantages of this approach include:

* The test runs much faster.
* The test should be reliable on any new platform regardless of CPU and
RAM.

PR-URL: #4739
Ref: #4674
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

rvagg pushed a commit that referenced this pull request

Jan 25, 2016
Two cluster tests have recently changed so that they are no longer
resource intensive. Move them back to parallel.

Ref: #4736
Ref: #4739
PR-URL: #4774
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

MylesBorins pushed a commit that referenced this pull request

Jan 28, 2016
Previously, test-cluster-disconnect-suicide-race had two issues:

* Magic numbers: How many times to spawn a worker was determined through
empirical experimentation. This means that as new platforms and new
CPU/RAM configurations are tested, the magic numbers require more
and more refinement. This brings us to...

* Non-determinism: The test seems to fail all the time when the bug
it tests for is present, but it's really a judgment based on sampling.
"Oh, with 8 workers per CPU, it fails about 80% of the time. Let's try
16..."

This revised version of the test takes a different approach. The fix
for the bug that the test was written for means that the disconnect
event will fire on a subsequent tick. So we check for that and the test
still fails when the fix is not in the code base and succeeds when it
is.

Advantages of this approach include:

* The test runs much faster.
* The test should be reliable on any new platform regardless of CPU and
RAM.

PR-URL: #4739
Ref: #4674
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

@rvagg rvagg mentioned this pull request

Feb 8, 2016

MylesBorins pushed a commit that referenced this pull request

Feb 11, 2016
Previously, test-cluster-disconnect-suicide-race had two issues:

* Magic numbers: How many times to spawn a worker was determined through
empirical experimentation. This means that as new platforms and new
CPU/RAM configurations are tested, the magic numbers require more
and more refinement. This brings us to...

* Non-determinism: The test seems to fail all the time when the bug
it tests for is present, but it's really a judgment based on sampling.
"Oh, with 8 workers per CPU, it fails about 80% of the time. Let's try
16..."

This revised version of the test takes a different approach. The fix
for the bug that the test was written for means that the disconnect
event will fire on a subsequent tick. So we check for that and the test
still fails when the fix is not in the code base and succeeds when it
is.

Advantages of this approach include:

* The test runs much faster.
* The test should be reliable on any new platform regardless of CPU and
RAM.

PR-URL: #4739
Ref: #4674
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit to MylesBorins/node that referenced this pull request

Feb 11, 2016
Previously, test-cluster-disconnect-suicide-race had two issues:

* Magic numbers: How many times to spawn a worker was determined through
empirical experimentation. This means that as new platforms and new
CPU/RAM configurations are tested, the magic numbers require more
and more refinement. This brings us to...

* Non-determinism: The test seems to fail all the time when the bug
it tests for is present, but it's really a judgment based on sampling.
"Oh, with 8 workers per CPU, it fails about 80% of the time. Let's try
16..."

This revised version of the test takes a different approach. The fix
for the bug that the test was written for means that the disconnect
event will fire on a subsequent tick. So we check for that and the test
still fails when the fix is not in the code base and succeeds when it
is.

Advantages of this approach include:

* The test runs much faster.
* The test should be reliable on any new platform regardless of CPU and
RAM.

PR-URL: nodejs#4739
Ref: nodejs#4674
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit to MylesBorins/node that referenced this pull request

Feb 13, 2016
Previously, test-cluster-disconnect-suicide-race had two issues:

* Magic numbers: How many times to spawn a worker was determined through
empirical experimentation. This means that as new platforms and new
CPU/RAM configurations are tested, the magic numbers require more
and more refinement. This brings us to...

* Non-determinism: The test seems to fail all the time when the bug
it tests for is present, but it's really a judgment based on sampling.
"Oh, with 8 workers per CPU, it fails about 80% of the time. Let's try
16..."

This revised version of the test takes a different approach. The fix
for the bug that the test was written for means that the disconnect
event will fire on a subsequent tick. So we check for that and the test
still fails when the fix is not in the code base and succeeds when it
is.

Advantages of this approach include:

* The test runs much faster.
* The test should be reliable on any new platform regardless of CPU and
RAM.

PR-URL: nodejs#4739
Ref: nodejs#4674
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit to MylesBorins/node that referenced this pull request

Feb 15, 2016
Previously, test-cluster-disconnect-suicide-race had two issues:

* Magic numbers: How many times to spawn a worker was determined through
empirical experimentation. This means that as new platforms and new
CPU/RAM configurations are tested, the magic numbers require more
and more refinement. This brings us to...

* Non-determinism: The test seems to fail all the time when the bug
it tests for is present, but it's really a judgment based on sampling.
"Oh, with 8 workers per CPU, it fails about 80% of the time. Let's try
16..."

This revised version of the test takes a different approach. The fix
for the bug that the test was written for means that the disconnect
event will fire on a subsequent tick. So we check for that and the test
still fails when the fix is not in the code base and succeeds when it
is.

Advantages of this approach include:

* The test runs much faster.
* The test should be reliable on any new platform regardless of CPU and
RAM.

PR-URL: nodejs#4739
Ref: nodejs#4674
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit that referenced this pull request

Mar 17, 2016
Two cluster tests have recently changed so that they are no longer
resource intensive. Move them back to parallel.

Ref: #4736
Ref: #4739
PR-URL: #4774
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

MylesBorins pushed a commit that referenced this pull request

Mar 21, 2016
Two cluster tests have recently changed so that they are no longer
resource intensive. Move them back to parallel.

Ref: #4736
Ref: #4739
PR-URL: #4774
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

scovetta pushed a commit to scovetta/node that referenced this pull request

Apr 2, 2016
Previously, test-cluster-disconnect-suicide-race had two issues:

* Magic numbers: How many times to spawn a worker was determined through
empirical experimentation. This means that as new platforms and new
CPU/RAM configurations are tested, the magic numbers require more
and more refinement. This brings us to...

* Non-determinism: The test seems to fail all the time when the bug
it tests for is present, but it's really a judgment based on sampling.
"Oh, with 8 workers per CPU, it fails about 80% of the time. Let's try
16..."

This revised version of the test takes a different approach. The fix
for the bug that the test was written for means that the disconnect
event will fire on a subsequent tick. So we check for that and the test
still fails when the fix is not in the code base and succeeds when it
is.

Advantages of this approach include:

* The test runs much faster.
* The test should be reliable on any new platform regardless of CPU and
RAM.

PR-URL: nodejs#4739
Ref: nodejs#4674
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

scovetta pushed a commit to scovetta/node that referenced this pull request

Apr 2, 2016
Two cluster tests have recently changed so that they are no longer
resource intensive. Move them back to parallel.

Ref: nodejs#4736
Ref: nodejs#4739
PR-URL: nodejs#4774
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

@Trott Trott deleted the improve-race-test branch

January 13, 2022 22:31