From e9ade01412eeafc693c4e6d752a0732eeb433709 Mon Sep 17 00:00:00 2001 From: Peter Hedenskog Date: Sat, 16 May 2026 20:14:30 +0200 Subject: [PATCH] Clean up base plugin class, add tests and CI The base class had a log() method that was unreachable because the constructor assigned this.log to the logger instance, shadowing the prototype method. Plugin authors who tried the documented wrapper got the raw logger instead. Remove the wrapper so what's documented matches what's actually used. While here, surface bad usage earlier: validate that the config object has the required fields up front with a clear error, instead of letting a confusing Cannot read properties of undefined escape later. Add a real README an engines field, and a minimal node:test suite plus a GitHub Actions workflow so regressions in the base class get caught before they reach plugin authors. Co-authored-by: Claude noreply@anthropic.com --- .eslintrc.json | 1 + .github/workflows/unittests.yml | 27 +++++++ README.md | 59 ++++++++++++++- package-lock.json | 6 ++ package.json | 6 +- plugin.js | 22 +++--- test/plugin.test.js | 130 ++++++++++++++++++++++++++++++++ 7 files changed, 236 insertions(+), 15 deletions(-) create mode 100644 .github/workflows/unittests.yml create mode 100644 test/plugin.test.js diff --git a/.eslintrc.json b/.eslintrc.json index 5e1e09a..8aa4652 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -20,6 +20,7 @@ "embeddedLanguageFormatting": "off" } ], + "eqeqeq": "error", "require-atomic-updates": 0, "no-extra-semi": 0, "no-mixed-spaces-and-tabs": 0, diff --git a/.github/workflows/unittests.yml b/.github/workflows/unittests.yml new file mode 100644 index 0000000..d086f1e --- /dev/null +++ b/.github/workflows/unittests.yml @@ -0,0 +1,27 @@ +name: Unit tests +on: + push: + branches: + - main + pull_request: + branches: + - main +jobs: + build: + runs-on: ubuntu-22.04 + strategy: + fail-fast: false + matrix: + node-version: [20.x, 22.x, 24.x] + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + with: + node-version: ${{ matrix.node-version }} + - name: Install plugin + run: npm ci + - name: Lint + run: npm run lint + - name: Run unit tests + run: npm test diff --git a/README.md b/README.md index f4c79e0..251e7c3 100644 --- a/README.md +++ b/README.md @@ -1 +1,58 @@ -# Base plugin \ No newline at end of file +# @sitespeed.io/plugin + +Base class for [sitespeed.io](https://www.sitespeed.io/) plugins. Extend it, +implement `processMessage`, and sitespeed.io will instantiate and drive your +plugin through the message queue. + +See the [plugin documentation](https://www.sitespeed.io/documentation/sitespeed.io/plugins/#how-to-create-your-own-plugin) +for the full plugin lifecycle. + +## Install + +```bash +npm install @sitespeed.io/plugin +``` + +## Usage + +```js +import { SitespeedioPlugin } from '@sitespeed.io/plugin'; + +export default class MyPlugin extends SitespeedioPlugin { + constructor(options, context, queue) { + super({ name: 'myplugin', options, context, queue }); + } + + async open() { + // optional: setup on startup + } + + async processMessage(message) { + if (message.type === 'url') { + this.log.info('Got a URL: %s', message.url); + await this.sendMessage('myplugin.data', { hello: 'world' }); + } + } + + async close() { + // optional: cleanup on shutdown + } +} +``` + +## API + +- `this.name` / `getName()` — plugin name +- `this.options` / `getOptions()` — sitespeed.io start options +- `this.context` / `getContext()` — sitespeed.io context +- `this.queue` — the message queue +- `this.log` / `getLog()` — logger (call levels directly: `this.log.info(...)`) +- `getStorageManager()` — storage manager for writing files +- `getFilterRegistry()` — filter registry for TimeSeries metrics +- `sendMessage(type, data, extras)` — post a message on the queue +- `open()` / `close()` — lifecycle hooks (override as needed) +- `processMessage(message)` — **must be implemented** by your subclass + +## License + +MIT diff --git a/package-lock.json b/package-lock.json index 87586f5..08c6bc0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,6 +14,9 @@ "eslint-plugin-prettier": "4.2.1", "eslint-plugin-unicorn": "45.0.2", "prettier": "2.8.4" + }, + "engines": { + "node": ">=20.0.0" } }, "node_modules/@babel/code-frame": { @@ -239,6 +242,7 @@ "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.8.2.tgz", "integrity": "sha512-xjIYgE8HBrkpd/sJqOGNspf8uHG+NOHGOw6a/Urj8taM2EXfdNAH2oFcPeIFfsv3+kz/mJrS5VuMqbNLjCa2vw==", "dev": true, + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -489,6 +493,7 @@ "resolved": "https://registry.npmjs.org/eslint/-/eslint-8.34.0.tgz", "integrity": "sha512-1Z8iFsucw+7kSqXNZVslXS8Ioa4u2KM7GPwuKtkTFAqZ/cHMcEaR+1+Br0wLlot49cNxIiZk5wp8EAbPcYZxTg==", "dev": true, + "peer": true, "dependencies": { "@eslint/eslintrc": "^1.4.1", "@humanwhocodes/config-array": "^0.11.8", @@ -1335,6 +1340,7 @@ "resolved": "https://registry.npmjs.org/prettier/-/prettier-2.8.4.tgz", "integrity": "sha512-vIS4Rlc2FNh0BySk3Wkd6xmwxB0FpOndW5fisM5H8hsZSxU2VWVB5CWIkIjWvrHjIhxk2g3bfMKM87zNTrZddw==", "dev": true, + "peer": true, "bin": { "prettier": "bin-prettier.js" }, diff --git a/package.json b/package.json index f1a709f..7f148a2 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,8 @@ ], "scripts": { "lint": "eslint .", - "lint:fix": "eslint . --fix" + "lint:fix": "eslint . --fix", + "test": "node --test" }, "keywords": [ "sitespeed.io", @@ -23,6 +24,9 @@ "url": "https://github.com/sitespeedio/plugin.git" }, "license": "MIT", + "engines": { + "node": ">=20.0.0" + }, "devDependencies": { "eslint": "8.34.0", "eslint-config-prettier": "8.6.0", diff --git a/plugin.js b/plugin.js index 62392a2..19691c6 100644 --- a/plugin.js +++ b/plugin.js @@ -6,9 +6,14 @@ */ export class SitespeedioPlugin { constructor(config) { - if (this.constructor == SitespeedioPlugin) { + if (this.constructor === SitespeedioPlugin) { throw new Error("Abstract plugin can't be instantiated."); } + if (!config || !config.name || !config.context || !config.queue) { + throw new Error( + 'SitespeedioPlugin requires a config object with name, context and queue' + ); + } if (config.name.includes('.')) { throw new Error("sitespeed.io plugin names can't contain dots"); } @@ -17,18 +22,9 @@ export class SitespeedioPlugin { this.context = config.context; this.queue = config.queue; this.make = config.context.messageMaker(this.name).make; - this.log = config.context.getLogger( - `sitespeed.io.plugin.${config.name}` - ); - } - - /** - * Log a message. Default log level is info. - * @param {*} message - * @param {*} level - trace|verbose|debug|info|warn|error|critical - */ - log(message, level = 'info', ...args) { - this.log[level](message, args); + // Logger instance. Call levels directly, e.g. this.log.info(msg). + // Levels: trace|verbose|debug|info|warn|error|critical + this.log = config.context.getLogger(`sitespeed.io.plugin.${config.name}`); } /** diff --git a/test/plugin.test.js b/test/plugin.test.js new file mode 100644 index 0000000..0b492c1 --- /dev/null +++ b/test/plugin.test.js @@ -0,0 +1,130 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; + +import { SitespeedioPlugin } from '../plugin.js'; + +function makeContext() { + return { + messageMaker(name) { + return { + make(type, data, extras) { + return { type, data, extras, source: name }; + } + }; + }, + getLogger(channel) { + return { channel, info() {}, warn() {}, error() {} }; + }, + filterRegistry: { id: 'registry' }, + storageManager: { id: 'storage' } + }; +} + +function makeQueue() { + const posted = []; + return { + posted, + postMessage(message) { + posted.push(message); + return message; + } + }; +} + +class TestPlugin extends SitespeedioPlugin { + async processMessage() {} +} + +class NoProcessPlugin extends SitespeedioPlugin {} + +test('abstract base class cannot be instantiated directly', () => { + assert.throws( + () => + new SitespeedioPlugin({ + name: 'x', + context: makeContext(), + queue: makeQueue() + }), + /Abstract plugin can't be instantiated/ + ); +}); + +test('rejects plugin names containing a dot', () => { + assert.throws( + () => + new TestPlugin({ + name: 'bad.name', + context: makeContext(), + queue: makeQueue() + }), + /can't contain dots/ + ); +}); + +test('rejects config missing required fields', () => { + assert.throws(() => new TestPlugin(), /requires a config object/); + assert.throws( + () => new TestPlugin({ context: makeContext(), queue: makeQueue() }), + /requires a config object/ + ); + assert.throws( + () => new TestPlugin({ name: 'p', queue: makeQueue() }), + /requires a config object/ + ); + assert.throws( + () => new TestPlugin({ name: 'p', context: makeContext() }), + /requires a config object/ + ); +}); + +test('getters return the values passed in via config', () => { + const context = makeContext(); + const queue = makeQueue(); + const options = { some: 'option' }; + const plugin = new TestPlugin({ name: 'myplugin', options, context, queue }); + + assert.equal(plugin.getName(), 'myplugin'); + assert.equal(plugin.getOptions(), options); + assert.equal(plugin.getContext(), context); + assert.equal(plugin.getStorageManager(), context.storageManager); + assert.equal(plugin.getFilterRegistry(), context.filterRegistry); + assert.equal(plugin.getLog().channel, 'sitespeed.io.plugin.myplugin'); +}); + +test('sendMessage posts a made message to the queue', async () => { + const context = makeContext(); + const queue = makeQueue(); + const plugin = new TestPlugin({ name: 'myplugin', context, queue }); + + await plugin.sendMessage('myplugin.data', { hello: 'world' }, { extra: 1 }); + + assert.equal(queue.posted.length, 1); + assert.deepEqual(queue.posted[0], { + type: 'myplugin.data', + data: { hello: 'world' }, + extras: { extra: 1 }, + source: 'myplugin' + }); +}); + +test('default processMessage throws when not overridden', async () => { + const plugin = new NoProcessPlugin({ + name: 'noproc', + context: makeContext(), + queue: makeQueue() + }); + await assert.rejects( + () => plugin.processMessage({ type: 'anything' }), + /must be implemented/ + ); +}); + +test('default open and close resolve without error', async () => { + const plugin = new TestPlugin({ + name: 'myplugin', + context: makeContext(), + queue: makeQueue() + }); + await plugin.open(); + await plugin.close(); +});