From 8ac0f9a001ee01b86a07ed0ef29c403e3773491d Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Fri, 6 Dec 2024 14:15:45 -0600 Subject: [PATCH 1/6] updated unit tests to use node test --- package.json | 2 +- tests/unit/gc-metrics.tap.js | 46 ---------------- tests/unit/gc-metrics.test.js | 44 +++++++++++++++ tests/unit/loop-metrics.tap.js | 97 --------------------------------- tests/unit/loop-metrics.test.js | 82 ++++++++++++++++++++++++++++ 5 files changed, 127 insertions(+), 144 deletions(-) delete mode 100644 tests/unit/gc-metrics.tap.js create mode 100644 tests/unit/gc-metrics.test.js delete mode 100644 tests/unit/loop-metrics.tap.js create mode 100644 tests/unit/loop-metrics.test.js diff --git a/package.json b/package.json index 0a4bf7b..5b1bd60 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "lint": "eslint .", "lint:fix": "eslint . --fix", "lint:lockfile": "lockfile-lint --path package-lock.json --type npm --allowed-hosts npm --validate-https --validate-integrity", - "unit": "c8 -o ./coverage/unit tap --expose-gc --jobs=1 --no-coverage tests/unit/*.tap.js", + "unit": "c8 -o ./coverage/unit node --test --expose-gc tests/unit/*.test.js", "integration": "c8 -o ./coverage/integration tap --timeout 360000 --jobs=1 --no-coverage tests/integration/*.tap.js", "native": "node tests/native/*.js", "test": "npm run unit && npm run integration", diff --git a/tests/unit/gc-metrics.tap.js b/tests/unit/gc-metrics.tap.js deleted file mode 100644 index d1863fa..0000000 --- a/tests/unit/gc-metrics.tap.js +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const tap = require('tap') - -tap.test('GC Metrics', function (t) { - t.plan(17) - const metricEmitter = require('../../')() - - global.gc() - - const gcs = metricEmitter.getGCMetrics() - const keys = Object.keys(gcs) - if (!t.ok(keys.length > 0, 'should notice at least one GC')) { - return t.end() - } - t.type(keys[0], 'string', 'should have strings as keys') - - t.comment('GC stats objects') - const stats = gcs[keys[0]] - t.type(stats, 'object', 'should have stats objects') - t.type(stats.typeId, 'number', 'should have the type ID') - t.type(stats.type, 'string', 'should have the type name') - if (!t.type(stats.metrics, 'object', 'should have a metrics object')) { - return t.end() - } - - t.comment('GC stats metrics') - const metrics = stats.metrics - t.type(metrics.total, 'number', 'should have total field') - t.type(metrics.min, 'number', 'should have min field') - t.type(metrics.max, 'number', 'should have max field') - t.type(metrics.sumOfSquares, 'number', 'should have sumOfSquares field') - t.type(metrics.count, 'number', 'should have count field') - - t.ok(metrics.total > 0, 'should have reasonable values for total') - t.ok(metrics.min > 0, 'should have reasonable values for min') - t.ok(metrics.max > 0, 'should have reasonable values for max') - t.ok(metrics.max >= metrics.min, 'should have a max larger than a min') - t.ok(metrics.sumOfSquares > 0, 'should have reasonable values for sumOfSquares') - t.ok(metrics.count > 0, 'should have reasonable values for count') -}) diff --git a/tests/unit/gc-metrics.test.js b/tests/unit/gc-metrics.test.js new file mode 100644 index 0000000..2af93ae --- /dev/null +++ b/tests/unit/gc-metrics.test.js @@ -0,0 +1,44 @@ +/* + * Copyright 2020 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +const test = require('node:test') +const assert = require('node:assert') + +test('GC Metrics', async (t) => { + const metricEmitter = require('../..')() + + global.gc() + + const gcs = metricEmitter.getGCMetrics() + const keys = Object.keys(gcs) + assert.ok(keys.length > 0, 'should notice at least one GC') + assert.strictEqual(typeof keys[0], 'string', 'should have strings as keys') + + await t.test('GC stats objects', () => { + const stats = gcs[keys[0]] + assert.strictEqual(typeof stats, 'object', 'should have stats objects') + assert.strictEqual(typeof stats.typeId, 'number', 'should have the type ID') + assert.strictEqual(typeof stats.type, 'string', 'should have the type name') + assert.strictEqual(typeof stats.metrics, 'object', 'should have a metrics object') + }) + + await t.test('GC stats metrics', () => { + const metrics = gcs[keys[0]].metrics + assert.strictEqual(typeof metrics.total, 'number', 'should have total field') + assert.strictEqual(typeof metrics.min, 'number', 'should have min field') + assert.strictEqual(typeof metrics.max, 'number', 'should have max field') + assert.strictEqual(typeof metrics.sumOfSquares, 'number', 'should have sumOfSquares field') + assert.strictEqual(typeof metrics.count, 'number', 'should have count field') + + assert.ok(metrics.total > 0, 'should have reasonable values for total') + assert.ok(metrics.min > 0, 'should have reasonable values for min') + assert.ok(metrics.max > 0, 'should have reasonable values for max') + assert.ok(metrics.max >= metrics.min, 'should have a max larger than a min') + assert.ok(metrics.sumOfSquares > 0, 'should have reasonable values for sumOfSquares') + assert.ok(metrics.count > 0, 'should have reasonable values for count') + }) +}) diff --git a/tests/unit/loop-metrics.tap.js b/tests/unit/loop-metrics.tap.js deleted file mode 100644 index bff022e..0000000 --- a/tests/unit/loop-metrics.tap.js +++ /dev/null @@ -1,97 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const tap = require('tap') - -tap.test('Loop Metrics', function (t) { - const MICRO_TO_MILLIS = 1e-3 - const SPIN_TIME = 2000 - const CPU_EPSILON = SPIN_TIME * 0.05 // Allowed fudge factor for CPU times in MS - const metricEmitter = require('../../')({ timeout: 10 }) - const testStart = Date.now() - - t.teardown(function () { - metricEmitter.unbind() - }) - - // Check the structure of the metric object. - let metric = metricEmitter.getLoopMetrics().usage - t.type(metric, Object, 'should provide a metric object') - t.type(metric.total, 'number', 'should have a total') - t.type(metric.min, 'number', 'should have a min') - t.type(metric.max, 'number', 'should have a max') - t.type(metric.sumOfSquares, 'number', 'should have a sumOfSquares') - t.type(metric.count, 'number', 'should have a count') - - // Short circuit if the structure isn't correct. - if (!t.passing()) { - return t.end() - } - - // Check that the values are reset after the first call. Since this is - // synchronous with the previous call, all results should be zero. - metric = metricEmitter.getLoopMetrics().usage - t.same( - metric, - { - total: 0, - min: 0, - max: 0, - sumOfSquares: 0, - count: 0 - }, - 'should reset all the values' - ) - - // Queue up a loop with some CPU burn. - // XXX Keep this as a timeout. On Node v4 it causes an extra period of idle - // wait on IO due to a bug in timers. This time should not be counted in - // the actual loop time because the process isn't doing anything. - setTimeout(spinner, 100) - - function spinner() { - t.comment('spinning cpu...') - const start = Date.now() - while (Date.now() - start < SPIN_TIME) {} // Spin the CPU for 2 seconds. - - // Finally, wait another tick and then check the loop stats. - setTimeout(afterSpin, 100) - } - - function afterSpin() { - metric = metricEmitter.getLoopMetrics() - const testDuration = Date.now() - testStart + CPU_EPSILON - const durationSquare = testDuration * testDuration - const usage = metric.usage - - const meanTime = usage.total / usage.count - if (process.arch === 'arm64') { - t.comment( - `{ min: ${usage.min}, max: ${usage.max}, meanTime: ${meanTime}, count: ${usage.count}, total: ${usage.total} }` - ) - } - t.ok( - usage.total * MICRO_TO_MILLIS > SPIN_TIME - CPU_EPSILON, - 'should have total greater than spin time' - ) - t.ok( - usage.total * MICRO_TO_MILLIS <= testDuration, - 'should have total less than wall-clock time' - ) - t.ok(usage.min <= meanTime, 'should have min less than the mean usage time') - t.ok(usage.max >= meanTime, 'should have max greater than the mean usage time') - t.ok(usage.max * MICRO_TO_MILLIS > SPIN_TIME - CPU_EPSILON, 'should have expected max') - t.ok( - usage.sumOfSquares * MICRO_TO_MILLIS * MICRO_TO_MILLIS < durationSquare, - 'should have expected sumOfSquares' - ) - t.ok(usage.count >= 2, 'should have expected count') - - // Done! - t.end() - } -}) diff --git a/tests/unit/loop-metrics.test.js b/tests/unit/loop-metrics.test.js new file mode 100644 index 0000000..a4a2c32 --- /dev/null +++ b/tests/unit/loop-metrics.test.js @@ -0,0 +1,82 @@ +/* + * Copyright 2020 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict' + +const test = require('node:test') +const assert = require('node:assert') + +test('Loop Metrics', async (t) => { + const MICRO_TO_MILLIS = 1e-3 + const SPIN_TIME = 2000 + const CPU_EPSILON = SPIN_TIME * 0.05 // Allowed fudge factor for CPU times in MS + const metricEmitter = require('../..')({ timeout: 10 }) + const testStart = Date.now() + + t.after(() => { + metricEmitter.unbind() + }) + + // Check the structure of the metric object. + let metric = metricEmitter.getLoopMetrics().usage + assert.strictEqual(typeof metric, 'object', 'should provide a metric object') + assert.strictEqual(typeof metric.total, 'number', 'should have a total') + assert.strictEqual(typeof metric.min, 'number', 'should have a min') + assert.strictEqual(typeof metric.max, 'number', 'should have a max') + assert.strictEqual(typeof metric.sumOfSquares, 'number', 'should have a sumOfSquares') + assert.strictEqual(typeof metric.count, 'number', 'should have a count') + + // Check that the values are reset after the first call. + metric = metricEmitter.getLoopMetrics().usage + assert.deepStrictEqual( + metric, + { + total: 0, + min: 0, + max: 0, + sumOfSquares: 0, + count: 0 + }, + 'should reset all the values' + ) + + // Queue up a loop with some CPU burn. + await new Promise((resolve) => setTimeout(resolve, 100)) + + console.log('spinning cpu...') + const start = Date.now() + while (Date.now() - start < SPIN_TIME) {} // Spin the CPU for 2 seconds. + + // Wait another tick and then check the loop stats. + await new Promise((resolve) => setTimeout(resolve, 100)) + + metric = metricEmitter.getLoopMetrics() + const testDuration = Date.now() - testStart + CPU_EPSILON + const durationSquare = testDuration * testDuration + const usage = metric.usage + + const meanTime = usage.total / usage.count + if (process.arch === 'arm64') { + console.log( + `{ min: ${usage.min}, max: ${usage.max}, meanTime: ${meanTime}, count: ${usage.count}, total: ${usage.total} }` + ) + } + assert.ok( + usage.total * MICRO_TO_MILLIS > SPIN_TIME - CPU_EPSILON, + 'should have total greater than spin time' + ) + assert.ok( + usage.total * MICRO_TO_MILLIS <= testDuration, + 'should have total less than wall-clock time' + ) + assert.ok(usage.min <= meanTime, 'should have min less than the mean usage time') + assert.ok(usage.max >= meanTime, 'should have max greater than the mean usage time') + assert.ok(usage.max * MICRO_TO_MILLIS > SPIN_TIME - CPU_EPSILON, 'should have expected max') + assert.ok( + usage.sumOfSquares * MICRO_TO_MILLIS * MICRO_TO_MILLIS < durationSquare, + 'should have expected sumOfSquares' + ) + assert.ok(usage.count >= 2, 'should have expected count') +}) From 8a9dfadd0b9adf575ce9345e87827b1140bd22e8 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Mon, 9 Dec 2024 12:11:52 -0600 Subject: [PATCH 2/6] fix comment --- tests/unit/loop-metrics.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/loop-metrics.test.js b/tests/unit/loop-metrics.test.js index a4a2c32..094896c 100644 --- a/tests/unit/loop-metrics.test.js +++ b/tests/unit/loop-metrics.test.js @@ -28,7 +28,8 @@ test('Loop Metrics', async (t) => { assert.strictEqual(typeof metric.sumOfSquares, 'number', 'should have a sumOfSquares') assert.strictEqual(typeof metric.count, 'number', 'should have a count') - // Check that the values are reset after the first call. + // Check that the values are reset after the first call. Since this is + // synchronous with the previous call, all results should be zero. metric = metricEmitter.getLoopMetrics().usage assert.deepStrictEqual( metric, From 374885bf0b96f4f174011de59649164c4730b285 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Mon, 9 Dec 2024 14:30:06 -0600 Subject: [PATCH 3/6] fix unit script --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 5b1bd60..6541afa 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "lint": "eslint .", "lint:fix": "eslint . --fix", "lint:lockfile": "lockfile-lint --path package-lock.json --type npm --allowed-hosts npm --validate-https --validate-integrity", - "unit": "c8 -o ./coverage/unit node --test --expose-gc tests/unit/*.test.js", + "unit": "c8 -o ./coverage/unit node --expose-gc --test tests/unit/*.test.js", "integration": "c8 -o ./coverage/integration tap --timeout 360000 --jobs=1 --no-coverage tests/integration/*.tap.js", "native": "node tests/native/*.js", "test": "npm run unit && npm run integration", From d8387d657df315df85ca8f89f52af9bc93980d40 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Mon, 9 Dec 2024 15:03:57 -0600 Subject: [PATCH 4/6] fix test and unit script --- package.json | 2 +- tests/unit/gc-metrics.test.js | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/package.json b/package.json index 6541afa..e33e48e 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "lint": "eslint .", "lint:fix": "eslint . --fix", "lint:lockfile": "lockfile-lint --path package-lock.json --type npm --allowed-hosts npm --validate-https --validate-integrity", - "unit": "c8 -o ./coverage/unit node --expose-gc --test tests/unit/*.test.js", + "unit": "c8 -o ./coverage/unit borp --timeout 180000 'tests/unit/*.test.js'", "integration": "c8 -o ./coverage/integration tap --timeout 360000 --jobs=1 --no-coverage tests/integration/*.tap.js", "native": "node tests/native/*.js", "test": "npm run unit && npm run integration", diff --git a/tests/unit/gc-metrics.test.js b/tests/unit/gc-metrics.test.js index 2af93ae..10937c4 100644 --- a/tests/unit/gc-metrics.test.js +++ b/tests/unit/gc-metrics.test.js @@ -16,23 +16,23 @@ test('GC Metrics', async (t) => { const gcs = metricEmitter.getGCMetrics() const keys = Object.keys(gcs) assert.ok(keys.length > 0, 'should notice at least one GC') - assert.strictEqual(typeof keys[0], 'string', 'should have strings as keys') + assert.equal(typeof keys[0], 'string', 'should have strings as keys') await t.test('GC stats objects', () => { const stats = gcs[keys[0]] - assert.strictEqual(typeof stats, 'object', 'should have stats objects') - assert.strictEqual(typeof stats.typeId, 'number', 'should have the type ID') - assert.strictEqual(typeof stats.type, 'string', 'should have the type name') - assert.strictEqual(typeof stats.metrics, 'object', 'should have a metrics object') + assert.equal(typeof stats, 'object', 'should have stats objects') + assert.equal(typeof stats.typeId, 'number', 'should have the type ID') + assert.equal(typeof stats.type, 'string', 'should have the type name') + assert.equal(typeof stats.metrics, 'object', 'should have a metrics object') }) await t.test('GC stats metrics', () => { const metrics = gcs[keys[0]].metrics - assert.strictEqual(typeof metrics.total, 'number', 'should have total field') - assert.strictEqual(typeof metrics.min, 'number', 'should have min field') - assert.strictEqual(typeof metrics.max, 'number', 'should have max field') - assert.strictEqual(typeof metrics.sumOfSquares, 'number', 'should have sumOfSquares field') - assert.strictEqual(typeof metrics.count, 'number', 'should have count field') + assert.equal(typeof metrics.total, 'number', 'should have total field') + assert.equal(typeof metrics.min, 'number', 'should have min field') + assert.equal(typeof metrics.max, 'number', 'should have max field') + assert.equal(typeof metrics.sumOfSquares, 'number', 'should have sumOfSquares field') + assert.equal(typeof metrics.count, 'number', 'should have count field') assert.ok(metrics.total > 0, 'should have reasonable values for total') assert.ok(metrics.min > 0, 'should have reasonable values for min') From 9c5147fc6efad4fb8074b01f59df8b84888ccfd3 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Mon, 9 Dec 2024 15:21:45 -0600 Subject: [PATCH 5/6] add expose gc to unit script --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index e33e48e..f309af2 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "lint": "eslint .", "lint:fix": "eslint . --fix", "lint:lockfile": "lockfile-lint --path package-lock.json --type npm --allowed-hosts npm --validate-https --validate-integrity", - "unit": "c8 -o ./coverage/unit borp --timeout 180000 'tests/unit/*.test.js'", + "unit": "c8 -o ./coverage/unit borp --expose-gc --timeout 180000 'tests/unit/*.test.js'", "integration": "c8 -o ./coverage/integration tap --timeout 360000 --jobs=1 --no-coverage tests/integration/*.tap.js", "native": "node tests/native/*.js", "test": "npm run unit && npm run integration", @@ -80,6 +80,7 @@ "@newrelic/proxy": "^2.0.0", "async": "^3.2.2", "aws-sdk": "^2.266.1", + "borp": "^0.19.0", "c8": "^8.0.0", "eslint": "^7.32.0", "eslint-config-prettier": "^8.3.0", From 3ee289cf15f554cbf9e399ec01b3211ebbf37ea0 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Tue, 10 Dec 2024 13:42:26 -0600 Subject: [PATCH 6/6] refactored comments in unit tests --- tests/unit/gc-metrics.test.js | 46 ++++++++++++++++----------------- tests/unit/loop-metrics.test.js | 8 ++---- 2 files changed, 24 insertions(+), 30 deletions(-) diff --git a/tests/unit/gc-metrics.test.js b/tests/unit/gc-metrics.test.js index 10937c4..0da8001 100644 --- a/tests/unit/gc-metrics.test.js +++ b/tests/unit/gc-metrics.test.js @@ -8,7 +8,7 @@ const test = require('node:test') const assert = require('node:assert') -test('GC Metrics', async (t) => { +test('GC Metrics', async () => { const metricEmitter = require('../..')() global.gc() @@ -18,27 +18,25 @@ test('GC Metrics', async (t) => { assert.ok(keys.length > 0, 'should notice at least one GC') assert.equal(typeof keys[0], 'string', 'should have strings as keys') - await t.test('GC stats objects', () => { - const stats = gcs[keys[0]] - assert.equal(typeof stats, 'object', 'should have stats objects') - assert.equal(typeof stats.typeId, 'number', 'should have the type ID') - assert.equal(typeof stats.type, 'string', 'should have the type name') - assert.equal(typeof stats.metrics, 'object', 'should have a metrics object') - }) - - await t.test('GC stats metrics', () => { - const metrics = gcs[keys[0]].metrics - assert.equal(typeof metrics.total, 'number', 'should have total field') - assert.equal(typeof metrics.min, 'number', 'should have min field') - assert.equal(typeof metrics.max, 'number', 'should have max field') - assert.equal(typeof metrics.sumOfSquares, 'number', 'should have sumOfSquares field') - assert.equal(typeof metrics.count, 'number', 'should have count field') - - assert.ok(metrics.total > 0, 'should have reasonable values for total') - assert.ok(metrics.min > 0, 'should have reasonable values for min') - assert.ok(metrics.max > 0, 'should have reasonable values for max') - assert.ok(metrics.max >= metrics.min, 'should have a max larger than a min') - assert.ok(metrics.sumOfSquares > 0, 'should have reasonable values for sumOfSquares') - assert.ok(metrics.count > 0, 'should have reasonable values for count') - }) + // GC stats objects + const stats = gcs[keys[0]] + assert.equal(typeof stats, 'object', 'should have stats objects') + assert.equal(typeof stats.typeId, 'number', 'should have the type ID') + assert.equal(typeof stats.type, 'string', 'should have the type name') + assert.equal(typeof stats.metrics, 'object', 'should have a metrics object') + + // GC stats metrics + const metrics = gcs[keys[0]].metrics + assert.equal(typeof metrics.total, 'number', 'should have total field') + assert.equal(typeof metrics.min, 'number', 'should have min field') + assert.equal(typeof metrics.max, 'number', 'should have max field') + assert.equal(typeof metrics.sumOfSquares, 'number', 'should have sumOfSquares field') + assert.equal(typeof metrics.count, 'number', 'should have count field') + + assert.ok(metrics.total > 0, 'should have reasonable values for total') + assert.ok(metrics.min > 0, 'should have reasonable values for min') + assert.ok(metrics.max > 0, 'should have reasonable values for max') + assert.ok(metrics.max >= metrics.min, 'should have a max larger than a min') + assert.ok(metrics.sumOfSquares > 0, 'should have reasonable values for sumOfSquares') + assert.ok(metrics.count > 0, 'should have reasonable values for count') }) diff --git a/tests/unit/loop-metrics.test.js b/tests/unit/loop-metrics.test.js index 094896c..60af5fe 100644 --- a/tests/unit/loop-metrics.test.js +++ b/tests/unit/loop-metrics.test.js @@ -46,7 +46,7 @@ test('Loop Metrics', async (t) => { // Queue up a loop with some CPU burn. await new Promise((resolve) => setTimeout(resolve, 100)) - console.log('spinning cpu...') + // spinning cpu... const start = Date.now() while (Date.now() - start < SPIN_TIME) {} // Spin the CPU for 2 seconds. @@ -59,11 +59,7 @@ test('Loop Metrics', async (t) => { const usage = metric.usage const meanTime = usage.total / usage.count - if (process.arch === 'arm64') { - console.log( - `{ min: ${usage.min}, max: ${usage.max}, meanTime: ${meanTime}, count: ${usage.count}, total: ${usage.total} }` - ) - } + assert.ok( usage.total * MICRO_TO_MILLIS > SPIN_TIME - CPU_EPSILON, 'should have total greater than spin time'