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

test: Adding web client testing framework #498

Merged
merged 16 commits into from
Sep 10, 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
21 changes: 21 additions & 0 deletions .github/workflows/test.yml
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: missing newline on this file

Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,24 @@ jobs:
- name: Kill miden-node
if: always()
run: make kill-node

integration_tests_web_client:
name: integration_tests_web_client
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@main
- name: Install Rust
run: |
rustup update --no-self-update
rustup target add wasm32-unknown-unknown
- name: Install Node.js
uses: actions/setup-node@v4
with:
node-version: "20"
- run: make clean-node
- run: make node
- run: make start-node > /dev/null &
- run: make integration-test-web-client
- name: Kill miden-node
if: always()
run: make kill-node
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* Added support for importing committed notes from older blocks than current (#472).
* Added support for account export in the CLI (#479).
* Added the Web Client Crate
* Added testing suite for the Web Client Crate
* [BREAKING] Refactored `TransactionRequest` to represent a generalized transaction (#438).

### Enhancements
Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ test-deps: ## Install dependencies for tests
integration-test: ## Run integration tests
cargo nextest run --workspace --exclude miden-client-web --release --test=integration $(FEATURES_CLI) --no-default-features

.PHONY: integration-test-web-client
integration-test-web-client: ## Run integration tests for the web client
cd ./crates/web-client && npm run test:clean

.PHONY: integration-test-full
integration-test-full: ## Run the integration test binary with ignored tests included
cargo nextest run --workspace --exclude miden-client-web --release --test=integration $(FEATURES_CLI)
Expand Down
6 changes: 6 additions & 0 deletions crates/web-client/.mocharc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"require": ["ts-node/register", "esm"],
"extension": ["ts"],
"spec": "test/**/*.test.ts",
"timeout": 600000
}
8 changes: 4 additions & 4 deletions crates/web-client/js/types/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export {
WebClient,
NewTransactionResult,
SerializedAccountStub,
NewSwapTransactionResult
WebClient,
NewTransactionResult,
SerializedAccountStub,
NewSwapTransactionResult,
} from "./crates/miden_client";
21 changes: 17 additions & 4 deletions crates/web-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,28 @@
"../LICENSE.md"
],
"scripts": {
"build": "rimraf dist && rollup -c rollup.config.js && cpr js/types dist && rimraf dist/wasm*"
"build": "rimraf dist && rollup -c rollup.config.js && cpr js/types dist && rimraf dist/wasm*",
"test": "node --loader ts-node/esm --loader esm ./node_modules/mocha/bin/mocha --file ./test/mocha.global.setup.mjs",
"test:logs": "DEBUG_MODE=true node --loader ts-node/esm --loader esm ./node_modules/mocha/bin/mocha --file ./test/mocha.global.setup.mjs",
"test:clean": "npm install && npm run build && node --loader ts-node/esm --loader esm ./node_modules/mocha/bin/mocha --file ./test/mocha.global.setup.mjs"
},
"devDependencies": {
"@rollup/plugin-commonjs": "^25.0.7",
"@rollup/plugin-node-resolve": "^15.2.3",
"@wasm-tool/rollup-plugin-rust": "^2.4.5",
"@types/chai": "^4.3.17",
"@types/mocha": "^10.0.7",
"@types/node": "^22.4.1",
"chai": "^5.1.1",
"cpr": "^3.0.1",
"rimraf": "^5.0.1",
"rollup": "^3.27.2"
"esm": "^3.2.25",
"http-server": "^14.1.1",
"mocha": "^10.7.3",
"puppeteer": "^23.1.0",
"rimraf": "^6.0.1",
"rollup": "^3.27.2",
"ts-node": "^10.9.2",
"typescript": "^5.5.4",
"@wasm-tool/rollup-plugin-rust": "wasm-tool/rollup-plugin-rust"
},
"dependencies": {
"dexie": "^4.0.1"
Expand Down
82 changes: 82 additions & 0 deletions crates/web-client/test/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# Testing
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should specify a node version somewhere the supported node version.


The .wasm must be run within the context of a webpage. To this end, we've set up a Mocha
test suite which hosts the .wasm on a local server and then executes WebClient commands
within the context of the web page.

## Prerequisites

1. [Node](https://nodejs.org/en/download/package-manager)
- Node Version >= v20.16.0
1. These instructions utilize [yarn](https://classic.yarnpkg.com/lang/en/docs/install) but can also be executed with npm

## Running tests

1. Install dependencies via `yarn`
1. Ensure the .wasm is built by running `yarn build`
1. In crates/web-client run `yarn test` to run all tests
igamigo marked this conversation as resolved.
Show resolved Hide resolved

- Can alternatively run `yarn test:clean` to run the .wasm build process prior to testing. We provide both paths as the build process can take some time.

1. For running an individual test by name run `yarn test -g <test-name>`
1. To enable logging from the client to the terminal, run `yarn test:logs`

## Writing tests

1. The test setup in `mocha.global.setup.mjs` should expose the `create_client` function which can be used inside tests.
- Any further setup of wasm code should be done in this file and similarly expose a function for testing here
1. `webClientTestUtils.js` should contain all interfaces for interacting with the web client. If further methods need to be added, follow existing patterns which use the exposed `testingPage` and pass through any required arguments to the page execution. Example:

```
/**
*
* @param {string} arg1
* @param {boolean} arg2
*
* @returns {Promise<string>} The result
*/
export const webClientCall = async (arg1, arg2) => {
return await testingPage.evaluate(
async (_arg1, _arg2) => {
if (!window.client) {
await window.create_client();
}
/** @type {WebClient} */
const client = window.client;
const result = client.webClientCall(_arg1, _arg2);
return result;
},
arg1,
arg2
);
};
```

- Add JSDocs to methods. This will allow typing in the `*.test.ts` files.
- We add the `if (!window.client)` to avoid spinning up clients unnecessarily lengthening test time. This unfortunately cannot be pulled out to a helper method as the testingPage scope does not share the scope of the file.
- Similarly, the boilerplate for passing args through as shown above is necessary due to scoping.

## Debugging

1. When inside of a `page.evaluate` , console logs are being sent to the servers console rather than your IDE's. You can uncomment the line as seen below in the `mocha.global.setup.mjs`:

```
page.on("console", (msg) => console.log("PAGE LOG:", msg.text()));
```

This will forward logs from the server to your terminal logs

## Troubleshooting

1. When trying to run the tests, if you receieve the following error:

```
Error: Could not find Chrome (ver. 128.0.6613.119). This can occur if either
1. you did not perform an installation before running the script (e.g. `npx puppeteer browsers install ${browserType}`) or
2. your cache path is incorrectly configured (which is: /Users/ignacioamigo/.cache/puppeteer).
For (2), check out our guide on configuring puppeteer at https://pptr.dev/guides/configuration.
```

Try running: `npx puppeteer browsers install` and then run the tests again
15 changes: 15 additions & 0 deletions crates/web-client/test/faucet.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { createNewFaucet, isValidAddress } from "./webClientTestUtils.js";

describe("faucet tests", () => {
it("create a new faucet", async () => {
const result = await createNewFaucet(
"OffChain",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a heads up, the storage mode names will change to "Private" and "Public" when #514 gets merged.

false,
"DMX",
"10",
"1000000"
);

isValidAddress(result);
});
});
5 changes: 5 additions & 0 deletions crates/web-client/test/global.test.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { Page } from "puppeteer";

declare module "./mocha.global.setup.mjs" {
export const testingPage: Page;
}
85 changes: 85 additions & 0 deletions crates/web-client/test/mocha.global.setup.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import puppeteer from "puppeteer";
import { spawn } from "child_process";

import { register } from "ts-node";
import { env } from "process";

register({
project: "./tsconfig.json",
});

let serverProcess;
let browser;
let testingPage;

const TEST_SERVER_PORT = 8080;
const TEST_SERVER = `http://localhost:${TEST_SERVER_PORT}`;

// Should be in sync with the rpc port in tests/config/miden-node.toml
const LOCAL_MIDEN_NODE_PORT = 57291;

before(async () => {
console.log("Starting test server...");
serverProcess = spawn("http-server", ["./dist", "-p", TEST_SERVER_PORT], {
stdio: "inherit",
});

browser = await puppeteer.launch({ headless: true });
testingPage = await browser.newPage();
await testingPage.goto(TEST_SERVER);

if (env.DEBUG_MODE) {
testingPage.on("console", (msg) => console.log("PAGE LOG:", msg.text()));
}

// Creates the client in the test context and attach to window object
await testingPage.exposeFunction("create_client", async () => {
await testingPage.evaluate(async (port) => {
const { WebClient } = await import("./index.js");
let rpc_url = `http://localhost:${port}`;
const client = new WebClient();
await client.create_client(rpc_url);

window.client = client;
}, LOCAL_MIDEN_NODE_PORT);
});
});

after(async () => {
console.log("Stopping test server...");

console.log("Closing browser...");
await browser.close();
console.log("Browser closed.");

console.log("Beginning server process cleanup...");
if (serverProcess && !serverProcess.killed) {
console.log("Killing server process...");
serverProcess.kill("SIGTERM"); // Send the SIGTERM signal to terminate the server
}

console.log("Waiting for server process to exit...");
await new Promise((resolve, reject) => {
if (serverProcess.exitCode !== null) {
// Process has already exited, resolve immediately
console.log(
`Server process had already exited with code ${serverProcess.exitCode}`
);
return resolve();
}

serverProcess.on("close", (code) => {
console.log(`Server process exited with code ${code}`);
resolve();
});

serverProcess.on("error", (error) => {
console.error("Error killing server process:", error);
reject(error);
});
});

console.log("Test server stopped.");
});

export { testingPage };
9 changes: 9 additions & 0 deletions crates/web-client/test/wallet.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { createNewWallet, isValidAddress } from "./webClientTestUtils.js";

describe("wallet tests", () => {
it("create a new wallet", async () => {
const result = await createNewWallet("OffChain", false);

isValidAddress(result);
});
});
Comment on lines +3 to +9
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be nice if we were to test that the accounts that the client is tracking (ie, get_accounts()) contained the new address. Maybe you were planning on this already for future tests but just in case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep that is part of future tests

Loading