Skip to content

Commit

Permalink
path: remove dead code in favor of unit tests
Browse files Browse the repository at this point in the history
Remove dead code paths that are created by assertions that will never
trigger. They may only trigger if either the `splitDeviceRe` or
`splitPathRe` regular expressions are modified. If at some point they
are modified, current unit tests will catch most of the resulting
errors and this commit adds extra tests to catch the remaining errors.

PR-URL: #2282
Reviewed-By: Roman Reiss <me@silverwind.io>
  • Loading branch information
nwoltman authored and rvagg committed Aug 17, 2015
1 parent 07a88b0 commit 7118b8a
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 15 deletions.
12 changes: 1 addition & 11 deletions lib/path.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ function win32SplitPath(filename) {
// Separate device+slash from tail
var result = splitDeviceRe.exec(filename),
device = (result[1] || '') + (result[2] || ''),
tail = result[3] || '';
tail = result[3];
// Split the tail into dir, basename and extension
var result2 = splitTailRe.exec(tail),
dir = result2[1],
Expand Down Expand Up @@ -386,9 +386,6 @@ win32.parse = function(pathString) {
assertPath(pathString);

var allParts = win32SplitPath(pathString);
if (!allParts || allParts.length !== 4) {
throw new TypeError("Invalid path '" + pathString + "'");
}
return {
root: allParts[0],
dir: allParts[0] + allParts[1].slice(0, -1),
Expand Down Expand Up @@ -590,13 +587,6 @@ posix.parse = function(pathString) {
assertPath(pathString);

var allParts = posixSplitPath(pathString);
if (!allParts || allParts.length !== 4) {
throw new TypeError("Invalid path '" + pathString + "'");
}
allParts[1] = allParts[1] || '';
allParts[2] = allParts[2] || '';
allParts[3] = allParts[3] || '';

return {
root: allParts[0],
dir: allParts[0] + allParts[1].slice(0, -1),
Expand Down
13 changes: 9 additions & 4 deletions test/parallel/test-path-parse-format.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ var winPaths = [
'\\foo\\C:',
'file',
'.\\file',
'',

// unc
'\\\\server\\share\\file_path',
Expand All @@ -32,7 +33,8 @@ var unixPaths = [
'file',
'.\\file',
'./file',
'C:\\foo'
'C:\\foo',
''
];

var unixSpecialCaseFormatTests = [
Expand All @@ -52,8 +54,6 @@ var errors = [
message: /Path must be a string. Received 1/},
{method: 'parse', input: [],
message: /Path must be a string. Received undefined/},
// {method: 'parse', input: [''],
// message: /Invalid path/}, // omitted because it's hard to trigger!
{method: 'format', input: [null],
message: /Parameter 'pathObject' must be an object, not/},
{method: 'format', input: [''],
Expand Down Expand Up @@ -93,8 +93,13 @@ function checkErrors(path) {
}

function checkParseFormat(path, paths) {
paths.forEach(function(element, index, array) {
paths.forEach(function(element) {
var output = path.parse(element);
assert.strictEqual(typeof output.root, 'string');
assert.strictEqual(typeof output.dir, 'string');
assert.strictEqual(typeof output.base, 'string');
assert.strictEqual(typeof output.ext, 'string');
assert.strictEqual(typeof output.name, 'string');
assert.strictEqual(path.format(output), element);
assert.strictEqual(output.dir, output.dir ? path.dirname(element) : '');
assert.strictEqual(output.base, path.basename(element));
Expand Down

0 comments on commit 7118b8a

Please sign in to comment.