Skip to content

Commit 6fe00c8

Browse files
authored
Add e2e tests, fix dial attempt stacking, add testing guidelines (#677)
1 parent 08c0925 commit 6fe00c8

32 files changed

+1746
-349
lines changed

.eslintrc.cjs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ module.exports = {
99
'dist',
1010
'docs',
1111
'playwright-report',
12+
'vitest-report',
13+
'vitest-e2e-report',
1214
/*
1315
* TODO(mc, 2023-04-06): something about nested node_modules in examples
1416
* is causing eslint to choke. Investigate workspaces as a solution

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,3 +118,5 @@ src/api-version.ts
118118
*.DS_Store
119119

120120
playwright-report/
121+
vitest-report/
122+
vitest-e2e-report/

CONTRIBUTING.md

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,54 @@ make test
5353
make lint
5454
make format
5555
```
56+
57+
## Testing
58+
59+
Our test suite is divided into unit tests and integration tests.
60+
61+
### Unit Testing
62+
63+
We use [Vitest](https://vitest.dev/) for unit testing to ensure that individual components of the SDK are well-tested in isolation.
64+
65+
**Key Principles:**
66+
67+
- **Location:** Unit tests are located in `src/**/__tests__`. Related test data and mocks are stored in adjacent `__fixtures__/` and `__mocks__/` directories, respectively.
68+
- **Isolation:** Tests must be independent. Use `vi.mock()` to mock external dependencies and ensure each test case can run on its own.
69+
- **Clarity:** Follow the Arrange-Act-Assert (AAA) pattern to structure tests clearly. Use descriptive names for `describe` blocks and test cases (e.g., `it('should do X when Y')`).
70+
71+
You can run all unit tests with:
72+
73+
```shell
74+
make test
75+
```
76+
77+
### Integration Testing
78+
79+
Integration tests verify the end-to-end interaction between the SDK and a live `viam-server`. We use [Vitest](https://vitest.dev/) for Node.js tests and [Playwright](https://playwright.dev/) for browser tests. All integration test code resides in the `e2e/` directory.
80+
81+
**Key Principles:**
82+
83+
- **File Naming:** Tests are separated by environment:
84+
- `*.node.spec.ts` for Node.js-only tests.
85+
- `*.browser.spec.ts` for browser-only tests.
86+
- **Browser Testing:** Browser tests interact with a UI test harness (`e2e/index.html`) via a Playwright Page Object Model (`e2e/fixtures/robot-page.ts`). This ensures tests are stable and maintainable.
87+
- **Node.js Testing:** Node.js tests interact with the SDK directly using a gRPC connection.
88+
89+
Before running integration tests for the first time, you must download the `viam-server` binary:
90+
91+
```shell
92+
cd e2e && ./setup.sh
93+
```
94+
95+
You can run the full integration test suite with:
96+
97+
```shell
98+
make test-e2e
99+
```
100+
101+
You can also run the Node.js and browser suites separately:
102+
103+
```shell
104+
npm run e2e:node
105+
npm run e2e:browser
106+
```

Makefile

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -95,15 +95,20 @@ build-docs: clean-docs build-buf
9595

9696
.PHONY: install-playwright
9797
install-playwright:
98-
cd e2e && npm install
99-
cd e2e && npx playwright install --with-deps
98+
npm run e2e:browser-install
10099

101100
e2e/bin/viam-server:
102101
bash e2e/setup.sh
103102

104-
.PHONY: run-e2e-server
105-
run-e2e-server: e2e/bin/viam-server
106-
e2e/bin/viam-server --config=./e2e/server_config.json
103+
.PHONY: test-e2e
104+
test-e2e: e2e/bin/viam-server install-playwright
105+
npm run e2e:browser
106+
npm run e2e:node
107107

108-
test-e2e: e2e/bin/viam-server build install-playwright
109-
cd e2e && npm run e2e:playwright
108+
.PHONY: test-e2e-node
109+
test-e2e-node: e2e/bin/viam-server
110+
npm run e2e:node
111+
112+
.PHONY: test-e2e-browser
113+
test-e2e-browser: e2e/bin/viam-server install-playwright
114+
npm run e2e:browser

e2e/fixtures/configs/base.json

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"network": {
3+
"fqdn": "e2e-ts-sdk",
4+
"bind_address": ":9090"
5+
},
6+
"components": [
7+
{
8+
"name": "base1",
9+
"type": "base",
10+
"model": "fake"
11+
},
12+
{
13+
"name": "servo1",
14+
"type": "servo",
15+
"model": "fake"
16+
},
17+
{
18+
"name": "motor1",
19+
"type": "motor",
20+
"model": "fake"
21+
}
22+
]
23+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import type { DialConf } from '../../main';
2+
3+
const DEFAULT_HOST = 'e2e-ts-sdk';
4+
const DEFAULT_SERVICE_HOST = 'http://localhost:9090';
5+
const DEFAULT_SIGNALING_ADDRESS = 'http://localhost:9090';
6+
const DEFAULT_ICE_SERVERS = [{ urls: 'stun:global.stun.twilio.com:3478' }];
7+
8+
export const defaultConfig: DialConf = {
9+
host: DEFAULT_HOST,
10+
serviceHost: DEFAULT_SERVICE_HOST,
11+
signalingAddress: DEFAULT_SIGNALING_ADDRESS,
12+
iceServers: DEFAULT_ICE_SERVERS,
13+
} as const;
14+
15+
export const invalidConfig: DialConf = {
16+
host: DEFAULT_HOST,
17+
serviceHost: 'http://invalid-host:9999',
18+
signalingAddress: DEFAULT_SIGNALING_ADDRESS,
19+
iceServers: DEFAULT_ICE_SERVERS,
20+
dialTimeout: 2000,
21+
} as const;
22+
23+
export const defaultNodeConfig: DialConf = {
24+
host: DEFAULT_SERVICE_HOST,
25+
noReconnect: true,
26+
} as const;
27+
28+
export const invalidNodeConfig: DialConf = {
29+
host: 'http://invalid-host:9999',
30+
noReconnect: true,
31+
} as const;

e2e/fixtures/robot-page.ts

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
import { test as base, type Page } from '@playwright/test';
2+
import type { Robot, RobotClient } from '../../src/robot';
3+
import type { ResolvedReturnType } from '../helpers/api-types';
4+
5+
export class RobotPage {
6+
private readonly connectionStatusID = 'connection-status';
7+
private readonly dialingStatusID = 'dialing-status';
8+
private readonly connectButtonID = 'connect-btn';
9+
private readonly disconnectButtonID = 'disconnect-btn';
10+
private readonly outputID = 'output';
11+
12+
constructor(private readonly page: Page) {}
13+
14+
async ensureReady(): Promise<void> {
15+
if (!this.page.url().includes('localhost:5173')) {
16+
await this.page.goto('/');
17+
await this.page.waitForSelector('body[data-ready="true"]');
18+
}
19+
}
20+
21+
async connect(): Promise<void> {
22+
await this.ensureReady();
23+
await this.page.getByTestId(this.connectButtonID).click();
24+
await this.page.waitForSelector(
25+
`[data-testid="${this.connectionStatusID}"]:is(:text("Connected"))`
26+
);
27+
}
28+
29+
async disconnect(): Promise<void> {
30+
await this.page.getByTestId(this.disconnectButtonID).click();
31+
await this.page.waitForSelector(
32+
`[data-testid="${this.connectionStatusID}"]:is(:text("Disconnected"))`
33+
);
34+
}
35+
36+
async getConnectionStatus(): Promise<string> {
37+
const connectionStatusEl = this.page.getByTestId(this.connectionStatusID);
38+
const text = await connectionStatusEl.textContent();
39+
return text ?? 'Unknown';
40+
}
41+
42+
async waitForDialing(): Promise<void> {
43+
await this.page.waitForSelector(
44+
`[data-testid="${this.dialingStatusID}"]:not(:empty)`,
45+
{ timeout: 5000 }
46+
);
47+
}
48+
49+
async waitForFirstDialingAttempt(): Promise<void> {
50+
await this.page.waitForFunction(
51+
(testId: string) => {
52+
const el = document.querySelector(`[data-testid="${testId}"]`);
53+
const text = el?.textContent ?? '';
54+
const match = text.match(/attempt (?<attemptNumber>\d+)/u);
55+
if (!match?.groups) {
56+
return false;
57+
}
58+
const attemptNumber = Number.parseInt(
59+
match.groups.attemptNumber ?? '0',
60+
10
61+
);
62+
return attemptNumber === 1;
63+
},
64+
this.dialingStatusID,
65+
{ timeout: 10_000 }
66+
);
67+
}
68+
69+
async waitForSubsequentDialingAttempts(): Promise<void> {
70+
await this.page.waitForFunction(
71+
(testId: string) => {
72+
const el = document.querySelector(`[data-testid="${testId}"]`);
73+
const text = el?.textContent ?? '';
74+
const match = text.match(/attempt (?<attemptNumber>\d+)/u);
75+
if (!match?.groups) {
76+
return false;
77+
}
78+
const attemptNumber = Number.parseInt(
79+
match.groups.attemptNumber ?? '0',
80+
10
81+
);
82+
return attemptNumber > 1;
83+
},
84+
this.dialingStatusID,
85+
{ timeout: 10_000 }
86+
);
87+
}
88+
89+
async getDialingStatus(): Promise<string> {
90+
const dialingStatusEl = this.page.getByTestId(this.dialingStatusID);
91+
const text = await dialingStatusEl.textContent();
92+
return text ?? '';
93+
}
94+
95+
async getOutput<T extends Robot, K extends keyof T>(): Promise<
96+
ResolvedReturnType<T[K]>
97+
> {
98+
// Wait for the output to be updated by checking for the data-has-output attribute
99+
await this.page.waitForSelector(
100+
`[data-testid="${this.outputID}"][data-has-output="true"]`,
101+
{ timeout: 30_000 }
102+
);
103+
const outputEl = this.page.getByTestId(this.outputID);
104+
const text = await outputEl.textContent();
105+
return JSON.parse(text ?? '{}') as ResolvedReturnType<T[K]>;
106+
}
107+
108+
async clickButton(testId: string): Promise<void> {
109+
await this.page.click(`[data-testid="${testId}"]`);
110+
}
111+
112+
async clickRobotAPIButton(apiName: keyof RobotClient): Promise<void> {
113+
await this.page.click(`[data-robot-api="${apiName}"]`);
114+
}
115+
116+
getPage(): Page {
117+
return this.page;
118+
}
119+
}
120+
121+
export const withRobot = base.extend<{ robotPage: RobotPage }>({
122+
robotPage: async ({ page }, use) => {
123+
const robotPage = new RobotPage(page);
124+
await use(robotPage);
125+
},
126+
});

e2e/helpers/api-types.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
2+
export type ArgumentsType<T> = T extends (...args: infer U) => any ? U : never;
3+
4+
export type ResolvedReturnType<T> = T extends (
5+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
6+
...args: any[]
7+
) => Promise<infer R>
8+
? R
9+
: never;

0 commit comments

Comments
 (0)