Skip to content
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
12 changes: 12 additions & 0 deletions .github/workflows/shopify-oxygen.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,15 @@ jobs:
with:
workspace_name: '@launchdarkly/shopify-oxygen-sdk'
workspace_path: packages/sdk/shopify-oxygen
- name: Install contract test service dependencies
run: yarn workspace @launchdarkly/shopify-oxygen-contract-tests install --no-immutable
- name: Build the test service
run: yarn workspace @launchdarkly/shopify-oxygen-contract-tests build
- name: Launch the test service in the background
run: yarn workspace @launchdarkly/shopify-oxygen-contract-tests start &> /dev/null &
- uses: launchdarkly/gh-actions/actions/contract-tests@61ea6c63de800b495a2fe40c0d4e4ba2a2833ee6
with:
test_service_port: 8000
token: ${{ secrets.GITHUB_TOKEN }}
# Based on run-test-harness.sh from the sdk package
extra_params: '--url http://localhost:8000 --skip "streaming.*" --skip "evaluation.*" --skip "event.*" --skip "service.*" --stop-service-at-end'
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@
"packages/telemetry/browser-telemetry",
"contract-tests",
"packages/sdk/combined-browser",
"packages/sdk/shopify-oxygen"
"packages/sdk/shopify-oxygen",
"packages/sdk/shopify-oxygen/contract-tests"
],
"private": true,
"scripts": {
Expand Down
1 change: 1 addition & 0 deletions packages/sdk/shopify-oxygen/contract-tests/.gitignore
Copy link
Member

Choose a reason for hiding this comment

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

nit: Please add a newline to the end of the file.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
*.log
24 changes: 24 additions & 0 deletions packages/sdk/shopify-oxygen/contract-tests/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"name": "@launchdarkly/shopify-oxygen-contract-tests",
"version": "0.0.0",
"main": "dist/index.js",
"scripts": {
"start": "node --inspect dist/index.js",
"build": "tsup",
"dev": "tsc --watch"
},
"type": "module",
"license": "Apache-2.0",
"private": true,
"dependencies": {
"@launchdarkly/js-server-sdk-common": "workspace:^",
"@launchdarkly/shopify-oxygen-sdk": "workspace:^",
"express": "^5.0.1"
},
"devDependencies": {
"@types/express": "^5.0.1",
"@types/node": "^18.11.9",
"tsup": "^8.5.0",
"typescript": "^4.9.0"
}
}
20 changes: 20 additions & 0 deletions packages/sdk/shopify-oxygen/contract-tests/run-test-harness.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/bin/bash

# If sdk-test-harness is not in the path, then you will need to set
# the SDK_TEST_HARNESS environment variable to the path to the sdk-test-harness binary.

# Default to the local sdk-test-harness binary if not provided
if [ -z "${SDK_TEST_HARNESS}" ]; then
SDK_TEST_HARNESS="sdk-test-harness"
fi

# Uncomment this to start the test service in the background
# yarn start &> test-service.log &

# skipping all tests that require streaming connections.
${SDK_TEST_HARNESS} --url http://localhost:8000 \
--skip "streaming.*" \
--skip "evaluation.*" \
--skip "event.*" \
--skip "service.*" \
--stop-service-at-end
68 changes: 68 additions & 0 deletions packages/sdk/shopify-oxygen/contract-tests/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import express, { Request, Response } from 'express';
import { Server } from 'http';

import { ClientPool } from './utils';

/* eslint-disable no-console */

// export DEBUG=true to enable debugging
// unset DEBUG to disable debugging
const debugging = process.env.DEBUG === 'true';

const app = express();
let server: Server | null = null;

app.use(express.json());

const port = 8000;

const clientPool = new ClientPool();

if (debugging) {
app.use((req: Request, res: Response, next: Function) => {
console.debug('request', req.method, req.url);
if (req.body) {
console.debug('request', JSON.stringify(req.body, null, 2));
}
next();
});
} else {
// NOOP global console.debug
console.debug = () => {};
}

app.get('/', (req: Request, res: Response) => {
res.header('Content-Type', 'application/json');
res.json({
capabilities: ['server-side-polling', 'server-side'],
});
});

app.delete('/', (req: Request, res: Response) => {
console.log('Test service has told us to exit');
res.status(204);
res.send();

if (server) {
server.close(() => process.exit());
}
});

app.post('/', async (req: Request, res: Response) => {
await clientPool.createClient(req.body, res);
});

app.post('/clients/:id', async (req: Request, res: Response) => {
await clientPool.runCommand(req.params.id, req.body, res);
});

app.delete('/clients/:id', async (req: Request, res: Response) => {
console.debug('DELETE request received /clients/:id');
console.debug(req.params.id);
await clientPool.deleteClient(req.params.id, res);
});

server = app.listen(port, () => {
// eslint-disable-next-line no-console
console.log('Listening on port %d', port);
});
121 changes: 121 additions & 0 deletions packages/sdk/shopify-oxygen/contract-tests/src/utils/clientPool.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import { Response } from 'express';

import { LDClient } from '@launchdarkly/js-server-sdk-common';
import { init } from '@launchdarkly/shopify-oxygen-sdk';

/* eslint-disable no-console */

// NOTE: Currently, this is a very simple client pool that only really handles the
// very limited Oxygen specific use cases... we should be expand this to be more
// general purpose in the future and maybe even come up with some shared ts interface
// to facilitate future contract testing.

// TODO: currently this class will handle the response sending as well, which may technically
// sit outside the scope of what it SHOULD be doing. We should refactor this to be more
// general purpose and allow the caller to handle the response sending.

/**
* ClientPool is a singleton that manages a pool of LDClient instances. Currently there is
* no separation between a managed client and this pool. Which means all of the client specs
* will be implemented in this class.
*
* @see https://github.com/launchdarkly/sdk-test-harness/blob/v2/docs/service_spec.md
*/
export default class ClientPool {
private _clients: Record<string, LDClient> = {};
private _clientCounter = 0;

constructor() {
this._clients = {};
this._clientCounter = 0;
}

private _makeId(): string {
this._clientCounter += 1;
return `client-${this._clientCounter}`;
}

public async runCommand(id: string, body: any, res: Response): Promise<void> {
const client = this._clients[id];
// TODO: handle the 'itCanFailCase'
if (client) {
try {
const { command, ...rest } = body;
switch (command) {
case 'evaluate': {
const { flagKey, context, defaultValue, detail } = rest.evaluate;
const evaluation = detail
? await client.variationDetail(flagKey, context, defaultValue)
: await client.variation(flagKey, context, defaultValue);
res.status(200);
res.json({ value: evaluation });
break;
}
default: {
res.status(400);
res.json({ error: `Unknown command: ${command}` });
break;
}
}
} catch (err) {
console.error(`Error running command: ${err}`);
res.status(500);
res.send();
}
} else {
res.status(404);
res.send();
}
}

public async deleteClient(id: string, res: Response): Promise<void> {
const client = this._clients[id];
if (client) {
client.close();
delete this._clients[id];
res.status(204);
res.send();
} else {
res.status(404);
res.send();
}
}

public async createClient(options: any, res: Response): Promise<void> {
try {
const id = this._makeId();
const {
configuration: { credential = 'unknown-sdk-key', polling },
} = options;

if (!polling) {
// We do not support non-polling clients yet
res.status(400);
res.send();
return;
}
const client = await init(credential, {
...(polling && {
baseUri: polling.baseUri,
}),
});

await client.waitForInitialization({ timeout: 10 });
this._clients[id] = client;
res.status(201);
res.set('Location', `/clients/${id}`);
if (!client.initialized()) {
res.status(500);
client.close();
res.send();
return;
}
console.debug(`Creating client with configuration: ${JSON.stringify(options.configuration)}`);
res.send();
} catch (err) {
console.error(`Error creating client: ${err}`);
res.status(500);
res.send();
}
Copy link

Choose a reason for hiding this comment

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

Bug: Initialization errors leave resources unclosed.

If waitForInitialization throws an exception (timeout or error), the catch block doesn't close the client, causing a resource leak. The client was created on line 97 but won't be cleaned up, potentially leaving connections or other resources open.

Fix in Cursor Fix in Web

Copy link
Member

Choose a reason for hiding this comment

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

(I think this doesn't matter in this use-case).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea it will just memory leak for a second :)

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default as ClientPool } from './clientPool';
21 changes: 21 additions & 0 deletions packages/sdk/shopify-oxygen/contract-tests/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"compilerOptions": {
"allowSyntheticDefaultImports": true,
"declaration": true,
"declarationMap": true,
"lib": ["es6"],
"module": "ES6",
"moduleResolution": "node",
"noImplicitOverride": true,
"outDir": "dist",
"resolveJsonModule": true,
"rootDir": ".",
"skipLibCheck": true,
"sourceMap": true,
"strict": true,
"stripInternal": true,
"target": "ES2017",
},
"include": ["src/**/*"],
"exclude": ["dist", "node_modules"]
}
26 changes: 26 additions & 0 deletions packages/sdk/shopify-oxygen/contract-tests/tsup.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// It is a dev dependency and the linter doesn't understand.
// @ts-ignore - tsup is a dev dependency installed at runtime
// eslint-disable-next-line import/no-extraneous-dependencies
import { defineConfig } from 'tsup';

export default defineConfig({
entry: {
index: 'src/index.ts',
},
minify: true,
format: ['esm', 'cjs'],
splitting: false,
sourcemap: false,
clean: true,
dts: true,
metafile: true,
esbuildOptions(opts) {
// This would normally be `^_(?!meta|_)`, but go doesn't support negative look-ahead assertions,
// so we need to craft something that works without it.
// So start of line followed by a character that isn't followed by m or underscore, but we
// want other things that do start with m, so we need to progressively handle more characters
// of meta with exclusions.
// eslint-disable-next-line no-param-reassign
opts.mangleProps = /^_([^m|_]|m[^e]|me[^t]|met[^a])/;
},
});