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.

Sign up for GitHub

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
Merged

Newrelic 3123/esm test loader #141

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cada6a6
Added JetBrains config to .gitignore
mrickard Sep 9, 2022
9de8e84
Get dependency type from package.json and use alternate loader for mo…
mrickard Sep 9, 2022
982da86
Corrected loader argument to cp/.spawn and allowed for loader to be d…
mrickard Sep 12, 2022
69a7cc2
Updated to use test loader, and renamed loader env var to NR_LOADER
mrickard Sep 12, 2022
334a69a
Refactored to supply loader from test.js, before spawn
mrickard Sep 14, 2022
8fa5bea
added unit tests for the loader logic in lib/versioned/test.js
bizob2828 Sep 14, 2022
683f724
cleaned up logic in runner for determining when to use loader
bizob2828 Sep 14, 2022
d857dd7
restored a file i removed
bizob2828 Sep 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
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
7 changes: 6 additions & 1 deletion lib/versioned/runner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,12 @@ 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.packageType && process.env.packageType === 'module') {
Copy link
Member

Choose a reason for hiding this comment

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

thinking more of this we could just default the type in lib/versioned/test.js to commonjs

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 initially did do that--it's easy enough to restore!

args = ['--loader ../../../../nr-esm-loader/test-loader.mjs', testFile]
mrickard marked this conversation as resolved.
Show resolved Hide resolved
}

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
5 changes: 3 additions & 2 deletions lib/versioned/test.js
View file
Open in desktop
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
mrickard marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

we have unit test for this file, might be good to add some assertions around type.

}

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 @@ -129,7 +129,8 @@ Test.prototype.run = function run() {
// 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, packageType: this.type }
})

const testRun = new TestRun(child, pkgs.length > 0)
Expand Down