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.

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

Jump to bottom

test: refactor test-fs-watchfile.js #2393

Closed
wants to merge 1 commit into from
Closed

test: refactor test-fs-watchfile.js #2393

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 4 additions & 17 deletions test/parallel/test-fs-watchfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ const common = require('../common');
const fs = require('fs');
const path = require('path');
const assert = require('assert');
const fixtures = path.join(__dirname, '..', 'fixtures');

// Basic usage tests.
assert.throws(function() {
Expand All @@ -19,7 +18,7 @@ assert.throws(function() {
fs.watchFile(new Object(), function() {});
}, /Path must be a string/);

const enoentFile = path.join(fixtures, 'non-existent-file');
const enoentFile = path.join(common.tmpDir, 'non-existent-file');
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't common.refreshTmpDir() be called before using common.tmpDir?

Copy link
Member Author

Choose a reason for hiding this comment

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

refreshTmpDir() should be called before creating files in common.tmpDir but there's no reason why the path to the file can't be created and saved first as it is here.

const expectedStatObject = new fs.Stats(
0, // dev
0, // mode
Expand All @@ -37,24 +36,13 @@ const expectedStatObject = new fs.Stats(
Date.UTC(1970, 0, 1, 0, 0, 0) // birthtime
);

function removeTestFile() {
try {
fs.unlinkSync(enoentFile);
} catch (ex) {
if (ex.code !== 'ENOENT') {
throw ex;
}
}
}

// Make sure that the file does not exist, when the test starts
removeTestFile();
common.refreshTmpDir();

// If the file initially didn't exist, and gets created at a later point of
// time, the callback should be invoked again with proper values in stat object
var fileExists = false;

fs.watchFile(enoentFile, common.mustCall(function(curr, prev) {
fs.watchFile(enoentFile, {interval: 0}, common.mustCall(function(curr, prev) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering, how does an interval of zero work?

Copy link
Member Author

Choose a reason for hiding this comment

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

I figured that would poll every time the event loop finished. Ultimately, interval gets passed to libuv's uv_fs_poll_start(). Judging from this line of code it seems that 0 gets changed to 1. Correction or confirmation on my interpretation would be very welcome.

Copy link
Member Author

Choose a reason for hiding this comment

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

I suppose the whole "what happens if you set interval to 0" thing should be covered in the docs. /cc @nodejs/documentation

Copy link
Member

Choose a reason for hiding this comment

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

Technically correct, although libuv may change it someday to mean the lowest supported or most optimal poll interval. From the manual:

For maximum portability, use multi-second intervals. Sub-second intervals will not detect all changes on many file systems.

if (!fileExists) {
// If the file does not exist, all the fields should be zero and the date
// fields should be UNIX EPOCH time
Expand All @@ -71,8 +59,7 @@ fs.watchFile(enoentFile, common.mustCall(function(curr, prev) {
// As the file just got created, previous ino value should be lesser than
// or equal to zero (non-existent file).
assert(prev.ino <= 0);
// Stop watching the file and delete it
// Stop watching the file
fs.unwatchFile(enoentFile);
removeTestFile();
}
}, 2));