Skip to content
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

Newrelic 3123/esm test loader #141

Merged
merged 8 commits into from
Sep 14, 2022
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,8 @@ node_modules
# VS Code settings
.vscode

# JetBrains settings
.idea

# Instanbul code coverage files
.nyc_output
9 changes: 8 additions & 1 deletion lib/versioned/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
'use strict'
/* eslint-disable no-console */

const path = require('path')
const a = require('async')
const cp = require('child_process')

Expand Down Expand Up @@ -97,7 +98,13 @@ function installPackages(cb) {
function runTests(cb) {
// TODO: Add tap arguments, such as color.
process.send({ status: 'running' })
spawn('node', [testFile], function testHandler(err) {
let args = [testFile]
if (process.env.PKG_TYPE === 'module') {
const loaderPath = path.resolve(process.env.NR_LOADER)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the wrong env var now. should we unset this env var after every run so subsequent suites will not inherit?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd agree that we should unset if we want to be sure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd think this should be failing--there's a disjuncture between the env vars on 102 and 103.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is out of date. it's been updated

args = ['--loader', loaderPath, testFile]
}

spawn('node', args, function testHandler(err) {
if (err) {
const error = new Error('Failed to execute test: ' + err.stack)
error.code = Math.abs(err.code)
Expand Down
23 changes: 21 additions & 2 deletions lib/versioned/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ function Test(directory, pkgVersions, opts = {}) {
this.duration = 0
this.allPkgs = !!allPkgs
this.strict = !!opts.strict
this.type = pkg.type || 'commonjs'
}

Test.prototype.next = function next() {
Expand All @@ -102,7 +103,6 @@ Test.prototype.peek = function peek() {

Test.prototype.run = function run() {
const task = this.next()

if (!task) {
return null
}
Expand All @@ -126,10 +126,29 @@ Test.prototype.run = function run() {
// the one packages you were installing. The runner has a new -a prop that is a boolean
const pkgList = this.allPkgs ? task.packageVersions : pkgs

const additionalArgs = {
PKG_TYPE: this.type
}

if (this.type === 'module') {
// The test loader is defined in the agent, so this uses process.cwd() instead of __directory.
const cwd = process.cwd()
let loaderPath = path.resolve(`${cwd}/test/lib/`, 'test-loader.mjs')
if (process.env.NR_LOADER) {
// Allow for override from environment
// This assumes that the loader path is defined in relation to the agent root.
loaderPath = path.resolve(cwd, process.env.NR_LOADER)
}
// changing env var for loader so subsequent commonjs
// runs that have NR_LOADER do not try to load it
additionalArgs.NR_LOADER = loaderPath
}

// Spawn another runner instance with list of packages to install
const child = cp.spawn('node', [TEST_EXECUTOR, task.test].concat(pkgList), {
cwd: this.directory,
stdio: ['ignore', 'pipe', 'pipe', 'ipc']
stdio: ['ignore', 'pipe', 'pipe', 'ipc'],
env: { ...process.env, ...additionalArgs }
})

const testRun = new TestRun(child, pkgs.length > 0)
Expand Down
10 changes: 10 additions & 0 deletions tests/unit/versioned/mock-esm-tests/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "mock-tests",
"version": "1.0.0",
"type": "module",
"private": true,
"tests": [{
"dependencies": {"redis": ">=1.0.0"},
"files": ["redis.mock.tap.js"]
}]
}
11 changes: 11 additions & 0 deletions tests/unit/versioned/mock-esm-tests/redis.mock.tap.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright 2020 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

'use strict'

console.log('stdout - redis.mock.tap.js')
console.error('stderr - redis.mock.tap.js')
/* eslint-disable no-process-exit */
process.exit(0)
66 changes: 66 additions & 0 deletions tests/unit/versioned/test.tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
const path = require('path')
const tap = require('tap')
const fs = require('fs')
const sinon = require('sinon')

const Test = require('../../../lib/versioned/test')

Expand All @@ -17,16 +18,81 @@ const pkgVersions = {
redis: { versions: ['1.0.0', '2.0.1', '2.1.0'], latest: '2.1.0' }
}

const ESM_MOCK_DIR = path.resolve(__dirname, 'mock-esm-tests')

tap.test('Test construction', function (t) {
let test = null
t.doesNotThrow(function () {
test = new Test(MOCK_TEST_DIR, pkgVersions)
}, 'should not throw when constructed')

t.type(test, Test, 'should construct a Test instance')
t.equal(test.type, 'commonjs', 'should default type to commonjs')
t.end()
})

tap.test('ESM Tests', (t) => {
t.autoend()
const cp = require('child_process')
const esmVersions = {
redis: { versions: ['1.0.0'] }
}

t.before(() => {
sinon.spy(cp, 'spawn')
})

t.afterEach(() => {
cp.spawn.resetHistory()
})

t.teardown(() => {
cp.spawn.restore()
})

t.test('should properly construct test with type of module', (t) => {
let test = null
t.doesNotThrow(function () {
test = new Test(ESM_MOCK_DIR, esmVersions)
}, 'should not throw when constructed')

t.type(test, Test, 'should construct a Test instance')
t.equal(test.type, 'module', 'should default type to module')
t.end()
})

t.test('should default loader to test-loader.mjs when NR_LOADER is not specified', (t) => {
const test = new Test(ESM_MOCK_DIR, esmVersions)
const testRun = test.run()
const { env } = cp.spawn.args[0][2]
t.equal(env.NR_LOADER, `${process.cwd()}/test/lib/test-loader.mjs`, 'should use default loader')
// must force the mocked test run to complete so tap can shut down
testRun.continue()
testRun.once('completed', function () {
testRun.continue()
t.end()
})
})

t.test('should use NR_LOADER when specified', (t) => {
const test = new Test(ESM_MOCK_DIR, esmVersions)
process.env.NR_LOADER = 'bogus-loader.mjs'
const testRun = test.run()
const { env } = cp.spawn.args[0][2]
t.equal(
env.NR_LOADER,
`${process.cwd()}/bogus-loader.mjs`,
'should use NR_LOADER but resolves path'
)
// must force the mocked test run to complete so tap can shut down
testRun.continue()
testRun.once('completed', function () {
testRun.continue()
t.end()
})
})
})

tap.test('Test methods and members', function (t) {
t.autoend()

Expand Down