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

chore: Converted agent unit tests to node:test #2414

Merged
merged 2 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 18 additions & 1 deletion THIRD_PARTY_NOTICES.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ code, the source code can be found at [https://github.com/newrelic/node-newrelic
* [proxyquire](#proxyquire)
* [rfdc](#rfdc)
* [rimraf](#rimraf)
* [self-cert](#self-cert)
* [should](#should)
* [sinon](#sinon)
* [superagent](#superagent)
Expand Down Expand Up @@ -4023,6 +4024,22 @@ IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

```

### self-cert

This product includes source derived from [self-cert](https://github.com/jsumners/self-cert) ([v2.0.0](https://github.com/jsumners/self-cert/tree/v2.0.0)), distributed under the [MIT License](https://github.com/jsumners/self-cert/blob/v2.0.0/Readme.md):

```
MIT License

Copyright (c) <year> <copyright holders>

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
```

### should

This product includes source derived from [should](https://github.com/shouldjs/should.js) ([v13.2.3](https://github.com/shouldjs/should.js/tree/v13.2.3)), distributed under the [MIT License](https://github.com/shouldjs/should.js/blob/v13.2.3/LICENSE):
Expand Down Expand Up @@ -4053,7 +4070,7 @@ THE SOFTWARE.

### sinon

This product includes source derived from [sinon](https://github.com/sinonjs/sinon) ([v4.5.0](https://github.com/sinonjs/sinon/tree/v4.5.0)), distributed under the [BSD-3-Clause License](https://github.com/sinonjs/sinon/blob/v4.5.0/LICENSE):
This product includes source derived from [sinon](https://github.com/sinonjs/sinon) ([v5.1.1](https://github.com/sinonjs/sinon/tree/v5.1.1)), distributed under the [BSD-3-Clause License](https://github.com/sinonjs/sinon/blob/v5.1.1/LICENSE):

```
(The BSD License)
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,9 @@
"proxyquire": "^1.8.0",
"rfdc": "^1.3.1",
"rimraf": "^2.6.3",
"self-cert": "^2.0.0",
"should": "*",
"sinon": "^4.5.0",
"sinon": "^5.1.1",
"superagent": "^9.0.1",
"tap": "^16.3.4",
"temp": "^0.8.1"
Expand Down
17 changes: 17 additions & 0 deletions test/lib/fake-cert.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Copyright 2024 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

'use strict'

const selfCert = require('self-cert')
module.exports = selfCert({
attrs: {
stateName: 'Georgia',
locality: 'Atlanta',
orgName: 'New Relic',
shortName: 'new_relic'
},
expires: new Date('2099-12-31')
})
28 changes: 28 additions & 0 deletions test/lib/promise-resolvers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright 2024 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

'use strict'

/**
* Implements https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/withResolvers
*
* This can be removed once Node.js v22 is the minimum.
*
* @returns {{resolve, reject, promise: Promise<unknown>}}
*/
module.exports = function promiseResolvers() {
if (typeof Promise.withResolvers === 'function') {
// Node.js >=22 natively supports this.
return Promise.withResolvers()
}

let resolve
let reject
const promise = new Promise((a, b) => {
resolve = a
reject = b
})
return { promise, resolve, reject }
}
194 changes: 194 additions & 0 deletions test/lib/test-collector.js
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was devised to replace the usage of nock in test/unit/agent/agent.test.js. I encountered an issue where one of the subtests was unable to be resolved due to a timing issue between promise resolution and the event loop finishing. Utilizing "real" network requests helped me resolve the problem. If I remember correctly, it was this test causing grief:

t.test('should not blow up when harvest cycle runs', (t) => {
agent.start(() => {
setTimeout(() => {
t.ok(redirect.isDone())
t.ok(connect.isDone())
t.ok(settings.isDone())
t.end()
}, 15)
})
})

Maybe the test could have been reworked with nock still being used, but I am less convinced of nock's utility nowadays than I used to be. While the pattern is not used by any tests in this PR, by using the approach in this PR we can do things like:

test('something', async t => {
	const { collector } = t.ctx
	const { promise, resolve = Promise.withResolvers()

	collector.addHandler('/some/endpoint', (req, res) => {
		// Verify that the client generated the correct request structure:
		assert.equal(req.url, '/some/endpoint?with=query_params')
		res.writeHead(200)
		res.end()
		resolve()
	})

	await promise
})

Yes, it is possible with nock, but, despite using it for many years, I find myself having to go reference the documentation every time I want to do something like that.

Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
/*
* Copyright 2024 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

'use strict'

// This provides an in-process http server to use in place of
// collector.newrelic.com. It allows for custom handlers so that test specific
// assertions can be made.

const https = require('node:https')
const querystring = require('node:querystring')
const helper = require('./agent_helper')
const fakeCert = require('./fake-cert')

class Collector {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be generic enough to utilize in all tests that need to talk to the remote collector. I haven't actually looked, but I bet we can replace the fake-collector references in test/integration/collector-remote-method.tap.js with this.

#handlers = new Map()
#server
#address

constructor() {
this.#server = https.createServer({
key: fakeCert.privateKey,
cert: fakeCert.certificate
})
this.#server.on('request', (req, res) => {
const qs = querystring.decode(req.url.slice(req.url.indexOf('?') + 1))
const handler = this.#handlers.get(qs.method)
if (typeof handler !== 'function') {
res.writeHead(500)
return res.end('handler not found: ' + req.url)
}

res.json = function ({ payload, code = 200 }) {
this.writeHead(code, { 'content-type': 'application/json' })
this.end(JSON.stringify(payload))
}

handler.isDone = true
handler(req, res)
})

// We don't need this server keeping the process alive.
this.#server.unref()
}

/**
* A configuration object that can be passed to an "agent" instance so that
* the agent will communicate with this test server instead of the real
* server.
*
* Important: the `.listen` method must be invoked first in order to have
* the `host` and `port` defined.
*
* @returns {object}
*/
get agentConfig() {
return {
host: this.host,
port: this.port,
license_key: 'testing',
certificates: [this.cert]
}
}

/**
* The host the server is listening on.
*
* @returns {string}
*/
get host() {
return this.#address?.address
}

/**
* The port number the server is listening on.
*
* @returns {number}
*/
get port() {
return this.#address?.port
}

/**
* A copy of the public certificate used to secure the server. Use this
* like `new Agent({ certificates: [collector.cert] })`.
*
* @returns {string}
*/
get cert() {
return fakeCert.certificate
}

/**
* The most basic `agent_settings` handler. Useful when you do not need to
* customize the handler.
*
* @returns {function}
*/
get agentSettingsHandler() {
return function (req, res) {
res.json({ payload: { return_value: [] } })
}
}

/**
* The most basic `preconnect` handler. Useful when you do not need to
* customize the handler.
*
* @returns {function}
*/
get preconnectHandler() {
const host = this.host
const port = this.port
return function (req, res) {
res.json({
payload: {
return_value: {
redirect_host: `${host}:${port}`,
security_policies: {}
}
}
})
}
}

/**
* Adds a new handler for the provided endpoint.
*
* @param {string} endpoint A string like
* `/agent_listener/invoke_raw_method?method=preconnect`. Notice that a query
* string with the `method` parameter is present. This is required, as the
* value of `method` will be used to look up the handler when receiving
* requests.
* @param {function} handler A typical `(req, res) => {}` handler. For
* convenience, `res` is extended with a `json({ payload, code = 200 })`
* method for easily sending JSON responses.
*/
addHandler(endpoint, handler) {
const qs = querystring.decode(endpoint.slice(endpoint.indexOf('?') + 1))
this.#handlers.set(qs.method, handler)
}

/**
* Shutdown the server and forcefully close all current connections.
*/
close() {
this.#server.closeAllConnections()
}

/**
* Determine if a handler has been invoked.
*
* @param {string} method Name of the method to check, e.g. "preconnect".
* @returns {boolean}
*/
isDone(method) {
return this.#handlers.get(method)?.isDone === true
}

/**
* Start the server listening for requests.
*
* @returns {Promise<object>} Returns a standard server address object.
*/
async listen() {
let address
await new Promise((resolve, reject) => {
this.#server.listen(0, '127.0.0.1', (err) => {
if (err) {
return reject(err)
}
address = this.#server.address()
resolve()
})
})

this.#address = address

// Add handlers for the required agent startup connections. These should
// be overwritten by tests that exercise the startup phase, but adding these
// stubs makes it easier to test other connection events.
this.addHandler(helper.generateCollectorPath('preconnect', 42), this.preconnectHandler)
this.addHandler(helper.generateCollectorPath('connect', 42), (req, res) => {
res.json({ payload: { return_value: { agent_run_id: 42 } } })
})
this.addHandler(helper.generateCollectorPath('agent_settings', 42), this.agentSettingsHandler)

return address
}
}

module.exports = Collector
Loading
Loading