-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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: migrate from tape to tap #1795
Conversation
@cclauss I probably should have run your Python changes through CI because there are some problems on Windows which may be coming from it? See https://ci.nodejs.org/job/nodegyp-test-commit/nodes=win2016-vs2017/678/console
Could that be coming from abef93d which did: - if textmap.has_key(encoding):
- print textmap[encoding]
+ if encoding in textmap:
+ print(textmap[encoding]) |
CI for this: https://ci.nodejs.org/view/All/job/nodegyp-test-pull-request/133/ CI isn't using tap output at all for anything automatic, but it is handy for text output. So what I've done is introduce a |
Our current Travis CI runs on Linux but we can easily add Windows and/or macOS if that would be useful. |
very useful, so many of our problems are OS specific here. |
A bunch more minor updates to make the standard There's a new deprecation in Node 12 that spits out to stderr because we're using a self-signed cert with an IP address, so there's a commit in here that replaces that with a self-signed for Test output:
|
Travis is probably better off with the prettified output. As it is it doesn't display all of it (output is too long): For the CI the only reason I can think of to keep the tap output is if we want to use tap2junit on it (which we don't currently). Since switching to |
👍 you're right, it looks pretty good in Travis: https://travis-ci.com/nodejs/node-gyp/jobs/210631751 |
PR-URL: #1795 Reviewed-By: Richard Lau <riclau@uk.ibm.com>
PR-URL: #1795 Reviewed-By: Richard Lau <riclau@uk.ibm.com>
landed in 395f843 |
This broke Node v6. CI failed because of the python issues, and that probably caused the real failures in v6 to go unnoticed. What should we do here? I don't think we should support unsupported Node versions at all, but for that we need to update the engines field and exclude from CI. Or should we revert this and keep supporting Node v6? It would probably be nicer to drop support on a semver-major version, though I'm not sure we strictly need to. cc @nodejs/node-gyp |
Can you please provide a URL to these failures? |
https://ci.nodejs.org/job/nodegyp-test-commit/676/ and https://ci.nodejs.org/job/nodegyp-test-commit/677/ There are two failures. On CI, every run of v6.2.1 and v6.14.4 failed while running
Locally, if I use a newer version of npm, this does not happen. This is probably an issue with npm or the module being installed, I don't know if it is related to this PR. If we're to support Node v6 we should work around this somehow if possible. However, after using a new npm to install, this error happens when running
|
addressing it in #1808, but maybe we need to extend our travis matrix to include the major Node versions, probably just on Linux (I don't know which version of Python, maybe it doesn't matter). |
alternative to #1171
Needs to be tested against CI. If we're not using tap output in CI I'd like to remove
-Rtap
.