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: integration tests setup #116

Merged
merged 2 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 3 additions & 0 deletions .github/actions/test/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,6 @@ runs:
- run: npm run test:miniflare
name: Miniflare Tests
shell: bash
- run: npm run test:integration
name: Integration Tests
shell: bash
10 changes: 10 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@
"build": "esbuild --bundle src/index.js --format=esm --sourcemap --minify --outfile=dist/worker.mjs && npm run build:tsc",
"build:debug": "esbuild --bundle src/index.js --format=esm --outfile=dist/worker.mjs",
"build:tsc": "tsc --build",
"test:miniflare": "npm run build:debug && mocha --experimental-vm-modules --recursive test/**/*.spec.js",
"test:miniflare": "npm run build:debug && mocha --experimental-vm-modules test/index.spec.js",
fforbeck marked this conversation as resolved.
Show resolved Hide resolved
"test:unit": "npm run build:debug && mocha --experimental-vm-modules --recursive test/unit/**/*.spec.js",
"test:integration": "npm run build:debug && mocha --experimental-vm-modules --recursive test/integration/**/*.spec.js --require test/fixtures/worker-fixture.js",
"lint": "standard",
"lint:fix": "standard --fix"
},
Expand Down Expand Up @@ -56,6 +57,7 @@
"multipart-byte-range": "^3.0.1",
"sinon": "^19.0.2",
"standard": "^17.1.0",
"tree-kill": "^1.2.2",
"typescript": "^5.3.3",
"wrangler": "^3.78.8"
},
Expand Down
2 changes: 1 addition & 1 deletion src/bindings.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export interface Environment {
ACCOUNTING_SERVICE_URL: string
RATE_LIMITER: RateLimit
AUTH_TOKEN_METADATA: KVNamespace
FF_RATE_LIMITER_ENABLED: boolean
FF_RATE_LIMITER_ENABLED: string
}

export type RateLimitExceeded = typeof RATE_LIMIT_EXCEEDED[keyof typeof RATE_LIMIT_EXCEEDED]
Expand Down
7 changes: 3 additions & 4 deletions src/handlers/rate-limiter.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,13 @@ import { Accounting } from '../services/accounting.js'
*/
export function withRateLimit (handler) {
return async (req, env, ctx) => {
if (env.FF_RATE_LIMITER_ENABLED !== true) {
if (env.FF_RATE_LIMITER_ENABLED !== 'true') {
return handler(req, env, ctx)
}

const { dataCid } = ctx
const rateLimitService = createRateLimitService(env, ctx)
const rateLimitService = create(env, ctx)
const isRateLimitExceeded = await rateLimitService.check(dataCid, req)

if (isRateLimitExceeded === RATE_LIMIT_EXCEEDED.YES) {
throw new HttpError('Too Many Requests', { status: 429 })
} else {
Expand All @@ -43,7 +42,7 @@ export function withRateLimit (handler) {
* @param {IpfsUrlContext} ctx
* @returns {RateLimitService}
*/
function createRateLimitService (env, ctx) {
function create (env, ctx) {
return {
/**
* @param {import('multiformats/cid').CID} cid
Expand Down
74 changes: 74 additions & 0 deletions test/fixtures/worker-fixture.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import path, { resolve } from 'node:path'
import { fileURLToPath } from 'node:url'
import { runWranglerDev } from '../helpers/run-wrangler.js'

// Get __dirname equivalent in ES module
const __filename = fileURLToPath(import.meta.url)
const __dirname = path.dirname(__filename)

/**
* The IP address of the test worker.
* @type {string}
*/
let ip = 'localhost'
Copy link
Member

Choose a reason for hiding this comment

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

suggestion Maybe let's put these all in one object (the return value of await runWranglerDev()), to make it clear that they all get set together. I had to read through the whole file a couple of times to understand what was setting these.

Along those lines, let's separate the requested port (which is a constant) from the actual port, and stop providing default values for ip and port. Right now, they're lying until Wrangler boots. If Wrangler's not up yet, we should signal that rather than provide fake values.

One way to set this up (which might be overkill) would be to have mochaGlobalSetup() store the promise from runWranglerDev() rather than await it itself, and have getWorkerInfo() be async. Then the tests will start booting Wrangler and not wait for it to be set up until the values are actually requested. (We don't really have a performance issue here, though, so the choice should be down to which one communicates what's happening the clearest.)


/**
* The port of the test worker.
* Default is 8585.
* @type {number}
*/
let port = 8585

/**
* Stops the test worker.
* @type {() => Promise<unknown> | undefined}
*/
let stop

/**
* Gets the output of the test worker.
* @type {() => string}
*/
let getOutput

/**
* The wrangler environment to use for the test worker.
* Default is "integration".
* @type {string}
*/
const wranglerEnv = process.env.WRANGLER_ENV || 'integration'

/**
* Sets up the test worker.
* @returns {Promise<void>}
*/
export const mochaGlobalSetup = async () => {
({ ip, port, stop, getOutput } = await runWranglerDev(
resolve(__dirname, '../../'), // The directory of the worker with the wrangler.toml
['--local', `--port=${port}`],
process.env,
wranglerEnv
))

console.log(`Output: ${getOutput()}`)
console.log(`Using wrangler environment: ${wranglerEnv}`)
console.log('Test worker started!')
}

/**
* Tears down the test worker.
* @returns {Promise<void>}
*/
export const mochaGlobalTeardown = async () => {
await stop?.()
// console.log('getOutput', getOutput()) // uncomment for debugging
console.log('Test worker stopped!')
}

/**
* Gets the worker info.
* @returns {{ ip: string, port: number, wranglerEnv: string, stop: (() => Promise<void>) | undefined, getOutput: () => string }}
*/
export function getWorkerInfo () {
return { ip, port, wranglerEnv, stop, getOutput }
}
122 changes: 122 additions & 0 deletions test/helpers/run-wrangler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
import assert from 'node:assert'
import { fork } from 'node:child_process'
import path from 'node:path'
import treeKill from 'tree-kill'
import { fileURLToPath } from 'node:url'

// Get __dirname equivalent in ES module
const __filename = fileURLToPath(import.meta.url)
const __dirname = path.dirname(__filename)

export const wranglerEntryPath = path.resolve(
__dirname,
'../../node_modules/wrangler/bin/wrangler.js'
)

/**
* Runs the command `wrangler dev` in a child process.
*
* Returns an object that gives you access to:
*
* - `ip` and `port` of the http-server hosting the pages project
* - `stop()` function that will close down the server.
*
* @param {string} cwd - The current working directory.
* @param {string[]} options - The options to pass to the wrangler command.
* @param {NodeJS.ProcessEnv} [env] - The environment variables.
* @param {string} [wranglerEnv] - The wrangler environment to use.
* @returns {Promise<{ip: string, port: number, stop: () => Promise<void>, getOutput: () => string, clearOutput: () => void}>}
*/
export async function runWranglerDev (
cwd,
options,
env,
wranglerEnv
) {
return runLongLivedWrangler(
['dev', `--env=${wranglerEnv}`, '--ip=127.0.0.1', ...options],
fforbeck marked this conversation as resolved.
Show resolved Hide resolved
cwd,
env
)
}

/**
* Runs a long-lived wrangler command in a child process.
*
* @param {string[]} command - The wrangler command to run.
* @param {string} cwd - The current working directory.
* @param {NodeJS.ProcessEnv} [env] - The environment variables.
* @returns {Promise<{ip: string, port: number, stop: () => Promise<void>, getOutput: () => string, clearOutput: () => void}>}
*/
async function runLongLivedWrangler (
command,
cwd,
env
) {
let settledReadyPromise = false
/** @type {(value: { ip: string port: number }) => void} */
let resolveReadyPromise
/** @type {(reason: unknown) => void} */
let rejectReadyPromise
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: Let's avoid all these lets and put the stuff below inside the new Promise(). It'll have access to resolve and reject, and we don't need to track settledReadyPromise at all: resolveing or rejecting after it's settled is a noop.


const ready = new Promise((resolve, reject) => {
resolveReadyPromise = resolve
rejectReadyPromise = reject
})

const wranglerProcess = fork(wranglerEntryPath, command, {
stdio: ['ignore', /* stdout */ 'pipe', /* stderr */ 'pipe', 'ipc'],
cwd,
env: { ...process.env, ...env, PWD: cwd }
}).on('message', (message) => {
if (settledReadyPromise) return
settledReadyPromise = true
clearTimeout(timeoutHandle)
resolveReadyPromise(JSON.parse(message.toString()))
})

const chunks = []
wranglerProcess.stdout?.on('data', (chunk) => {
chunks.push(chunk)
})
wranglerProcess.stderr?.on('data', (chunk) => {
chunks.push(chunk)
})
const getOutput = () => Buffer.concat(chunks).toString()
const clearOutput = () => (chunks.length = 0)

const timeoutHandle = setTimeout(() => {
if (settledReadyPromise) return
settledReadyPromise = true
const separator = '='.repeat(80)
const message = [
'Timed out starting long-lived Wrangler:',
separator,
getOutput(),
separator
].join('\n')
rejectReadyPromise(new Error(message))
}, 50_000)

async function stop () {
return new Promise((resolve) => {
assert(
wranglerProcess.pid,
`Command "${command.join(' ')}" had no process id`
)
treeKill(wranglerProcess.pid, (e) => {
if (e) {
console.error(
'Failed to kill command: ' + command.join(' '),
wranglerProcess.pid,
e
)
fforbeck marked this conversation as resolved.
Show resolved Hide resolved
}
resolve()
})
})
}

const { ip, port } = await ready
return { ip, port, stop, getOutput, clearOutput }
}
32 changes: 32 additions & 0 deletions test/integration/rate-limit.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { expect } from 'chai'
import { describe, it } from 'mocha'
import fetch from 'node-fetch'
import { getWorkerInfo } from '../fixtures/worker-fixture.js'

describe('Rate Limit Handler', () => {
const { ip, port } = getWorkerInfo()

// This is a test CID that is known to be stored in the staging environment
// See https://bafybeibv7vzycdcnydl5n5lbws6lul2omkm6a6b5wmqt77sicrwnhesy7y.ipfs.w3s.link
const cid = 'bafybeibv7vzycdcnydl5n5lbws6lul2omkm6a6b5wmqt77sicrwnhesy7y'

it('should enforce rate limits', async () => {
const maxRequests = 130
let successCount = 0
let rateLimitCount = 0

const requests = Array.from({ length: maxRequests }, async () => {
const response = await fetch(`http://${ip}:${port}/ipfs/${cid}`)
if (response.status === 200) {
successCount++
} else if (response.status === 429) {
rateLimitCount++
}
})

await Promise.all(requests)

expect(successCount).to.be.lessThan(maxRequests)
expect(rateLimitCount).to.be.greaterThan(0)
fforbeck marked this conversation as resolved.
Show resolved Hide resolved
}).timeout(30_000)
})
2 changes: 1 addition & 1 deletion test/unit/middlewares/rate-limiter.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('withRateLimits', () => {
}
env = {
RATE_LIMITER: rateLimiter,
FF_RATE_LIMITER_ENABLED: true,
FF_RATE_LIMITER_ENABLED: 'true',
fforbeck marked this conversation as resolved.
Show resolved Hide resolved
ACCOUNTING_SERVICE_URL: 'http://example.com',
AUTH_TOKEN_METADATA: {
get: sandbox.stub(),
Expand Down
27 changes: 26 additions & 1 deletion wrangler.toml
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,29 @@ simple = { limit = 5, period = 60 }

[[env.fforbeck.kv_namespaces]]
binding = "AUTH_TOKEN_METADATA"
id = "07c22b9612b244e797b780e24f74e2cf"
id = "f848730e45d94f17bcaf3b6d0915da40"


### Integration Tests Configuration
[env.integration]
name = "freeway-integration-test"
workers_dev = true
account_id = "fffa4b4363a7e5250af8357087263b3a"
r2_buckets = [
{ binding = "CARPARK", bucket_name = "carpark-integration-0", preview_bucket_name = "carpark-integration-preview-0" }
]

[env.integration.vars]
DEBUG = "true"
FF_RATE_LIMITER_ENABLED = "true"
CONTENT_CLAIMS_SERVICE_URL = "https://staging.claims.web3.storage"

[[env.integration.unsafe.bindings]]
name = "RATE_LIMITER"
type = "ratelimit"
namespace_id = "0"
simple = { limit = 100, period = 60 }

[[env.integration.kv_namespaces]]
binding = "AUTH_TOKEN_METADATA"
id = "a355501ee4f242b1affa32c1b335db2b"
Loading