Skip to content

test: rename regression tests with descriptive file names #19212

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.

Sign up for GitHub

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

Closed
ryzokuken wants to merge 10 commits into from
Closed

test: rename regression tests with descriptive file names #19212

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ae40a78
test: rename test-regress-GH-1531
ryzokuken Mar 7, 2018
363aa96
test: rename test-regress-GH-2245
ryzokuken Mar 7, 2018
9de9014
test: rename test-regress-GH-3238
ryzokuken Mar 7, 2018
18c7841
test: rename test-regress-GH-3542
ryzokuken Mar 8, 2018
8bdf66f
test: rename test-regress-GH-3739
ryzokuken Mar 8, 2018
3dd9688
test: change test-fs-existssync-false.js
ryzokuken Mar 8, 2018
edd6a58
test: rename test-regress-GH-4256
ryzokuken Mar 8, 2018
8e50585
test: handle (almost) all nits in #19212
ryzokuken Mar 8, 2018
fb4050f
test: handle all nits in #19212
ryzokuken Mar 8, 2018
c8654f5
test: punctuate tests in #19212
ryzokuken Mar 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
test: change test-fs-existssync-false.js 
Change test-fs-existssync-false.js to make sure it conforms to
the standard test structure.

Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure
  • Loading branch information
@ryzokuken
ryzokuken committed Mar 8, 2018
commit 3dd96889ce67fe9462bf9245d8de89c33bbbcf68
3 changes: 1 addition & 2 deletions test/parallel/test-fs-existssync-false.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'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)
Copy link
Copy Markdown
Member

@BridgeAR BridgeAR Mar 8, 2018

Choose a reason for hiding this comment

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

Nit: It would be nice to punctuate the new comments.

Expand All @@ -9,8 +10,6 @@ const assert = require('assert');
const fs = require('fs');
const path = require('path');

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

Copy link
Copy Markdown
Member

@BridgeAR BridgeAR Mar 8, 2018

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
Copy Markdown
Contributor Author

@ryzokuken ryzokuken Mar 8, 2018

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
Copy Markdown
Member

@BridgeAR BridgeAR Mar 8, 2018

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