From 49c354a2233e9410ec80ef93800b0f93decb0140 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Wed, 16 May 2018 14:03:23 -0700 Subject: [PATCH 1/2] Revert "Revert "Add coverage reporting for JavaScript and TypeScript files"" --- gulpfile.ts | 2 +- package.json | 21 ++++++++++++--- packages/grpc-js-core/src/channel.ts | 37 ++++++++++++++++++++++----- packages/grpc-protobufjs/package.json | 5 +++- run-tests.sh | 29 ++++++++++++++++++--- test/api/interop_sanity_test.js | 2 +- test/gulpfile.js | 24 ++++++++++++----- test/interop/interop_client.js | 8 ++++-- 8 files changed, 104 insertions(+), 24 deletions(-) diff --git a/gulpfile.ts b/gulpfile.ts index 388a74f4..dac1421e 100644 --- a/gulpfile.ts +++ b/gulpfile.ts @@ -102,7 +102,7 @@ gulp.task('test.only', 'Run tests without rebuilding anything', ['js.core.test', 'native.test.only']); gulp.task('test', 'Run all tests', (callback) => { - runSequence('build', 'test.only', callback); + runSequence('build', 'test.only', 'internal.test.test', callback); }); gulp.task('doc.gen', 'Generate documentation', ['native.core.doc.gen']); diff --git a/package.json b/package.json index fb626214..e8d7a112 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,7 @@ "@types/node": "^8.0.32", "@types/pify": "^3.0.0", "@types/semver": "^5.5.0", + "coveralls": "^3.0.1", "del": "^3.0.0", "execa": "^0.8.0", "gulp": "^3.9.1", @@ -35,8 +36,11 @@ "mocha": "^3.5.3", "mocha-jenkins-reporter": "^0.3.9", "ncp": "^2.0.0", + "nyc": "^11.7.2", "pify": "^3.0.0", + "run-sequence": "^2.2.0", "semver": "^5.5.0", + "symlink": "^2.1.0", "through2": "^2.0.3", "ts-node": "^3.3.0", "tslint": "^5.5.0", @@ -48,8 +52,19 @@ "name": "Google Inc." } ], - "dependencies": { - "run-sequence": "^2.2.0", - "symlink": "^2.1.0" + "nyc": { + "include": [ + "packages/grpc-health-check/health.js", + "packages/grpc-js-core/build/src/*", + "packages/grpc-native-core/index.js", + "packages/grpc-native-core/src/*.js", + "packages/grpc-protobufjs/build/src/*" + ], + "cache": true, + "all": true + }, + "scripts": { + "test": "nyc gulp test", + "coverage": "nyc report --reporter=text-lcov | coveralls" } } diff --git a/packages/grpc-js-core/src/channel.ts b/packages/grpc-js-core/src/channel.ts index c03389d8..40b5a93d 100644 --- a/packages/grpc-js-core/src/channel.ts +++ b/packages/grpc-js-core/src/channel.ts @@ -82,6 +82,15 @@ export interface Channel extends EventEmitter { /* tslint:enable:no-any */ } +/* This should be a real subchannel class that contains a ClientHttp2Session, + * but for now this serves its purpose */ +type Http2SubChannel = http2.ClientHttp2Session & { + /* Count the number of currently active streams associated with the session. + * The purpose of this is to keep the session reffed if and only if there + * is at least one active stream */ + streamCount?: number; +}; + export class Http2Channel extends EventEmitter implements Channel { private readonly userAgent: string; private readonly target: url.URL; @@ -91,7 +100,7 @@ export class Http2Channel extends EventEmitter implements Channel { private connecting: Promise|null = null; /* For now, we have up to one subchannel, which will exist as long as we are * connecting or trying to connect */ - private subChannel: http2.ClientHttp2Session|null = null; + private subChannel: Http2SubChannel|null = null; private filterStackFactory: FilterStackFactory; private subChannelConnectCallback: () => void = () => {}; @@ -160,7 +169,7 @@ export class Http2Channel extends EventEmitter implements Channel { } private startConnecting(): void { - let subChannel: http2.ClientHttp2Session; + let subChannel: Http2SubChannel; const secureContext = this.credentials.getSecureContext(); if (secureContext === null) { subChannel = http2.connect(this.target); @@ -260,11 +269,25 @@ export class Http2Channel extends EventEmitter implements Channel { headers[HTTP2_HEADER_PATH] = methodName; headers[HTTP2_HEADER_TE] = 'trailers'; if (this.connectivityState === ConnectivityState.READY) { - const session: http2.ClientHttp2Session = this.subChannel!; - // Prevent the HTTP/2 session from keeping the process alive. - // Note: this function is only available in Node 9 - session.unref(); - stream.attachHttp2Stream(session.request(headers)); + const session: Http2SubChannel = this.subChannel!; + let http2Stream = session.request(headers); + /* This is a very ad-hoc reference counting scheme. This should be + * handled by a subchannel class */ + session.ref(); + if (!session.streamCount) { + session.streamCount = 0; + } + session.streamCount += 1; + http2Stream.on('close', () => { + if (!session.streamCount) { + session.streamCount = 0; + } + session.streamCount -= 1; + if (session.streamCount <= 0) { + session.unref(); + } + }); + stream.attachHttp2Stream(http2Stream); } else { /* In this case, we lost the connection while finalizing * metadata. That should be very unusual */ diff --git a/packages/grpc-protobufjs/package.json b/packages/grpc-protobufjs/package.json index 1ed26975..8890edb8 100644 --- a/packages/grpc-protobufjs/package.json +++ b/packages/grpc-protobufjs/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/proto-loader", - "version": "0.1.0", + "version": "0.2.0", "author": "Google Inc.", "contributors": [ { @@ -47,5 +47,8 @@ "clang-format": "^1.2.2", "gts": "^0.5.3", "typescript": "~2.7.2" + }, + "engines": { + "node": ">=6" } } diff --git a/run-tests.sh b/run-tests.sh index 300a2527..4484d40b 100755 --- a/run-tests.sh +++ b/run-tests.sh @@ -39,9 +39,25 @@ npm install --unsafe-perm mkdir -p reports export JOBS=8 +export JUNIT_REPORT_STACK=1 + +OS=$(uname) + +if [ "$OS" = "Linux" ] +then + gsutil cp gs://grpc-testing-secrets/coveralls_credentials/grpc-node.rc /tmp + set +x + . /tmp/grpc-node.rc + set -x + export COVERALLS_REPO_TOKEN + export COVERALLS_SERVICE_NAME=Kokoro + export COVERALLS_SERVICE_JOB_ID=$KOKORO_BUILD_ID +fi # TODO(mlumish): Add electron tests +FAILED="false" + for version in ${node_versions} do # Install and setup node for the version we want. @@ -51,6 +67,8 @@ do nvm use $version set -ex + export JUNIT_REPORT_PATH="reports/node$version/" + # https://github.com/mapbox/node-pre-gyp/issues/362 npm install -g node-gyp @@ -62,8 +80,8 @@ do ./node_modules/.bin/gulp clean.all ./node_modules/.bin/gulp setup - # Rebuild libraries and run tests. - JUNIT_REPORT_PATH="reports/node$version/" JUNIT_REPORT_STACK=1 ./node_modules/.bin/gulp test || FAILED="true" + # npm test calls nyc gulp test + npm test || FAILED="true" done set +ex @@ -72,7 +90,12 @@ set -ex node merge_kokoro_logs.js -if [ "$FAILED" != "" ] +if [ "$FAILED" = "true" ] then exit 1 +else + if [ "$OS" = "Linux" ] + then + npm run coverage + fi fi diff --git a/test/api/interop_sanity_test.js b/test/api/interop_sanity_test.js index 1152b310..0ff4c28b 100644 --- a/test/api/interop_sanity_test.js +++ b/test/api/interop_sanity_test.js @@ -68,7 +68,7 @@ describe('Interop tests', function() { throw new Error(`Server exited with signal ${signal}`); } } - }) + }); }); after(function() { serverProcess.send({}); diff --git a/test/gulpfile.js b/test/gulpfile.js index 61ff1658..2aa1c9c2 100644 --- a/test/gulpfile.js +++ b/test/gulpfile.js @@ -21,6 +21,7 @@ const mocha = require('gulp-mocha'); const execa = require('execa'); const path = require('path'); const del = require('del'); +const semver = require('semver'); const linkSync = require('../util').linkSync; // gulp-help monkeypatches tasks to have an additional description parameter @@ -38,6 +39,10 @@ gulp.task('clean.all', 'Delete all files created by tasks', () => {}); gulp.task('test', 'Run API-level tests', () => { // run mocha tests matching a glob with a pre-required fixture, // returning the associated gulp stream + if (!semver.satisfies(process.version, '>=9.4')) { + console.log(`Skipping cross-implementation tests for Node ${process.version}`); + return; + } const apiTestGlob = `${apiTestDir}/*.js`; const runTestsWithFixture = (server, client) => new Promise((resolve, reject) => { const fixture = `${server}_${client}`; @@ -51,12 +56,19 @@ gulp.task('test', 'Run API-level tests', () => { .on('end', resolve) .on('error', reject); }); - const runTestsArgPairs = [ - ['native', 'native'], - ['native', 'js'], - // ['js', 'native'], - // ['js', 'js'] - ]; + var runTestsArgPairs; + if (semver.satisfies(process.version, '>=9.4')) { + runTestsArgPairs = [ + ['native', 'native'], + ['native', 'js'], + // ['js', 'native'], + // ['js', 'js'] + ]; + } else { + runTestsArgPairs = [ + ['native', 'native'] + ]; + } return runTestsArgPairs.reduce((previousPromise, argPair) => { return previousPromise.then(runTestsWithFixture.bind(null, argPair[0], argPair[1])); }, Promise.resolve()); diff --git a/test/interop/interop_client.js b/test/interop/interop_client.js index fb6026e0..195ef8d4 100644 --- a/test/interop/interop_client.js +++ b/test/interop/interop_client.js @@ -616,8 +616,12 @@ if (require.main === module) { }; runTest(argv.server_host + ':' + argv.server_port, argv.server_host_override, argv.test_case, argv.use_tls === 'true', argv.use_test_ca === 'true', - function () { - console.log('OK:', argv.test_case); + function (err) { + if (err) { + throw err; + } else { + console.log('OK:', argv.test_case); + } }, extra_args); } From 5daba7e249f08ef6506328aed9232fb13da66799 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 16 May 2018 14:18:38 -0700 Subject: [PATCH 2/2] Skip coverage reporting if token file can't be downloaded --- run-tests.sh | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/run-tests.sh b/run-tests.sh index 4484d40b..a5195716 100755 --- a/run-tests.sh +++ b/run-tests.sh @@ -43,17 +43,6 @@ export JUNIT_REPORT_STACK=1 OS=$(uname) -if [ "$OS" = "Linux" ] -then - gsutil cp gs://grpc-testing-secrets/coveralls_credentials/grpc-node.rc /tmp - set +x - . /tmp/grpc-node.rc - set -x - export COVERALLS_REPO_TOKEN - export COVERALLS_SERVICE_NAME=Kokoro - export COVERALLS_SERVICE_JOB_ID=$KOKORO_BUILD_ID -fi - # TODO(mlumish): Add electron tests FAILED="false" @@ -96,6 +85,14 @@ then else if [ "$OS" = "Linux" ] then + # If we can't download the token file, just skip reporting coverage + gsutil cp gs://grpc-testing-secrets/coveralls_credentials/grpc-node.rc /tmp || exit 0 + set +x + . /tmp/grpc-node.rc + set -x + export COVERALLS_REPO_TOKEN + export COVERALLS_SERVICE_NAME=Kokoro + export COVERALLS_SERVICE_JOB_ID=$KOKORO_BUILD_ID npm run coverage fi fi