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

Retry long path tests on Linux under real tmp dir #3929

Closed
wants to merge 3 commits into from
Closed

Retry long path tests on Linux under real tmp dir #3929

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7c76592
Retry long path tests on Linux under real tmp dir
rsp Nov 19, 2015
fa83454
Use template strings and const instead of var
rsp Nov 20, 2015
2f5ca5d
Use process.platform instead of os.type()
rsp Dec 1, 2015
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
56 changes: 34 additions & 22 deletions test/parallel/test-fs-long-path.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,44 @@ var common = require('../common');
var fs = require('fs');
var path = require('path');
var assert = require('assert');
var os = require('os');

var successes = 0;
// when it fails test again under real OS tmp dir on Linux when it is
// readable/writable to avoid failing tests on ecryptfs filesystems:
// https://github.com/nodejs/node/issues/2255
// it follows advice in comments to:
// https://github.com/nodejs/node/pull/3925
try {
common.refreshTmpDir();
testFsLongPath(common.tmpDir);
common.refreshTmpDir();
} catch (e) {
if (os.type() == 'Linux') {
Copy link
Member

Choose a reason for hiding this comment

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

perhaps use process.platform == 'linux' instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jbergstroem Yes, it would be the same. This code already uses the os module for other things so it seemed to make sense to use the normalized name for the comparison but it could test process.platform or os.platform() just as well as os.type(). I can change it if that's what's holding the PR #3936 back.

Copy link
Member

Choose a reason for hiding this comment

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

process.platform is a little more idiomatic, at least w.r.t. to the test suite.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bnoordhuis @jbergstroem Done. It's using process.platform now. See: 2f5ca5d

fs.accessSync(os.tmpdir(), fs.R_OK | fs.W_OK);
var tmpDir = path.join(os.tmpdir(),
'node-' + process.version + '-test-' + (Math.random() * 1e6 | 0));
Copy link
Member

Choose a reason for hiding this comment

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

use a template string to shorten this

fs.mkdirSync(tmpDir);
testFsLongPath(tmpDir);
fs.rmdirSync(tmpDir);
} else {
throw e;
}
}

// make a path that will be at least 260 chars long.
var fileNameLen = Math.max(260 - common.tmpDir.length - 1, 1);
var fileName = path.join(common.tmpDir, new Array(fileNameLen + 1).join('x'));
var fullPath = path.resolve(fileName);
function testFsLongPath(tmpDir) {

common.refreshTmpDir();
// make a path that will be at least 260 chars long.
var fileNameLen = Math.max(260 - tmpDir.length - 1, 1);
var fileName = path.join(tmpDir, new Array(fileNameLen + 1).join('x'));
var fullPath = path.resolve(fileName);

console.log({
filenameLength: fileName.length,
fullPathLength: fullPath.length
});

fs.writeFile(fullPath, 'ok', function(err) {
if (err) throw err;
successes++;

fs.stat(fullPath, function(err, stats) {
if (err) throw err;
successes++;
console.log({
filenameLength: fileName.length,
fullPathLength: fullPath.length
});
});

process.on('exit', function() {
fs.writeFileSync(fullPath, 'ok');
fs.statSync(fullPath);
fs.unlinkSync(fullPath);
assert.equal(2, successes);
});

}
53 changes: 40 additions & 13 deletions test/parallel/test-require-long-path.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,49 @@
const common = require('../common');
const fs = require('fs');
const path = require('path');
const os = require('os');

// make a path that is more than 260 chars long.
const dirNameLen = Math.max(260 - common.tmpDir.length, 1);
const dirName = path.join(common.tmpDir, 'x'.repeat(dirNameLen));
const fullDirPath = path.resolve(dirName);
// when it fails test again under real OS tmp dir on Linux when it is
// readable/writable to avoid failing tests on ecryptfs filesystems:
// https://github.com/nodejs/node/issues/2255
// it takes into account comments in:
// https://github.com/nodejs/node/pull/3925
try {
common.refreshTmpDir();
testRequireLongPath(common.tmpDir);
common.refreshTmpDir();
} catch (e) {
if (os.type() == 'Linux') {
fs.accessSync(os.tmpdir(), fs.R_OK | fs.W_OK);
var tmpDir = path.join(os.tmpdir(),
'node-' + process.version + '-test-' + (Math.random() * 1e6 | 0));
fs.mkdirSync(tmpDir);
testRequireLongPath(tmpDir);
fs.rmdirSync(tmpDir);
} else {
throw e;
}
}

const indexFile = path.join(fullDirPath, 'index.js');
const otherFile = path.join(fullDirPath, 'other.js');
function testRequireLongPath(tmpDir) {

common.refreshTmpDir();
// make a path that is more than 260 chars long.
const dirNameLen = Math.max(260 - tmpDir.length, 1);
const dirName = path.join(tmpDir, 'x'.repeat(dirNameLen));
const fullDirPath = path.resolve(dirName);

fs.mkdirSync(fullDirPath);
fs.writeFileSync(indexFile, 'require("./other");');
fs.writeFileSync(otherFile, '');
const indexFile = path.join(fullDirPath, 'index.js');
const otherFile = path.join(fullDirPath, 'other.js');

require(indexFile);
require(otherFile);
fs.mkdirSync(fullDirPath);
fs.writeFileSync(indexFile, 'require("./other");');
fs.writeFileSync(otherFile, '');

common.refreshTmpDir();
require(indexFile);
require(otherFile);

fs.unlinkSync(indexFile);
fs.unlinkSync(otherFile);
fs.rmdirSync(fullDirPath);

}