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: adjust Makefile/test-ci, add to vcbuild.bat #1530

Merged
merged 1 commit into from Apr 27, 2015

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Apr 26, 2015

make test-ci consistent with test and introduce it to vcbuild.bat -- jenkins is now looking for it so the builds are failing without this addition (I've been testing tap output). Also puts in linting to CI which has been missing until now.

@rvagg
Copy link
Member Author

rvagg commented Apr 26, 2015

changed the windows build cmd on the current set up to just test not test-ci which is missing - I need to fix this once this is merged

@mscdex mscdex added the test Issues and PRs related to the tests. label Apr 26, 2015
@jbergstroem
Copy link
Member

Missing cctest. Not sure if intentional since it still lacks tap output -- we need to create a script that transforms the xml generated by gtest into tap output. If we don't rely on output being tap just yet you should add it.

Also, I'm not super keen about having jslint/cclint as part of the test suite -- it should rather be another step. That's probably for another PR/issue though since this is consistent with the test target.

@rvagg
Copy link
Member Author

rvagg commented Apr 26, 2015

tbh I'm trying to be relatively minimal here, it's also missing test-addon which really needs to go in but afaik it's still broken in this configuration - I just need to get something basic going and we can iterate from there.

@jbergstroem if you can see a path forward from here to enhance then perhaps lgtm this PR and put in another to take it up a notch, right now I just want to get the basics sorted.

@jbergstroem
Copy link
Member

The patch improves our current situation, so LGTM.

PR-URL: nodejs#1530
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@rvagg rvagg merged commit 5472139 into nodejs:master Apr 27, 2015
@rvagg rvagg deleted the adjust-test-ci branch April 27, 2015 05:02
@rvagg rvagg mentioned this pull request Apr 28, 2015
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Apr 29, 2015
PR-URL: nodejs#1530
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 14, 2015
PR-URL: nodejs#1530
PORT-PR-URL: nodejs#1560
PORT-FROM: v2.x / 5472139
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@@ -112,7 +112,9 @@ test-all-valgrind: test-build
$(PYTHON) tools/test.py --mode=debug,release --valgrind

test-ci:
$(PYTHON) tools/test.py -p tap --logfile test.tap -J parallel sequential message

Choose a reason for hiding this comment

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

Test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants