Skip to content

Commit

Permalink
Address fourth set of comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@mhdawson
mhdawson committed Nov 17, 2015
1 parent a742444 commit 7719c39
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 23 deletions.
4 changes: 2 additions & 2 deletions lib/configure.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ function configure (gyp, argv, callback) {
// for AIX we need to set up the path to the exp file
// which contains the symbols needed for linking.
// The file will either be in one of the following
// depeding on whether are are in installed or
// depending on whether it is an installed or
// development environment:
// - the include/node directory
// - the out/Release directory
Expand All @@ -316,7 +316,7 @@ function configure (gyp, argv, callback) {
'out/Release/node.exp',
'out/Debug/node.exp',
'node.exp']
for (var next in candidates) {
for (var next = 0; next < candidates.length; next++) {
node_exp_file = path.resolve(node_root_dir, candidates[next])
try {
fs.accessSync(node_exp_file, fs.R_OK)
Expand Down
42 changes: 21 additions & 21 deletions test/test-find-node-directory.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ var platforms = ['darwin', 'freebsd', 'linux', 'sunos', 'win32', 'aix']
// .... /deps/npm
test('test find-node-directory - node install', function (t) {
t.plan(platforms.length)
for (currentPlatform in platforms) {
var processObj = {execPath: '/x/y/bin/node', platform: currentPlatform}
for (var next = 0; next < platforms.length; next++) {
var processObj = {execPath: '/x/y/bin/node', platform: platforms[next]}
t.equal(
findNodeDirectory('/x/deps/npm/node_modules/node-gyp/lib', processObj),
path.join('/x'))
Expand All @@ -20,14 +20,14 @@ test('test find-node-directory - node install', function (t) {

// we should find the directory based on the directory
// the script is running in and it should match the layout
// in a installed tree where npm is installed in
// in an installed tree where npm is installed in
// .... /lib/node_modules/npm or .../node_modules/npm
// depending on the patform
test('test find-node-directory - node build', function (t) {
t.plan(platforms.length)
for (currentPlatform in platforms) {
var processObj = {execPath: '/x/y/bin/node', platform: currentPlatform}
if (currentPlatform === 'win32') {
for (var next = 0; next < platforms.length; next++) {
var processObj = {execPath: '/x/y/bin/node', platform: platforms[next]}
if (platforms[next] === 'win32') {
t.equal(
findNodeDirectory('/y/node_modules/npm/node_modules/node-gyp/lib',
processObj), path.join('/y'))
Expand All @@ -43,8 +43,8 @@ test('test find-node-directory - node build', function (t) {
// for node and match because it was in the bin directory
test('test find-node-directory - node in bin directory', function (t) {
t.plan(platforms.length)
for (currentPlatform in platforms) {
var processObj = {execPath: '/x/y/bin/node', platform: currentPlatform}
for (var next = 0; next < platforms.length; next++) {
var processObj = {execPath: '/x/y/bin/node', platform: platforms[next]}
t.equal(
findNodeDirectory('/nothere/npm/node_modules/node-gyp/lib', processObj),
path.join('/x/y'))
Expand All @@ -55,13 +55,13 @@ test('test find-node-directory - node in bin directory', function (t) {
// for node and match because it was in the Release directory
test('test find-node-directory - node in build release dir', function (t) {
t.plan(platforms.length)
for (currentPlatform in platforms) {
for (var next = 0; next < platforms.length; next++) {
var processObj
if (currentPlatform === 'win32') {
processObj = {execPath: '/x/y/Release/node', platform: currentPlatform}
if (platforms[next] === 'win32') {
processObj = {execPath: '/x/y/Release/node', platform: platforms[next]}
} else {
processObj = {execPath: '/x/y/out/Release/node',
platform: currentPlatform}
platform: platforms[next]}
}

t.equal(
Expand All @@ -74,12 +74,12 @@ test('test find-node-directory - node in build release dir', function (t) {
// for node and match because it was in the Debug directory
test('test find-node-directory - node in Debug release dir', function (t) {
t.plan(platforms.length)
for (currentPlatform in platforms) {
for (var next = 0; next < platforms.length; next++) {
var processObj
if (currentPlatform === 'win32') {
processObj = {execPath: '/a/b/Debug/node', platform: currentPlatform}
if (platforms[next] === 'win32') {
processObj = {execPath: '/a/b/Debug/node', platform: platforms[next]}
} else {
processObj = {execPath: '/a/b/out/Debug/node', platform: currentPlatform}
processObj = {execPath: '/a/b/out/Debug/node', platform: platforms[next]}
}

t.equal(
Expand All @@ -88,12 +88,12 @@ test('test find-node-directory - node in Debug release dir', function (t) {
}
})

// we shoudl not find it as it will not match based on the execPath nor
// we should not find it as it will not match based on the execPath nor
// the directory from which the script is running
test('test find-node-directory - not found', function (t) {
t.plan(platforms.length)
for (currentPlatform in platforms) {
var processObj = {execPath: '/x/y/z/y', platform:currentPlatform}
for (var next = 0; next < platforms.length; next++) {
var processObj = {execPath: '/x/y/z/y', platform:next}
t.equal(findNodeDirectory('/a/b/c/d', processObj), '')
}
})
Expand All @@ -106,8 +106,8 @@ test('test find-node-directory - not found', function (t) {
// don't cause an issue
test('test find-node-directory - node install', function (t) {
t.plan(platforms.length)
for (currentPlatform in platforms) {
var processObj = {execPath: '/x/y/bin/node', platform: currentPlatform}
for (var next = 0; next < platforms.length; next++) {
var processObj = {execPath: '/x/y/bin/node', platform: platforms[next]}
t.equal(
findNodeDirectory('/x/y/z/a/b/c/deps/npm/node_modules/node-gyp/lib',
processObj), path.join('/x/y/z/a/b/c'))
Expand Down

0 comments on commit 7719c39

Please sign in to comment.