Skip to content

Conversation

@rvagg
Copy link
Member

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
MemberAuthor

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

@mscdexmscdex 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
MemberAuthor

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.

@rvaggrvagg merged commit 5472139 into nodejs:masterApr 27, 2015
@rvaggrvagg deleted the adjust-test-ci branch April 27, 2015 05:02
@rvaggrvagg mentioned this pull request Apr 28, 2015
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Apr 29, 2015
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 <[email protected]>
@Fishrock123Fishrock123 mentioned this pull request May 14, 2015

Choose a reason for hiding this comment

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

Test

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@rvagg@jbergstroem@jesusleonardo@mscdex