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

tools: use local or specified $NODE for test-npm #1984

Merged
merged 1 commit into from
Jul 25, 2015
Merged

tools: use local or specified $NODE for test-npm #1984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ test-debugger: all
$(PYTHON) tools/test.py debugger

test-npm: $(NODE_EXE)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are still correct, right? I'm not too sure how makefile uses this part.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, NODE_EXE is mostly used as build target.

NODE_EXE=$(NODE_EXE) tools/test-npm.sh
NODE=$(NODE) tools/test-npm.sh

test-npm-publish: $(NODE_EXE)
npm_package_config_publishtest=true $(NODE) deps/npm/test/run.js
Expand Down
13 changes: 7 additions & 6 deletions tools/test-npm.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ set -e
# always change the working directory to the project's root directory
cd $(dirname $0)/..

# pass NODE_EXE from something like Makefile
# it should point to either {./}node or {./}node.exe, depending on the platform
if [ -z $NODE_EXE ]; then
# pass a $NODE environment variable from something like Makefile
# it should point to either ./iojs or ./iojs.exe, depending on the platform
if [ -z $NODE ]; then
echo "No node executable provided. Bailing." >&2
exit 0
fi
Expand All @@ -29,9 +29,10 @@ export npm_config_prefix="npm-prefix"
export npm_config_tmp="npm-tmp"

# install npm devDependencies and run npm's tests
../$NODE_EXE cli.js install --ignore-scripts
../$NODE_EXE cli.js run-script test-legacy
../$NODE_EXE cli.js run-script test

../$NODE cli.js install --ignore-scripts
../$NODE cli.js run-script test-legacy
../$NODE cli.js run-script test
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually on second thought this always used the local node to run cli.js anyways because these are relative links.

Still, I think this PR is better practice given our makefile setup.


# clean up everything one single shot
cd .. && rm -rf test-npm