Skip to content

Commit

Permalink
test: use relative path in pipePrefix
Browse files Browse the repository at this point in the history
Modified pipePrefix to use relative path on windows,
previously tests failed when the full path was 120+ characters

PR-URL: #15988
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
  • Loading branch information
rhanford authored and joyeecheung committed Oct 14, 2017
1 parent 4826ac5 commit 971aad1
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ Object.defineProperty(exports, 'hasFipsCrypto', {
});

{
const pipePrefix = exports.isWindows ? '\\\\.\\pipe\\' : `${exports.tmpDir}/`;
const localRelative = path.relative(process.cwd(), `${exports.tmpDir}/`);
const pipePrefix = exports.isWindows ? '\\\\.\\pipe\\' : localRelative;
const pipeName = `node-test.${process.pid}.sock`;
exports.PIPE = pipePrefix + pipeName;
}
Expand Down

3 comments on commit 971aad1

@refack
Copy link
Contributor

@refack refack commented on 971aad1 Oct 22, 2017

Choose a reason for hiding this comment

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

(not passing the blame, It's just that only now I realized this was not part of c34ae48)
Tiny bit of PM for #16364

const localRelative = path.relative(process.cwd(), `${exports.tmpDir}/`);

Strips the last / so later exports.PIPE = pipePrefix + pipeName; now requires a path.join

@joyeecheung
Copy link
Member

Choose a reason for hiding this comment

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

@refack yikes..

@refack
Copy link
Contributor

@refack refack commented on 971aad1 Oct 22, 2017

Choose a reason for hiding this comment

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

path can always surprise you πŸ€·β€β™‚οΈ
Like I told Rich, I was sure I wrote this, since this is totally something I would do.

Please sign in to comment.