From 06701c9a8c7cb4f9e26fde11700aebbc42e48135 Mon Sep 17 00:00:00 2001 From: James Sumners Date: Mon, 23 Sep 2024 10:46:43 -0400 Subject: [PATCH 1/5] chore: Migrate block of unit tests to `node:test` --- test/unit/index.test.js | 502 +++++++++++++++++++++------------------- 1 file changed, 261 insertions(+), 241 deletions(-) diff --git a/test/unit/index.test.js b/test/unit/index.test.js index fc0a0f24f8..ec5f1031c3 100644 --- a/test/unit/index.test.js +++ b/test/unit/index.test.js @@ -1,189 +1,185 @@ /* - * Copyright 2023 New Relic Corporation. All rights reserved. + * Copyright 2024 New Relic Corporation. All rights reserved. * SPDX-License-Identifier: Apache-2.0 */ 'use strict' +const test = require('node:test') +const assert = require('node:assert') + const sinon = require('sinon') -const { test } = require('tap') + +const { match } = require('../lib/custom-assertions') + const proxyquire = require('proxyquire').noCallThru() const createLoggerMock = require('./mocks/logger') const createMockAgent = require('./mocks/agent') const createShimmerMock = require('./mocks/shimmer') const createMetricsMock = require('./mocks/metrics') -test('loader metrics', (t) => { - t.autoend() - let metricsMock - let MockAgent - let shimmerMock - let loggerMock - let ApiMock - let sandbox - - t.beforeEach(() => { - sandbox = sinon.createSandbox() - metricsMock = createMetricsMock(sandbox) - MockAgent = createMockAgent(sandbox, metricsMock) - shimmerMock = createShimmerMock(sandbox) - loggerMock = createLoggerMock(sandbox) - - ApiMock = function (agent) { +test('loader metrics', async (t) => { + t.beforeEach((ctx) => { + const sandbox = sinon.createSandbox() + const metricsMock = createMetricsMock(sandbox) + const MockAgent = createMockAgent(sandbox, metricsMock) + const shimmerMock = createShimmerMock(sandbox) + const loggerMock = createLoggerMock(sandbox) + + const ApiMock = function (agent) { this.agent = agent } + + ctx.nr = { + sandbox, + metricsMock, + MockAgent, + shimmerMock, + loggerMock, + ApiMock + } }) - t.afterEach(() => { + t.afterEach((ctx) => { process.execArgv = [] - sandbox.restore() + ctx.nr.sandbox.restore() delete require.cache.__NR_cache }) - t.test('should load preload metric when agent is loaded via -r', (t) => { + await test('should load preload metric when agent is loaded via -r', (t) => { process.execArgv = ['-r', 'newrelic'] const agent = proxyquire('../../index', { - './lib/agent': MockAgent, - './lib/shimmer': shimmerMock, - './api': ApiMock + './lib/agent': t.nr.MockAgent, + './lib/shimmer': t.nr.shimmerMock, + './api': t.nr.ApiMock }) - const metricCall = agent.agent.metrics.getOrCreateMetric - t.equal(metricCall.args.length, 1) - t.equal(metricCall.args[0][0], 'Supportability/Features/CJS/Preload') - t.end() + assert.equal(metricCall.args.length, 1) + assert.equal(metricCall.args[0][0], 'Supportability/Features/CJS/Preload') }) - t.test('should not load preload metric if -r is present but is not newrelic', (t) => { + await t.test('should not load preload metric if -r is present but is not newrelic', (t) => { process.execArgv = ['-r', 'some-cool-lib'] const agent = proxyquire('../../index', { - './lib/agent': MockAgent, - './lib/shimmer': shimmerMock, - './lib/logger': loggerMock, - './api': ApiMock + './lib/agent': t.nr.MockAgent, + './lib/shimmer': t.nr.shimmerMock, + './lib/logger': t.nr.loggerMock, + './api': t.nr.ApiMock }) const metricCall = agent.agent.metrics.getOrCreateMetric - t.equal(metricCall.args.length, 1) - t.equal(metricCall.args[0][0], 'Supportability/Features/CJS/Require') - t.match( - loggerMock.debug.args[4][1], - /node \-r some-cool-lib.*index\.test\.js/, - 'should log how the agent is called' + assert.equal(metricCall.args.length, 1) + assert.equal(metricCall.args[0][0], 'Supportability/Features/CJS/Require') + assert.equal( + match( + t.nr.loggerMock.debug.args[4][1], + /node -r some-cool-lib.*index\.test\.js/, + 'should log how the agent is called' + ), + true ) - t.end() }) - t.test( + await t.test( 'should detect preload metric if newrelic is one of the -r calls but not the first', (t) => { process.execArgv = ['-r', 'some-cool-lib', '--inspect', '-r', 'newrelic'] const agent = proxyquire('../../index', { - './lib/agent': MockAgent, - './lib/shimmer': shimmerMock, - './api': ApiMock + './lib/agent': t.nr.MockAgent, + './lib/shimmer': t.nr.shimmerMock, + './api': t.nr.ApiMock }) const metricCall = agent.agent.metrics.getOrCreateMetric - t.equal(metricCall.args.length, 1) - t.equal(metricCall.args[0][0], 'Supportability/Features/CJS/Preload') - t.end() + assert.equal(metricCall.args.length, 1) + assert.equal(metricCall.args[0][0], 'Supportability/Features/CJS/Preload') } ) - t.test('should load preload and require metric if is esm loader and -r to load agent', (t) => { - process.execArgv = ['--loader', 'newrelic/esm-loader.mjs', '-r', 'newrelic'] - const agent = proxyquire('../../index', { - './lib/agent': MockAgent, - './lib/shimmer': shimmerMock, - './lib/logger': loggerMock, - './api': ApiMock - }) + await t.test( + 'should load preload and require metric if is esm loader and -r to load agent', + (t) => { + process.execArgv = ['--loader', 'newrelic/esm-loader.mjs', '-r', 'newrelic'] + const agent = proxyquire('../../index', { + './lib/agent': t.nr.MockAgent, + './lib/shimmer': t.nr.shimmerMock, + './lib/logger': t.nr.loggerMock, + './api': t.nr.ApiMock + }) - const metricCall = agent.agent.metrics.getOrCreateMetric + const metricCall = agent.agent.metrics.getOrCreateMetric - t.equal(metricCall.args.length, 2) - t.equal(metricCall.args[0][0], 'Supportability/Features/ESM/Loader') - t.equal(metricCall.args[1][0], 'Supportability/Features/CJS/Preload') - t.match( - loggerMock.debug.args[4][1], - /node \-\-loader newrelic\/esm-loader.mjs \-r newrelic.*index\.test\.js/, - 'should log how the agent is called' - ) - t.end() - }) + assert.equal(metricCall.args.length, 2) + assert.equal(metricCall.args[0][0], 'Supportability/Features/ESM/Loader') + assert.equal(metricCall.args[1][0], 'Supportability/Features/CJS/Preload') + assert.equal( + match( + t.nr.loggerMock.debug.args[4][1], + /node --loader newrelic\/esm-loader.mjs -r newrelic.*index\.test\.js/, + 'should log how the agent is called' + ), + true + ) + } + ) - t.test('should load preload and require metric if esm loader and require of agent', (t) => { - process.execArgv = ['--loader', 'newrelic/esm-loader.mjs'] - const agent = proxyquire('../../index', { - './lib/agent': MockAgent, - './lib/shimmer': shimmerMock, - './api': ApiMock - }) + await t.test( + 'should load preload and require metric if esm loader and require of agent', + (t) => { + process.execArgv = ['--loader', 'newrelic/esm-loader.mjs'] + const agent = proxyquire('../../index', { + './lib/agent': t.nr.MockAgent, + './lib/shimmer': t.nr.shimmerMock, + './api': t.nr.ApiMock + }) - const metricCall = agent.agent.metrics.getOrCreateMetric + const metricCall = agent.agent.metrics.getOrCreateMetric - t.equal(metricCall.args.length, 2) - t.equal(metricCall.args[0][0], 'Supportability/Features/ESM/Loader') - t.equal(metricCall.args[1][0], 'Supportability/Features/CJS/Require') - t.end() - }) + assert.equal(metricCall.args.length, 2) + assert.equal(metricCall.args[0][0], 'Supportability/Features/ESM/Loader') + assert.equal(metricCall.args[1][0], 'Supportability/Features/CJS/Require') + } + ) - t.test('should load require metric when agent is required', (t) => { + await t.test('should load require metric when agent is required', (t) => { const agent = proxyquire('../../index', { - './lib/agent': MockAgent, - './lib/shimmer': shimmerMock, - './api': ApiMock + './lib/agent': t.nr.MockAgent, + './lib/shimmer': t.nr.shimmerMock, + './api': t.nr.ApiMock }) const metricCall = agent.agent.metrics.getOrCreateMetric - t.equal(metricCall.args.length, 1) - t.equal(metricCall.args[0][0], 'Supportability/Features/CJS/Require') - t.end() + assert.equal(metricCall.args.length, 1) + assert.equal(metricCall.args[0][0], 'Supportability/Features/CJS/Require') }) - t.test('should load enable source map metric when --enable-source-maps is present', (t) => { + await t.test('should load enable source map metric when --enable-source-maps is present', (t) => { process.execArgv = ['--enable-source-maps'] const agent = proxyquire('../../index', { - './lib/agent': MockAgent, - './lib/shimmer': shimmerMock, - './api': ApiMock + './lib/agent': t.nr.MockAgent, + './lib/shimmer': t.nr.shimmerMock, + './api': t.nr.ApiMock }) const metricCall = agent.agent.metrics.getOrCreateMetric - t.equal(metricCall.args.length, 2) - t.equal(metricCall.args[1][0], 'Supportability/Features/EnableSourceMaps') - t.end() + assert.equal(metricCall.args.length, 2) + assert.equal(metricCall.args[1][0], 'Supportability/Features/EnableSourceMaps') }) }) -test('index tests', (t) => { - t.autoend() - let sandbox - let loggerMock - let processVersionStub - let configMock - let mockConfig - let MockAgent - let k2Stub - let shimmerMock - let metricsMock - let workerThreadsStub - - t.beforeEach(() => { - sandbox = sinon.createSandbox() - metricsMock = createMetricsMock(sandbox) - MockAgent = createMockAgent(sandbox, metricsMock) - processVersionStub = { - satisfies: sandbox.stub() - } - loggerMock = createLoggerMock(sandbox) - mockConfig = { +test('index tests', async (t) => { + t.beforeEach((ctx) => { + const sandbox = sinon.createSandbox() + const metricsMock = createMetricsMock(sandbox) + const MockAgent = createMockAgent(sandbox, metricsMock) + const processVersionStub = { satisfies: sandbox.stub() } + const loggerMock = createLoggerMock(sandbox) + const mockConfig = { applications: sandbox.stub(), agent_enabled: true, logging: {}, @@ -191,180 +187,204 @@ test('index tests', (t) => { security: { agent: { enabled: false } }, worker_threads: { enabled: false } } - configMock = { + const configMock = { getOrCreateInstance: sandbox.stub().returns(mockConfig) } - workerThreadsStub = { - isMainThread: true - } + const workerThreadsStub = { isMainThread: true } + const k2Stub = { start: sandbox.stub() } + sandbox.stub(console, 'error') - k2Stub = { start: sandbox.stub() } processVersionStub.satisfies.onCall(0).returns(true) processVersionStub.satisfies.onCall(1).returns(false) mockConfig.applications.returns(['my-app-name']) MockAgent.prototype.start.yields(null) - shimmerMock = createShimmerMock(sandbox) + const shimmerMock = createShimmerMock(sandbox) + + ctx.nr = { + sandbox, + metricsMock, + MockAgent, + processVersionStub, + loggerMock, + mockConfig, + configMock, + workerThreadsStub, + k2Stub, + shimmerMock + } }) - function loadIndex() { + t.afterEach((ctx) => { + ctx.nr.sandbox.restore() + delete require.cache.__NR_cache + }) + + function loadIndex(ctx) { return proxyquire('../../index', { - 'worker_threads': workerThreadsStub, - './lib/util/process-version': processVersionStub, - './lib/logger': loggerMock, - './lib/agent': MockAgent, - './lib/config': configMock, - './lib/shimmer': shimmerMock, - '@newrelic/security-agent': k2Stub + 'worker_threads': ctx.nr.workerThreadsStub, + './lib/util/process-version': ctx.nr.processVersionStub, + './lib/logger': ctx.nr.loggerMock, + './lib/agent': ctx.nr.MockAgent, + './lib/config': ctx.nr.configMock, + './lib/shimmer': ctx.nr.shimmerMock, + '@newrelic/security-agent': ctx.nr.k2Stub }) } - t.afterEach(() => { - sandbox.restore() - delete require.cache.__NR_cache - }) - - t.test('should properly register when agent starts and add appropriate metrics', (t) => { - const api = loadIndex() + await t.test('should properly register when agent starts and add appropriate metrics', (t) => { + const api = loadIndex(t) const version = /^v(\d+)/.exec(process.version) - t.equal(api.agent.recordSupportability.callCount, 5, 'should log 5 supportability metrics') - t.equal(api.agent.recordSupportability.args[0][0], `Nodejs/Version/${version[1]}`) - t.equal(api.agent.recordSupportability.args[1][0], 'Nodejs/FeatureFlag/flag_1/enabled') - t.equal(api.agent.recordSupportability.args[2][0], 'Nodejs/FeatureFlag/flag_2/disabled') - t.equal(api.agent.recordSupportability.args[3][0], 'Nodejs/Application/Opening/Duration') - t.equal(api.agent.recordSupportability.args[4][0], 'Nodejs/Application/Initialization/Duration') + assert.equal(api.agent.recordSupportability.callCount, 5, 'should log 5 supportability metrics') + assert.equal(api.agent.recordSupportability.args[0][0], `Nodejs/Version/${version[1]}`) + assert.equal(api.agent.recordSupportability.args[1][0], 'Nodejs/FeatureFlag/flag_1/enabled') + assert.equal(api.agent.recordSupportability.args[2][0], 'Nodejs/FeatureFlag/flag_2/disabled') + assert.equal(api.agent.recordSupportability.args[3][0], 'Nodejs/Application/Opening/Duration') + assert.equal( + api.agent.recordSupportability.args[4][0], + 'Nodejs/Application/Initialization/Duration' + ) api.agent.emit('started') - t.equal(api.agent.recordSupportability.args[5][0], 'Nodejs/Application/Registration/Duration') - t.equal(k2Stub.start.callCount, 0, 'should not register security agent') - t.equal(loggerMock.debug.callCount, 6, 'should log 6 debug messages') - t.end() + assert.equal( + api.agent.recordSupportability.args[5][0], + 'Nodejs/Application/Registration/Duration' + ) + assert.equal(t.nr.k2Stub.start.callCount, 0, 'should not register security agent') + assert.equal(t.nr.loggerMock.debug.callCount, 6, 'should log 6 debug messages') }) - t.test('should set api on require.cache.__NR_cache', (t) => { - const api = loadIndex() - t.same(require.cache.__NR_cache, api) - t.end() + await t.test('should set api on require.cache.__NR_cache', (t) => { + const api = loadIndex(t) + assert.equal(match(require.cache.__NR_cache, api), true) }) - t.test('should load k2 agent if config.security.agent.enabled', (t) => { - mockConfig.security.agent.enabled = true - const api = loadIndex() - t.equal(k2Stub.start.callCount, 1, 'should register security agent') - t.same(k2Stub.start.args[0][0], api, 'should call start on security agent with proper args') - t.end() + await t.test('should load k2 agent if config.security.agent.enabled', (t) => { + t.nr.mockConfig.security.agent.enabled = true + const api = loadIndex(t) + assert.equal(t.nr.k2Stub.start.callCount, 1, 'should register security agent') + assert.equal( + match( + t.nr.k2Stub.start.args[0][0], + api, + 'should call start on security agent with proper args' + ), + true + ) }) - t.test('should record double load when NR_cache and agent exist on NR_cache', (t) => { - const mockAgent = new MockAgent() + await t.test('should record double load when NR_cache and agent exist on NR_cache', (t) => { + const mockAgent = new t.nr.MockAgent() require.cache.__NR_cache = { agent: mockAgent } - loadIndex() - t.equal(mockAgent.recordSupportability.callCount, 1, 'should record double load') - t.equal(mockAgent.recordSupportability.args[0][0], 'Agent/DoubleLoad') - t.equal(loggerMock.debug.callCount, 0) - t.end() + loadIndex(t) + assert.equal(mockAgent.recordSupportability.callCount, 1, 'should record double load') + assert.equal(mockAgent.recordSupportability.args[0][0], 'Agent/DoubleLoad') + assert.equal(t.nr.loggerMock.debug.callCount, 0) }) - t.test('should throw error if using an unsupported version of Node.js', (t) => { - processVersionStub.satisfies.onCall(0).returns(false) - loadIndex() - t.equal(loggerMock.error.callCount, 1, 'should log an error') - t.match(loggerMock.error.args[0][0], /New Relic for Node.js requires a version of Node/) - t.end() + await t.test('should throw error if using an unsupported version of Node.js', (t) => { + t.nr.processVersionStub.satisfies.onCall(0).returns(false) + loadIndex(t) + assert.equal(t.nr.loggerMock.error.callCount, 1, 'should log an error') + assert.equal( + match(t.nr.loggerMock.error.args[0][0], /New Relic for Node.js requires a version of Node/), + true + ) }) - t.test('should log warning if using an odd version of node', (t) => { - processVersionStub.satisfies.onCall(0).returns(true) - processVersionStub.satisfies.onCall(1).returns(true) - configMock.getOrCreateInstance.returns(null) - loadIndex() - t.equal(loggerMock.warn.callCount, 1, 'should log an error') - t.match(loggerMock.warn.args[0][0], /New Relic for Node\.js.*has not been tested on Node/) - t.end() + await t.test('should log warning if using an odd version of node', (t) => { + t.nr.processVersionStub.satisfies.onCall(0).returns(true) + t.nr.processVersionStub.satisfies.onCall(1).returns(true) + t.nr.configMock.getOrCreateInstance.returns(null) + loadIndex(t) + assert.equal(t.nr.loggerMock.warn.callCount, 1, 'should log an error') + assert.equal( + match(t.nr.loggerMock.warn.args[0][0], /New Relic for Node\.js.*has not been tested on Node/), + true + ) }) - t.test('should use stub api if no config detected', (t) => { - configMock.getOrCreateInstance.returns(null) - processVersionStub.satisfies.onCall(0).returns(true) - processVersionStub.satisfies.onCall(1).returns(false) - const api = loadIndex() - t.equal(loggerMock.info.callCount, 2, 'should log info logs') - t.equal(loggerMock.info.args[1][0], 'No configuration detected. Not starting.') - t.equal(api.constructor.name, 'Stub') - t.end() + await t.test('should use stub api if no config detected', (t) => { + t.nr.configMock.getOrCreateInstance.returns(null) + t.nr.processVersionStub.satisfies.onCall(0).returns(true) + t.nr.processVersionStub.satisfies.onCall(1).returns(false) + const api = loadIndex(t) + assert.equal(t.nr.loggerMock.info.callCount, 2, 'should log info logs') + assert.equal(t.nr.loggerMock.info.args[1][0], 'No configuration detected. Not starting.') + assert.equal(api.constructor.name, 'Stub') }) - t.test('should use stub api if agent_enabled is false', (t) => { - configMock.getOrCreateInstance.returns({ agent_enabled: false }) - processVersionStub.satisfies.onCall(0).returns(true) - processVersionStub.satisfies.onCall(1).returns(false) - const api = loadIndex() - t.equal(loggerMock.info.callCount, 2, 'should log info logs') - t.equal(loggerMock.info.args[1][0], 'Module disabled in configuration. Not starting.') - t.equal(api.constructor.name, 'Stub') - t.end() + await t.test('should use stub api if agent_enabled is false', (t) => { + t.nr.configMock.getOrCreateInstance.returns({ agent_enabled: false }) + t.nr.processVersionStub.satisfies.onCall(0).returns(true) + t.nr.processVersionStub.satisfies.onCall(1).returns(false) + const api = loadIndex(t) + assert.equal(t.nr.loggerMock.info.callCount, 2, 'should log info logs') + assert.equal(t.nr.loggerMock.info.args[1][0], 'Module disabled in configuration. Not starting.') + assert.equal(api.constructor.name, 'Stub') }) - t.test('should log warning when logging diagnostics is enabled', (t) => { - mockConfig.logging.diagnostics = true - processVersionStub.satisfies.onCall(0).returns(true) - processVersionStub.satisfies.onCall(1).returns(false) - loadIndex() - t.equal( - loggerMock.warn.args[0][0], + await t.test('should log warning when logging diagnostics is enabled', (t) => { + t.nr.mockConfig.logging.diagnostics = true + t.nr.processVersionStub.satisfies.onCall(0).returns(true) + t.nr.processVersionStub.satisfies.onCall(1).returns(false) + loadIndex(t) + assert.equal( + t.nr.loggerMock.warn.args[0][0], 'Diagnostics logging is enabled, this may cause significant overhead.' ) - t.end() }) - t.test('should throw error is app name is not set in config', (t) => { - processVersionStub.satisfies.onCall(0).returns(true) - processVersionStub.satisfies.onCall(1).returns(false) - mockConfig.applications.returns([]) - loadIndex() - t.equal(loggerMock.error.callCount, 1, 'should log an error') - t.match(loggerMock.error.args[0][0], /New Relic requires that you name this application!/) - t.end() + await t.test('should throw error is app name is not set in config', (t) => { + t.nr.processVersionStub.satisfies.onCall(0).returns(true) + t.nr.processVersionStub.satisfies.onCall(1).returns(false) + t.nr.mockConfig.applications.returns([]) + loadIndex(t) + assert.equal(t.nr.loggerMock.error.callCount, 1, 'should log an error') + assert.equal( + match(t.nr.loggerMock.error.args[0][0], /New Relic requires that you name this application!/), + true + ) }) - t.test('should log error if agent startup failed', (t) => { - processVersionStub.satisfies.onCall(0).returns(true) - processVersionStub.satisfies.onCall(1).returns(false) - mockConfig.applications.returns(['my-app-name']) + await t.test('should log error if agent startup failed', (t) => { + t.nr.processVersionStub.satisfies.onCall(0).returns(true) + t.nr.processVersionStub.satisfies.onCall(1).returns(false) + t.nr.mockConfig.applications.returns(['my-app-name']) const err = new Error('agent start failed') - MockAgent.prototype.start.yields(err) - loadIndex() - t.equal(loggerMock.error.callCount, 1, 'should log a startup error') - t.equal(loggerMock.error.args[0][1], 'New Relic for Node.js halted startup due to an error:') - t.end() + t.nr.MockAgent.prototype.start.yields(err) + loadIndex(t) + assert.equal(t.nr.loggerMock.error.callCount, 1, 'should log a startup error') + assert.equal( + t.nr.loggerMock.error.args[0][1], + 'New Relic for Node.js halted startup due to an error:' + ) }) - t.test('should log warning if not in main thread and make a stub api', (t) => { - workerThreadsStub.isMainThread = false - const api = loadIndex() - t.equal(loggerMock.warn.callCount, 1) - t.equal( - loggerMock.warn.args[0][0], + await t.test('should log warning if not in main thread and make a stub api', (t) => { + t.nr.workerThreadsStub.isMainThread = false + const api = loadIndex(t) + assert.equal(t.nr.loggerMock.warn.callCount, 1) + assert.equal( + t.nr.loggerMock.warn.args[0][0], 'New Relic for Node.js in worker_threads is not officially supported. Not starting! To bypass this, set `config.worker_threads.enabled` to true in configuration.' ) - t.not(api.agent, 'should not initialize an agent') - t.equal(api.constructor.name, 'Stub') - t.end() + assert.equal(api.agent, undefined, 'should not initialize an agent') + assert.equal(api.constructor.name, 'Stub') }) - t.test( + await t.test( 'should log warning if not in main thread and worker_threads.enabled is true and init agent', (t) => { - mockConfig.worker_threads.enabled = true - workerThreadsStub.isMainThread = false - const api = loadIndex() - t.equal(loggerMock.warn.callCount, 1) - t.equal( - loggerMock.warn.args[0][0], + t.nr.mockConfig.worker_threads.enabled = true + t.nr.workerThreadsStub.isMainThread = false + const api = loadIndex(t) + assert.equal(t.nr.loggerMock.warn.callCount, 1) + assert.equal( + t.nr.loggerMock.warn.args[0][0], 'Attempting to load agent in worker thread. This is not officially supported. Use at your own risk.' ) - t.ok(api.agent) - t.equal(api.agent.constructor.name, 'MockAgent', 'should initialize an agent') - t.equal(api.constructor.name, 'API') - t.end() + assert.ok(api.agent) + assert.equal(api.agent.constructor.name, 'MockAgent', 'should initialize an agent') + assert.equal(api.constructor.name, 'API') } ) }) From a9d27466446c5bf9548847d0e28e6cf389ea9a1d Mon Sep 17 00:00:00 2001 From: James Sumners Date: Mon, 23 Sep 2024 10:48:27 -0400 Subject: [PATCH 2/5] update instrumentation-descriptor tests --- test/unit/instrumentation-descriptor.test.js | 24 +++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/test/unit/instrumentation-descriptor.test.js b/test/unit/instrumentation-descriptor.test.js index bf9c02d846..bcd0008255 100644 --- a/test/unit/instrumentation-descriptor.test.js +++ b/test/unit/instrumentation-descriptor.test.js @@ -5,10 +5,12 @@ 'use strict' -const tap = require('tap') +const test = require('node:test') +const assert = require('node:assert') + const InstrumentationDescriptor = require('../../lib/instrumentation-descriptor') -tap.test('constructs instances', async (t) => { +test('constructs instances', async () => { const desc = new InstrumentationDescriptor({ type: 'generic', module: 'foo', @@ -19,17 +21,17 @@ tap.test('constructs instances', async (t) => { onError }) - t.equal(desc.type, InstrumentationDescriptor.TYPE_GENERIC) - t.equal(desc.module, 'foo') - t.equal(desc.moduleName, 'foo') - t.equal(desc.absolutePath, '/foo') - t.equal(desc.resolvedName, '/opt/app/node_modules/foo') - t.equal(desc.onRequire, onRequire) - t.equal(desc.onError, onError) - t.equal(desc.instrumentationId, 0) + assert.equal(desc.type, InstrumentationDescriptor.TYPE_GENERIC) + assert.equal(desc.module, 'foo') + assert.equal(desc.moduleName, 'foo') + assert.equal(desc.absolutePath, '/foo') + assert.equal(desc.resolvedName, '/opt/app/node_modules/foo') + assert.equal(desc.onRequire, onRequire) + assert.equal(desc.onError, onError) + assert.equal(desc.instrumentationId, 0) const desc2 = new InstrumentationDescriptor({ moduleName: 'foo' }) - t.equal(desc2.instrumentationId, 1) + assert.equal(desc2.instrumentationId, 1) function onRequire() {} function onError() {} From 7ebbcf8cbbb5d9fbc87fd41ccdd35aa13ee2e191 Mon Sep 17 00:00:00 2001 From: James Sumners Date: Mon, 23 Sep 2024 10:56:04 -0400 Subject: [PATCH 3/5] update instrumentation-tracker tests --- test/unit/instrumentation-tracker.test.js | 81 ++++++++++++----------- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/test/unit/instrumentation-tracker.test.js b/test/unit/instrumentation-tracker.test.js index 8d72e50c4d..12c2dc0db3 100644 --- a/test/unit/instrumentation-tracker.test.js +++ b/test/unit/instrumentation-tracker.test.js @@ -5,86 +5,91 @@ 'use strict' -const tap = require('tap') +const test = require('node:test') +const assert = require('node:assert') + +const { match } = require('../lib/custom-assertions') + const InstrumentationTracker = require('../../lib/instrumentation-tracker') const InstrumentationDescriptor = require('../../lib/instrumentation-descriptor') -tap.test('can inspect object type', async (t) => { +test('can inspect object type', async () => { const tracker = new InstrumentationTracker() - t.equal(Object.prototype.toString.call(tracker), '[object InstrumentationTracker]') + assert.equal(Object.prototype.toString.call(tracker), '[object InstrumentationTracker]') }) -tap.test('track method tracks new items and updates existing ones', async (t) => { +test('track method tracks new items and updates existing ones', async () => { const tracker = new InstrumentationTracker() const inst1 = new InstrumentationDescriptor({ moduleName: 'foo' }) tracker.track('foo', inst1) - t.equal(tracker.getAllByName('foo').length, 1) + assert.equal(tracker.getAllByName('foo').length, 1) // Module already tracked and instrumentation id is the same. tracker.track('foo', inst1) - t.equal(tracker.getAllByName('foo').length, 1) + assert.equal(tracker.getAllByName('foo').length, 1) // Module already tracked, but new instrumentation with different id. const inst2 = new InstrumentationDescriptor({ moduleName: 'foo' }) tracker.track('foo', inst2) - t.equal(tracker.getAllByName('foo').length, 2) + assert.equal(tracker.getAllByName('foo').length, 2) }) -tap.test('can get a tracked item by instrumentation', async (t) => { +test('can get a tracked item by instrumentation', async () => { const tracker = new InstrumentationTracker() const inst = new InstrumentationDescriptor({ moduleName: 'foo' }) tracker.track('foo', inst) const item = tracker.getTrackedItem('foo', inst) - t.equal(item.instrumentation, inst) - t.same(item.meta, { instrumented: false, didError: undefined }) + assert.equal(item.instrumentation, inst) + assert.equal(match(item.meta, { instrumented: false, didError: undefined }), true) }) -tap.test('sets hook failure correctly', async (t) => { +test('sets hook failure correctly', async () => { const tracker = new InstrumentationTracker() const inst = new InstrumentationDescriptor({ moduleName: 'foo' }) tracker.track('foo', inst) const item = tracker.getTrackedItem('foo', inst) tracker.setHookFailure(item) - t.equal(item.meta.instrumented, false) - t.equal(item.meta.didError, true) + assert.equal(item.meta.instrumented, false) + assert.equal(item.meta.didError, true) // Double check that the item in the map got updated. const items = tracker.getAllByName('foo') - t.equal(items[0].meta.instrumented, false) - t.equal(items[0].meta.didError, true) + assert.equal(items[0].meta.instrumented, false) + assert.equal(items[0].meta.didError, true) }) -tap.test('sets hook success correctly', async (t) => { +test('sets hook success correctly', async () => { const tracker = new InstrumentationTracker() const inst = new InstrumentationDescriptor({ moduleName: 'foo' }) tracker.track('foo', inst) const item = tracker.getTrackedItem('foo', inst) tracker.setHookSuccess(item) - t.equal(item.meta.instrumented, true) - t.equal(item.meta.didError, false) + assert.equal(item.meta.instrumented, true) + assert.equal(item.meta.didError, false) // Double check that the item in the map got updated. const items = tracker.getAllByName('foo') - t.equal(items[0].meta.instrumented, true) - t.equal(items[0].meta.didError, false) + assert.equal(items[0].meta.instrumented, true) + assert.equal(items[0].meta.didError, false) }) -tap.test('setResolvedName', (t) => { - t.beforeEach((t) => { - t.context.tracker = new InstrumentationTracker() +test('setResolvedName', async (t) => { + t.beforeEach((ctx) => { + ctx.nr = {} + ctx.nr.tracker = new InstrumentationTracker() }) - t.test('throws expected error', async (t) => { - const { tracker } = t.context - t.throws(() => tracker.setResolvedName('foo', 'bar'), 'module not tracked: foo') + await t.test('throws expected error', (t) => { + const { tracker } = t.nr + assert.throws(() => tracker.setResolvedName('foo', 'bar'), Error('module not tracked: foo')) }) - t.test('skips existing tracked items', async (t) => { - const { tracker } = t.context + await t.test('skips existing tracked items', (t) => { + const { tracker } = t.nr const inst = new InstrumentationDescriptor({ moduleName: 'foo', resolvedName: '/opt/app/node_modules/foo' @@ -92,11 +97,11 @@ tap.test('setResolvedName', (t) => { tracker.track('foo', inst) tracker.setResolvedName('foo', '/opt/app/node_modules/foo') - t.equal(tracker.getAllByName('foo').length, 1) + assert.equal(tracker.getAllByName('foo').length, 1) }) - t.test('adds new tracked item for new resolved name', async (t) => { - const { tracker } = t.context + await t.test('adds new tracked item for new resolved name', (t) => { + const { tracker } = t.nr const inst1 = new InstrumentationDescriptor({ moduleName: 'foo', resolvedName: '/opt/app/node_modules/foo' @@ -106,15 +111,15 @@ tap.test('setResolvedName', (t) => { tracker.setResolvedName('foo', '/opt/app/node_modules/transitive-dep/node_modules/foo') const items = tracker.getAllByName('foo') - t.equal(items[0].instrumentation.resolvedName, '/opt/app/node_modules/foo') - t.equal( + assert.equal(items[0].instrumentation.resolvedName, '/opt/app/node_modules/foo') + assert.equal( items[1].instrumentation.resolvedName, '/opt/app/node_modules/transitive-dep/node_modules/foo' ) }) - t.test('updates all registered instrumentations with resolve name', async (t) => { - const { tracker } = t.context + await t.test('updates all registered instrumentations with resolve name', (t) => { + const { tracker } = t.nr const inst1 = new InstrumentationDescriptor({ moduleName: 'foo' }) const inst2 = new InstrumentationDescriptor({ moduleName: 'foo' }) @@ -123,9 +128,7 @@ tap.test('setResolvedName', (t) => { tracker.setResolvedName('foo', '/opt/app/node_modules/foo') const items = tracker.getAllByName('foo') - t.equal(items[0].instrumentation.resolvedName, '/opt/app/node_modules/foo') - t.equal(items[1].instrumentation.resolvedName, '/opt/app/node_modules/foo') + assert.equal(items[0].instrumentation.resolvedName, '/opt/app/node_modules/foo') + assert.equal(items[1].instrumentation.resolvedName, '/opt/app/node_modules/foo') }) - - t.end() }) From adf3ae2ac57932a544d0f5b23d27fb91436b775a Mon Sep 17 00:00:00 2001 From: James Sumners Date: Mon, 23 Sep 2024 10:57:14 -0400 Subject: [PATCH 4/5] update is-absolute-path tests --- test/unit/is-absolute-path.test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/unit/is-absolute-path.test.js b/test/unit/is-absolute-path.test.js index 5510c63954..14f11975d9 100644 --- a/test/unit/is-absolute-path.test.js +++ b/test/unit/is-absolute-path.test.js @@ -5,10 +5,12 @@ 'use strict' -const tap = require('tap') +const test = require('node:test') +const assert = require('node:assert') + const isAbsolutePath = require('../../lib/util/is-absolute-path') -tap.test('verifies paths correctly', async (t) => { +test('verifies paths correctly', async () => { const tests = [ ['./foo/bar.js', true], ['/foo/bar.cjs', true], @@ -19,6 +21,6 @@ tap.test('verifies paths correctly', async (t) => { ] for (const [input, expected] of tests) { - t.equal(isAbsolutePath(input), expected) + assert.equal(isAbsolutePath(input), expected) } }) From 337f971be770260320fb4039f2a5c9be0e069e30 Mon Sep 17 00:00:00 2001 From: James Sumners Date: Tue, 24 Sep 2024 10:02:54 -0400 Subject: [PATCH 5/5] address feedback --- test/unit/{ => util}/is-absolute-path.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename test/unit/{ => util}/is-absolute-path.test.js (88%) diff --git a/test/unit/is-absolute-path.test.js b/test/unit/util/is-absolute-path.test.js similarity index 88% rename from test/unit/is-absolute-path.test.js rename to test/unit/util/is-absolute-path.test.js index 14f11975d9..f7ed30b6aa 100644 --- a/test/unit/is-absolute-path.test.js +++ b/test/unit/util/is-absolute-path.test.js @@ -8,7 +8,7 @@ const test = require('node:test') const assert = require('node:assert') -const isAbsolutePath = require('../../lib/util/is-absolute-path') +const isAbsolutePath = require('../../../lib/util/is-absolute-path') test('verifies paths correctly', async () => { const tests = [