From 90669b4eea080b4e4e1140426704e93d148af831 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 23 Jul 2024 17:54:39 +0000 Subject: [PATCH 1/4] fix(tests): delete flaky test This test is already disabled for current and future versions of NodeJS, and I don't think it's worth putting in the effort to fix flakiness when the whole test will soon be a no-op. --- test/test-plugins-no-project-num.ts | 171 ---------------------------- 1 file changed, 171 deletions(-) delete mode 100644 test/test-plugins-no-project-num.ts diff --git a/test/test-plugins-no-project-num.ts b/test/test-plugins-no-project-num.ts deleted file mode 100644 index 6eb3e581..00000000 --- a/test/test-plugins-no-project-num.ts +++ /dev/null @@ -1,171 +0,0 @@ -// Copyright 2015 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -import * as assert from 'assert'; -import {describe, before, after, it} from 'mocha'; -import * as semver from 'semver'; - -let write; - -describe('test-plugins-no-project-num', () => { - let savedProject; - - before(() => { - savedProject = process.env.GCLOUD_PROJECT; - delete process.env.GCLOUD_PROJECT; - require('../..').start(); - }); - - after(() => { - process.env.GCLOUD_PROJECT = savedProject; - }); - - describe('should not break without project num', () => { - // Skip this set for incompatible versions of Node - const skip = !semver.satisfies(process.version, '<20'); - (skip ? describe.skip : describe)('Skipping anything over 20', () => { - before(() => { - // Mute stderr to satiate appveyor - write = process.stderr.write; - process.stderr.write = function (c, e?, cb?) { - if (cb) { - cb(); - } - return true; - }; - }); - after(() => { - process.stderr.write = write; - }); - it('mongo', done => { - // eslint-disable-next-line @typescript-eslint/no-var-requires - const mongoose = require('./plugins/fixtures/mongoose4'); - const Simple = mongoose.model( - 'Simple', - new mongoose.Schema({ - f1: String, - f2: Boolean, - f3: Number, - }) - ); - mongoose.connect('mongodb://localhost:27017/testdb', err => { - assert( - !err, - 'Skipping: error connecting to mongo at localhost:27017.' - ); - Simple.find({}, () => { - mongoose.connection.close(() => { - done(); - }); - }); - }); - }); - - it('redis', done => { - // eslint-disable-next-line @typescript-eslint/no-var-requires - const redis = require('./plugins/fixtures/redis2.3'); - const client = redis.createClient(); - client.set('i', 1, () => { - client.quit(() => { - done(); - }); - }); - }); - - it('express', done => { - // eslint-disable-next-line @typescript-eslint/no-var-requires - const http = require('http'); - // eslint-disable-next-line @typescript-eslint/no-var-requires - const express = require('./plugins/fixtures/express4'); - const app = express(); - app.get('/', (req, res) => { - res.send('hi'); - server.close(); - done(); - }); - const server = app.listen(8081, () => { - http.get({port: 8081}); - }); - }); - - it('restify', done => { - // eslint-disable-next-line @typescript-eslint/no-var-requires - const http = require('http'); - // eslint-disable-next-line @typescript-eslint/no-var-requires - const restify = require('./plugins/fixtures/restify4'); - const server = restify.createServer(); - server.get('/', (req, res) => { - res.writeHead(200, { - 'Content-Type': 'text/plain', - }); - res.write('hi'); - res.end(); - server.close(); - done(); - }); - server.listen(8081, () => { - http.get({port: 8081}); - }); - }); - - it.skip('hapi', done => { - // eslint-disable-next-line @typescript-eslint/no-var-requires - const http = require('http'); - // eslint-disable-next-line @typescript-eslint/no-var-requires - const hapi = require('./plugins/fixtures/hapi8'); - const server = new hapi.Server(); - server.connection({port: 8081}); - server.route({ - method: 'GET', - path: '/', - handler: function (req, reply) { - reply('hi'); - server.stop(); - done(); - }, - }); - server.start(() => { - http.get({port: 8081}); - }); - }); - - it('http', done => { - const req = require('http').get({port: 8081}); - req.on('error', () => { - done(); - }); - }); - - const mysql_implementations = ['mysql-2', 'mysql2-1']; - mysql_implementations.forEach(impl => { - it(impl, done => { - // eslint-disable-next-line @typescript-eslint/no-var-requires - const mysql = require('./plugins/fixtures/' + impl); - // eslint-disable-next-line @typescript-eslint/no-var-requires - const pool = mysql.createPool(require('./mysql-config' /*.js*/)); - pool.getConnection((err, conn) => { - assert(!err, 'Skipping: Failed to connect to mysql.'); - conn.query('SHOW TABLES', () => { - conn.release(); - pool.end(); - done(); - }); - }); - }); - }); - }); - }); -}); - -export default {}; From 5294588d515540218108cf1aef8a46f6f1b4a6f3 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 23 Jul 2024 21:20:50 +0000 Subject: [PATCH 2/4] fix(tests): work around type check failures --- test/plugins/test-trace-http2.ts | 2 +- test/trace.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/plugins/test-trace-http2.ts b/test/plugins/test-trace-http2.ts index d5152b64..e8e3c468 100644 --- a/test/plugins/test-trace-http2.ts +++ b/test/plugins/test-trace-http2.ts @@ -81,7 +81,7 @@ maybeSkipHttp2('Trace Agent integration with http2', () => { }); it('should patch the necessary functions', () => { - assert.strictEqual(http2.connect['__wrapped'], true); + assert.strictEqual((http2 as any).connect['__wrapped'], true); }); it('should accurately measure request time', done => { diff --git a/test/trace.ts b/test/trace.ts index 8336287b..7b7d81ea 100644 --- a/test/trace.ts +++ b/test/trace.ts @@ -120,7 +120,7 @@ export function get(): PluginTypes.Tracer { } export function setLoggerForTest(impl?: typeof logger.Logger) { - if (logger.Logger.__wrapped) { + if ((logger.Logger as any).__wrapped) { shimmer.unwrap(logger, 'Logger'); } if (impl) { From 842185016d796164dc23bcff513f659160882c38 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 23 Jul 2024 21:31:09 +0000 Subject: [PATCH 3/4] fix(test): disable codecov Codecov uploads are being throttled (429) because we are exceeding monthly quota --- .github/workflows/ci.yaml | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 32ca8c2f..3bc28ae1 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -43,11 +43,6 @@ jobs: - run: node --version - run: npm install - run: npm test - - name: coverage - uses: codecov/codecov-action@v3 - with: - name: actions ${{ matrix.node }} - fail_ci_if_error: true windows: runs-on: windows-latest env: @@ -61,11 +56,6 @@ jobs: node-version: 14 - run: npm install - run: npm test - - name: coverage - uses: codecov/codecov-action@v3 - with: - name: actions windows - fail_ci_if_error: true lint: runs-on: ubuntu-latest steps: From 0e01a7dfa29d62bbd5925dc323308cb7d1577811 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Wed, 24 Jul 2024 14:53:52 +0000 Subject: [PATCH 4/4] fix: migrate eslint ignores --- scripts/index.ts | 2 +- src/plugins/plugin-connect.ts | 2 +- src/plugins/plugin-hapi.ts | 2 +- src/plugins/plugin-http2.ts | 2 +- src/plugins/plugin-koa.ts | 2 +- src/trace-writer.ts | 2 +- test/non-interference/http-e2e.js | 2 +- test/non-interference/start-agent.js | 4 ++-- test/plugins/test-trace-http2.ts | 4 ++-- test/plugins/test-trace-node-fetch.ts | 2 +- 10 files changed, 12 insertions(+), 12 deletions(-) diff --git a/scripts/index.ts b/scripts/index.ts index 4119a4a8..6fbd83fb 100644 --- a/scripts/index.ts +++ b/scripts/index.ts @@ -87,6 +87,6 @@ async function run(steps: string[]) { run(steps).catch(err => { console.error(err); - // eslint-disable-next-line no-process-exit + // eslint-disable-next-line n/no-process-exit process.exit(1); }); diff --git a/src/plugins/plugin-connect.ts b/src/plugins/plugin-connect.ts index b6b6aac0..8ef97a44 100644 --- a/src/plugins/plugin-connect.ts +++ b/src/plugins/plugin-connect.ts @@ -13,7 +13,7 @@ // limitations under the License. import {IncomingMessage, ServerResponse} from 'http'; -// eslint-disable-next-line node/no-deprecated-api +// eslint-disable-next-line n/no-deprecated-api import {parse as urlParse} from 'url'; import {PluginTypes} from '..'; diff --git a/src/plugins/plugin-hapi.ts b/src/plugins/plugin-hapi.ts index a336a975..71b49cda 100644 --- a/src/plugins/plugin-hapi.ts +++ b/src/plugins/plugin-hapi.ts @@ -14,7 +14,7 @@ import {ServerResponse} from 'http'; import * as shimmer from 'shimmer'; -// eslint-disable-next-line node/no-deprecated-api +// eslint-disable-next-line n/no-deprecated-api import {parse as urlParse} from 'url'; import {PluginTypes} from '..'; diff --git a/src/plugins/plugin-http2.ts b/src/plugins/plugin-http2.ts index 0a629ad4..b8137c87 100644 --- a/src/plugins/plugin-http2.ts +++ b/src/plugins/plugin-http2.ts @@ -15,7 +15,7 @@ // This is imported only for types. Generated .js file should NOT load 'http2'. // `http2` must be used only in type annotations, not in expressions. -// eslint-disable-next-line node/no-unsupported-features/node-builtins +// eslint-disable-next-line n/no-unsupported-features/node-builtins import * as http2 from 'http2'; import * as shimmer from 'shimmer'; import {URL} from 'url'; diff --git a/src/plugins/plugin-koa.ts b/src/plugins/plugin-koa.ts index 02a339c8..1fb49b8d 100644 --- a/src/plugins/plugin-koa.ts +++ b/src/plugins/plugin-koa.ts @@ -14,7 +14,7 @@ import {ServerResponse} from 'http'; import * as shimmer from 'shimmer'; -// eslint-disable-next-line node/no-deprecated-api +// eslint-disable-next-line n/no-deprecated-api import {parse as urlParse} from 'url'; import {PluginTypes} from '..'; diff --git a/src/trace-writer.ts b/src/trace-writer.ts index ff35cd61..661701e2 100644 --- a/src/trace-writer.ts +++ b/src/trace-writer.ts @@ -152,7 +152,7 @@ export class TraceWriter extends Service { this.flushBuffer(); if (onUncaughtException === 'flushAndExit') { setTimeout(() => { - // eslint-disable-next-line no-process-exit + // eslint-disable-next-line n/no-process-exit process.exit(1); }, 2000); } diff --git a/test/non-interference/http-e2e.js b/test/non-interference/http-e2e.js index 33602945..ba79ac50 100644 --- a/test/non-interference/http-e2e.js +++ b/test/non-interference/http-e2e.js @@ -44,7 +44,7 @@ const testCommonPath = [ }); if (!testCommonPath) { console.error('No common.js or common/index.js found in test directory'); - // eslint-disable-next-line no-process-exit + // eslint-disable-next-line n/no-process-exit process.exit(1); } cp.execFileSync('sed', [ diff --git a/test/non-interference/start-agent.js b/test/non-interference/start-agent.js index 273ae965..8a6d8553 100644 --- a/test/non-interference/start-agent.js +++ b/test/non-interference/start-agent.js @@ -15,7 +15,7 @@ 'use strict'; const shimmer = require('shimmer'); -// eslint-disable-next-line node/no-missing-require +// eslint-disable-next-line n/no-missing-require const util = require('../../build/src/util.js'); // Stub generateTraceContext so that it always returns the same thing. // This is because web framework unit tests check that similar/identical @@ -29,7 +29,7 @@ shimmer.wrap(util, 'generateTraceContext', () => { return 'ffeeddccbbaa99887766554433221100/0?o=1'; }; }); -// eslint-disable-next-line node/no-missing-require +// eslint-disable-next-line n/no-missing-require require('../..').start({ projectId: '0', logLevel: 1, diff --git a/test/plugins/test-trace-http2.ts b/test/plugins/test-trace-http2.ts index e8e3c468..e6f2bfd1 100644 --- a/test/plugins/test-trace-http2.ts +++ b/test/plugins/test-trace-http2.ts @@ -16,7 +16,7 @@ import * as assert from 'assert'; import {describe, it, after, afterEach, before} from 'mocha'; // This is imported only for types. Generated .js file should NOT load 'http2' // in this place. It is dynamically loaded later from each test suite below. -// eslint-disable-next-line node/no-unsupported-features/node-builtins +// eslint-disable-next-line n/no-unsupported-features/node-builtins import * as http2Types from 'http2'; import * as semver from 'semver'; import * as stream from 'stream'; @@ -46,7 +46,7 @@ maybeSkipHttp2('Trace Agent integration with http2', () => { traceTestModule.setPluginLoaderForTest(); traceTestModule.setCLSForTest(); traceTestModule.start(); - // eslint-disable-next-line node/no-unsupported-features/node-builtins + // eslint-disable-next-line n/no-unsupported-features/node-builtins http2 = require('http2'); }); diff --git a/test/plugins/test-trace-node-fetch.ts b/test/plugins/test-trace-node-fetch.ts index 2380fe50..3f8798dc 100644 --- a/test/plugins/test-trace-node-fetch.ts +++ b/test/plugins/test-trace-node-fetch.ts @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// eslint-disable-next-line node/no-extraneous-import +// eslint-disable-next-line n/no-extraneous-import import * as fetchTypes from 'node-fetch'; // For types only. import * as testTraceModule from '../trace'; import * as assert from 'assert';