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.

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

test: fix test/test-configure-python on AIX #1131

Closed
wants to merge 1 commit into from

Conversation

richardlau
Copy link
Member

nodejs/citgm#370 (comment), test/test-configure-python is failing on AIX:

-bash-4.3$ node --version
v6.10.0
-bash-4.3$ node test/test-configure-python.js
TAP version 13
# configure PYTHONPATH with no existing env
gyp ERR! find exports file Could not find exports file
/home/users/riclau/sandbox/github/node-gyp/lib/configure.js:245
        return callback(new Error(msg))
               ^

TypeError: callback is not a function
    at runGyp (/home/users/riclau/sandbox/github/node-gyp/lib/configure.js:245:16)
    at findConfigs (/home/users/riclau/sandbox/github/node-gyp/lib/configure.js:170:23)
    at /home/users/riclau/sandbox/github/node-gyp/lib/configure.js:183:9
    at Object.stat (/home/users/riclau/sandbox/github/node-gyp/test/test-configure-python.js:10:35)
    at findConfigs (/home/users/riclau/sandbox/github/node-gyp/lib/configure.js:173:8)
    at /home/users/riclau/sandbox/github/node-gyp/lib/configure.js:183:9
    at Object.stat (/home/users/riclau/sandbox/github/node-gyp/test/test-configure-python.js:10:35)
    at findConfigs (/home/users/riclau/sandbox/github/node-gyp/lib/configure.js:173:8)
    at Object.writeFile (/home/users/riclau/sandbox/github/node-gyp/test/test-configure-python.js:9:46)
    at createConfigFile (/home/users/riclau/sandbox/github/node-gyp/lib/configure.js:164:8)
-bash-4.3$

On AIX, lib/configure.js attempts to locate node.exp via calls to fs.openSync() and fs.closeSync(). Add these functions to the mocked graceful-fs object in test/test-configure-python.js.

@richardlau richardlau mentioned this pull request Feb 27, 2017
1 task
Copy link
Member

@gdams gdams left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

Would it be useful to add AIX to node-gyp's CI?

@richardlau
Copy link
Member Author

Would it be useful to add AIX to node-gyp's CI?

I think so. The code to search for node.exp is AIX specific, so won't be executed on any other platform. This test hasn't worked on AIX since it landed in October last year.

I've proposed adding node-gyp to CITGM's default lookup.json (nodejs/citgm#370) which should give platform coverage when Node.js does new releases or tests significant PRs, but it would be much faster to catch these sort of things on node-gyp's CI runs if possible.

cc @nodejs/platform-aix

@gibfahn
Copy link
Member

gibfahn commented Feb 27, 2017

+1 to adding AIX to node-gyp's CI. I can take a look at doing that.

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@Fishrock123
Copy link
Contributor

Fishrock123 commented Apr 18, 2017

@bnoordhuis can this land to unblock nodejs/citgm#370?

@bnoordhuis
Copy link
Member

Sure, go ahead and land it. Or is your question if I want to land it?

@gibfahn
Copy link
Member

gibfahn commented Apr 25, 2017

@richardlau should these be squashed? Let me know and I'll land.

On AIX, lib/configure.js attempts to locate node.exp via calls to
fs.openSync() and fs.closeSync(). Add these functions to the mocked
`graceful-fs` object in test/test-configure-python.js.
@richardlau
Copy link
Member Author

@gibfahn Rebased and squashed.

@gibfahn
Copy link
Member

gibfahn commented Apr 25, 2017

CI: https://ci.nodejs.org/view/All/job/nodegyp-test-pull-request/11/

EDIT: 4 test failures on 0.10.48 Ubuntu, can reproduce locally, seems to be broken since e3778d9 (Fixed in #1172).

not ok 123 (unnamed assert)
  ---
    operator: ok
    expected: true
    actual:   false
    at: Object.f.stat (/home/iojs/build/workspace/nodegyp-test-commit/nodes/ubuntu1604-64/test/test-find-python.js:291:7)
  ...
not ok 124 (unnamed assert)
  ---
    operator: ok
    expected: true
    actual:   false
    at: Object.f.execFile (/home/iojs/build/workspace/nodegyp-test-commit/nodes/ubuntu1604-64/test/test-find-python.js:283:9)
  ...
ok 125 (unnamed assert)
not ok 126 (unnamed assert)
  ---
    operator: ok
    expected: true
    actual:   false
    at: Object.done [as callback] (/home/iojs/build/workspace/nodegyp-test-commit/nodes/ubuntu1604-64/test/test-find-python.js:297:7)
  ...
# find python - no python, no python launcher, bad guess
ok 127 should be equal
ok 128 should be equal
not ok 129 (unnamed assert)
  ---
    operator: ok
    expected: true
    actual:   false
    at: Object.f.stat (/home/iojs/build/workspace/nodegyp-test-commit/nodes/ubuntu1604-64/test/test-find-python.js:317:7)
  ...

gibfahn pushed a commit that referenced this pull request Apr 25, 2017
On AIX, lib/configure.js attempts to locate node.exp via calls to
fs.openSync() and fs.closeSync(). Add these functions to the mocked
`graceful-fs` object in test/test-configure-python.js.

PR-URL: #1131
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@gibfahn
Copy link
Member

gibfahn commented Apr 25, 2017

Landed in a83a380

@gibfahn gibfahn closed this Apr 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants