[Tracking issue] Converting tests to use Countdown

[jasnell]

One possible set of tasks that I can suggest for new contributors who are bit more confident in their Node.js skills, would be converting tests to use the new ../common/countdown utility module.

For instance, if you take a look at: https://github.com/nodejs/node/blob/master/test/parallel/test-http2-client-destroy.js, you'll see that there is a remaining counter (https://github.com/nodejs/node/blob/master/test/parallel/test-http2-client-destroy.js#L19). The test then counts down remaining before closing the server. There are quite a large number of tests in our suite that perform similar actions in ways that are rather inconsistent. The ../common/countdown utility was designed to bring some consistency there.

The way the Countdown utility works is straightforward:

const common = require('../common');
const Countdown = require('../common/countdown');

// ...

const countdown = new Countdown(n, common.mustCall(() => {
  // do something here
}));

// Decrement the counter, the callback is called synchronously when
// countdown.dec is called n times.
countdown.dec();

I know that there are quite a few of the http2 tests that can benefit from this, along with a bunch of other http and https tests.

These tasks would be for folks who are a bit more comfortable with their Node.js skills.

There are many tests in the suite that could be modified to use this new utility. However, rather than modifying them all at once, if a new contributor wishes to take this on, please one change one or two at a time, leaving other tests for other contributors to pick up on. Just reference this tracking issue in the commit/PR so that we can keep track.

[Bamieh]

@jasnell I am willing to contribute to this issue, but first i recommend creating a list with all the places where this refactor is needed so we an keep a tracked list of whats being worked on and what is yet to be refactored.

[jasnell]

@Bamieh ... absolutely... I'll start digging through later on today and building a list out here in this issue :-)

[joyeecheung]

Would be nice if there is a guide or a link to https://github.com/nodejs/node/tree/master/test/common#countdown-module in https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md

(Aside: we should add a note on the usage of common/fixtures in that testing guide as well..)

[Bamieh]

Here is a full list of the http test cases to use Countdown:

Feel free to update it please!

Http Test Name	Complete	Pull request ready
test/parallel/test-http-1.0-keep-alive.js	no	no
test/parallel/test-http-after-connect.js	yes	no
test/parallel/test-http-agent-maxsockets-regress-4050.js	yes	no
test/parallel/test-http-agent-maxsockets.js	yes	no
test/parallel/test-http-agent.js	no	no
test/parallel/test-http-allow-req-after-204-res.js	no	yes
test/parallel/test-http-client-abort.js	yes	no
test/parallel/test-http-client-agent.js	no	no
test/parallel/test-http-client-check-http-token.js	yes	no
test/parallel/test-http-client-default-headers-exist.js	yes	no
test/parallel/test-http-client-parse-error.js	yes	no
test/parallel/test-http-client-timeout-agent.js (not sure)	no	no
test/parallel/test-http-content-length.js	no	yes
test/parallel/test-http-end-throw-socket-handling.js	no	yes
test/parallel/test-http-exceptions.js	no	yes
test/parallel/test-http-expect-continue.js	no	no
test/parallel/test-http-expect-handling.js	no	no
test/parallel/test-http-get-pipeline-problem.js	no	no
test/parallel/test-http-host-header-ipv6-fail.js	no	no
test/parallel/test-http-host-headers.js	no	no
test/parallel/test-http-incoming-pipelined-socket-destroy.js	no	no
test/parallel/test-http-invalidheaderfield.js	no	no
test/parallel/test-http-keep-alive-close-on-header.js	no	no
test/parallel/test-http-keepalive-client.js	no	no
test/parallel/test-http-keepalive-override.js	no	no
test/parallel/test-http-keepalive-request.js	no	no
test/parallel/test-http-malformed-request.js	no	no
test/parallel/test-http-max-headers-count.js	no	no
test/parallel/test-http-parser-bad-ref.js	no	no
test/parallel/test-http-parser-free.js	no	no
test/parallel/test-http-pipe-fs.js	no	yes
test/parallel/test-http-pipeline-flood.js	no	no
test/parallel/test-http-pipeline-regr-2639.js	no	no
test/parallel/test-http-pipeline-regr-3332.js	no	no
test/parallel/test-http-proxy.js	no	no
test/parallel/test-http-request-dont-override-options.js	no	no
test/parallel/test-http-res-write-end-dont-take-array.js	no	no
test/parallel/test-http-response-multi-content-length.js	no	no
test/parallel/test-http-response-multiheaders.js	no	no
test/parallel/test-http-response-splitting.js	no	yes
test/parallel/test-http-response-status-message.js	no	no
test/parallel/test-http-response-statuscode.js	no	yes
test/parallel/test-http-same-map.js	no	no
test/parallel/test-http-server.js	no	no
test/parallel/test-http-set-cookies.js	no	no
test/parallel/test-http-set-trailers.js	no	no
test/parallel/test-http-should-keep-alive.js	no	no
test/parallel/test-http-status-code.js	no	no
test/parallel/test-http-status-reason-invalid-chars.js	no	yes
test/parallel/test-http-timeout-overflow.js	no	no
test/parallel/test-http-timeout.js	no	yes
test/parallel/test-http-upgrade-advertise.js (not sure)	no	no
test/parallel/test-http-upgrade-client.js	no	yes
test/parallel/test-http-upgrade-server.js	no	no
test/parallel/test-http.js (not sure)	no	no

The ones marked with (not sure) have arrays of data that they use both as a counter and as the data for each counter, so i am not sure if they should be used in combination with a countdown or just kept as is.

[Bamieh]

@AyoAlfonso you have to fork the node package on github, then apply your changes on the fork. Finally you submit a pull request against the main node github master branch. good luck!