Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: rename regression tests with descriptive file names #19212

Closed
wants to merge 10 commits into from
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
'use strict';
const common = require('../common');

// Check that cluster works perfectly for both `kill` and `disconnect` cases.
// Also take into account that the `disconnect` event may be received after the
// `exit` event.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Maybe add a link to the GH issue that this test was named for originally?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how I could have missed this, will do.

// https://github.com/nodejs/node/issues/3238

const assert = require('assert');
const cluster = require('cluster');

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
/* eslint-disable strict */
require('../common');
const assert = require('assert');

/*
In Node.js 0.10, a bug existed that caused strict functions to not capture
their environment when evaluated. When run in 0.10 `test()` fails with a
`ReferenceError`. See https://github.com/nodejs/node/issues/2245 for details.
*/
// In Node.js 0.10, a bug existed that caused strict functions to not capture
// their environment when evaluated. When run in 0.10 `test()` fails with a
// `ReferenceError`. See https://github.com/nodejs/node/issues/2245 for details.

const assert = require('assert');

function test() {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
'use strict';

const common = require('../common');
const tmpdir = require('../common/tmpdir');

// This test ensures that fs.existsSync doesn't incorrectly return false.
// (especially on Windows)
// https://github.com/nodejs/node-v0.x-archive/issues/3739

const assert = require('assert');
const fs = require('fs');
const path = require('path');

const tmpdir = require('../common/tmpdir');

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I personally would not mode this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that in conflict with the canonical test structure?

Ref: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#lines-1-3

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is specific to the require('../common'); but it is fine to keep it as is.

let dir = path.resolve(tmpdir.path);

// Make sure that the tmp directory is clean
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
'use strict';
const common = require('../common');

// This test is only relevant on Windows.
if (!common.isWindows)
common.skip('Windows specific test.');

// This test ensures fs.realpathSync works on properly on Windows without
// throwing ENOENT when the path involves a fileserver.
// https://github.com/nodejs/node-v0.x-archive/issues/3542

const assert = require('assert');
const fs = require('fs');
const path = require('path');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
'use strict';
const common = require('../common');

// This test ensures that a http request callback is called
// when the agent option is set
// See https://github.com/nodejs/node-v0.x-archive/issues/1531

if (!common.hasCrypto)
common.skip('missing crypto');

const fixtures = require('../common/fixtures');

// This test ensures that a http request callback is called when the agent
// option is set.
// See https://github.com/nodejs/node-v0.x-archive/issues/1531

const https = require('https');

const options = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@

'use strict';
require('../common');

// This test ensures that setting `process.domain` to `null` does not result in
// node crashing with a segfault.
// https://github.com/nodejs/node-v0.x-archive/issues/4256

process.domain = null;
setTimeout(function() {
console.log('this console.log statement should not make node crash');
Expand Down