Skip to content

Commit 8285627

Browse files
authored
Implement comprehensive UI test suite with meaningful assertions and best practices (#921)
1 parent 55f8b7d commit 8285627

16 files changed

Lines changed: 1148 additions & 277 deletions

File tree

.github/copilot-instructions.md

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,94 @@ electron . --enable-mcp-introspection --remote-debugging-port=9223
5454
3. **Include screenshots** - Visual verification is required for UI changes
5555
4. **Handle asynchronous operations properly** - This is an MQTT message queue tool
5656

57+
### Best Practices for UI Tests
58+
59+
#### 1. Use Given-When-Then Pattern
60+
Structure tests with clear Given-When-Then comments to make them readable:
61+
62+
```typescript
63+
it('Given a JSON message sent to topic foo/bar/baz, the tree should display nested topics', async function () {
64+
// Given: Mock MQTT publishes JSON to foo/bar/baz
65+
// When: We wait for the topic to appear in the tree
66+
// Then: Topic hierarchy should be visible (foo -> bar -> baz)
67+
})
68+
```
69+
70+
#### 2. Wait for Elements, Don't Use Fixed Delays
71+
Prefer `waitFor` over `sleep` whenever possible:
72+
73+
```typescript
74+
// ✓ Good: Wait for specific element
75+
const topic = await page.locator('span[data-test-topic="kitchen"]')
76+
await topic.waitFor({ state: 'visible', timeout: 5000 })
77+
78+
// ✗ Bad: Fixed delay without verification
79+
await sleep(5000)
80+
```
81+
82+
#### 3. Use Meaningful Assertions
83+
Every test should have explicit assertions that verify the expected state:
84+
85+
```typescript
86+
// ✓ Good: Explicit assertion with meaningful message
87+
const treeNodes = await page.locator('[class*="TreeNode"]')
88+
const count = await treeNodes.count()
89+
expect(count).to.be.greaterThan(0, 'Topic tree should contain nodes')
90+
91+
// ✗ Bad: No assertion, only screenshot
92+
await page.screenshot({ path: 'test.png' })
93+
```
94+
95+
#### 4. Test Data-Driven Scenarios
96+
Write tests that describe the data flow:
97+
98+
```typescript
99+
it('Given messages sent to livingroom/lamp/state and livingroom/lamp/brightness, both should appear under livingroom/lamp', async function () {
100+
// Test implementation verifies the specific data flow
101+
})
102+
```
103+
104+
#### 5. Use Data Test Attributes
105+
Leverage `data-test-*` attributes for reliable selectors:
106+
107+
```typescript
108+
// ✓ Good: Use data-test attributes
109+
const topic = await page.locator('span[data-test-topic="kitchen"]')
110+
111+
// ⚠ Acceptable: Use role/text when data attributes aren't available
112+
const button = await page.locator('//button/span[contains(text(),"Connect")]')
113+
114+
// ✗ Bad: Rely on CSS classes that may change
115+
const topic = await page.locator('.MuiTreeItem-label')
116+
```
117+
118+
#### 6. Verify Multiple Aspects
119+
Test should verify both state and UI:
120+
121+
```typescript
122+
// Verify the action completed
123+
const isVisible = await disconnectButton.isVisible()
124+
expect(isVisible).to.be.true
125+
126+
// Capture screenshot for visual verification
127+
await page.screenshot({ path: 'test-screenshot-connection.png' })
128+
```
129+
130+
#### 7. Handle MQTT Asynchronous Nature
131+
Account for message propagation time:
132+
133+
```typescript
134+
// Publish message
135+
await mockClient.publish('topic/name', 'value')
136+
137+
// Wait for UI to update
138+
await page.locator(`text="value"`).waitFor({ timeout: 5000 })
139+
140+
// Verify state
141+
const value = await page.textContent('.message-value')
142+
expect(value).toBe('value')
143+
```
144+
57145
### Handling MQTT Asynchronous Operations
58146

59147
MQTT is inherently asynchronous. When writing tests:

.github/workflows/tests.yml

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,45 @@ jobs:
1818
run: yarn build
1919
- name: Test
2020
run: yarn test
21-
- name: UI-Test
21+
22+
ui-tests:
23+
runs-on: ubuntu-latest
24+
container:
25+
image: ghcr.io/thomasnordquist/mqtt-explorer-ui-tests:latest
26+
volumes:
27+
- ./:/app
28+
options: --user root
29+
steps:
30+
- uses: actions/checkout@v4
31+
- name: Install Packages
32+
run: yarn install --frozen-lockfile
33+
- name: Build
34+
run: yarn build
35+
- name: Run UI Tests
36+
run: ./scripts/runUiTests.sh
37+
- name: Upload Test Screenshots
38+
if: always()
39+
uses: actions/upload-artifact@v4
40+
with:
41+
name: ui-test-screenshots
42+
path: |
43+
test-screenshot-*.png
44+
retention-days: 30
45+
46+
demo-video:
47+
runs-on: ubuntu-latest
48+
container:
49+
image: ghcr.io/thomasnordquist/mqtt-explorer-ui-tests:latest
50+
volumes:
51+
- ./:/app
52+
options: --user root
53+
steps:
54+
- uses: actions/checkout@v4
55+
- name: Install Packages
56+
run: yarn install --frozen-lockfile
57+
- name: Build
58+
run: yarn build
59+
- name: Generate Demo Video
2260
run: yarn ui-test
2361
- name: Post-processing
2462
run: ./scripts/prepareVideo.sh

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,6 @@ screen*.png
1414
mqtt-explorer-mcp-screenshot.png
1515
screenshot-mcp-*.png
1616
test-mcp-introspection.js
17+
18+
# UI test artifacts
19+
test-screenshot-*.png

Readme.md

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,28 +41,35 @@ The `app` directory contains all the rendering logic, the `backend` directory cu
4141

4242
## Automated Tests
4343

44-
To achieve a reliable product automated tests run regularly on travis.
44+
To achieve a reliable product automated tests run regularly on CI.
4545

46-
- Data model
47-
- MQTT integration
48-
- UI-Tests (The demo is a recorded ui test)
46+
- **Data model tests**: `yarn test:backend`
47+
- **App tests**: `yarn test:app`
48+
- **UI test suite**: `yarn test:ui` (independent, deterministic tests)
49+
- **Demo video**: `yarn ui-test` (UI test recording for documentation)
4950

50-
## Run UI-tests
51+
### Run UI Test Suite
5152

52-
A [mosquitto](https://mosquitto.org/) MQTT broker is required to run the ui-tests.
53-
54-
Run tests with
53+
The UI test suite validates core functionality through automated browser tests. Each test is independent and deterministic.
5554

5655
```bash
57-
# Run chromedriver in a separate terminal session
58-
./node_modules/.bin/chromedriver --url-base=wd/hub --port=9515 --verbose
56+
# Run with automated setup (recommended)
57+
./scripts/runUiTests.sh
58+
59+
# Or run directly (requires manual MQTT broker setup)
60+
yarn build
61+
yarn test:ui
5962
```
6063

61-
Compile and execute tests
64+
See [docs/UI-TEST-SUITE.md](docs/UI-TEST-SUITE.md) for more details.
65+
66+
### Run Demo Video Generation
67+
68+
A [mosquitto](https://mosquitto.org/) MQTT broker is required to generate the demo video.
6269

6370
```bash
64-
npm run build
65-
node dist/src/spec/webdriverio.js
71+
yarn build
72+
yarn ui-test
6673
```
6774

6875
## Create a release

app/src/actions/Publish.ts

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,27 @@ export const setTopic = (topic?: string): Action => {
1414
}
1515
}
1616

17-
export const openFile = (encoding: 'utf8' = 'utf8') => async (dispatch: Dispatch<any>, getState: () => AppState) => {
18-
try {
19-
const file = await getFileContent(encoding)
20-
if (file) {
21-
dispatch(
22-
setPayload(file.data))
17+
export const openFile =
18+
(encoding: 'utf8' = 'utf8') =>
19+
async (dispatch: Dispatch<any>, getState: () => AppState) => {
20+
try {
21+
const file = await getFileContent(encoding)
22+
if (file) {
23+
dispatch(setPayload(file.data))
24+
}
25+
} catch (error) {
26+
dispatch(showError(error))
2327
}
24-
} catch (error) {
25-
dispatch(showError(error))
2628
}
27-
}
2829

2930
type FileParameters = {
30-
name: string,
31+
name: string
3132
data: string
3233
}
3334
async function getFileContent(encoding: string): Promise<FileParameters | undefined> {
3435
const rejectReasons = {
3536
noFileSelected: 'No file selected',
36-
errorReadingFile: 'Error reading file'
37+
errorReadingFile: 'Error reading file',
3738
}
3839

3940
const { canceled, filePaths } = await rendererRpc.call(makeOpenDialogRpc(), {

app/src/components/Sidebar/CodeDiff/ChartPreview.tsx

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,24 +52,28 @@ function ChartPreview(props: Props) {
5252
/>
5353
</Tooltip>
5454
) : (
55-
<Tooltip title="Add to chart panel, not enough data for preview">
56-
<ShowChart
57-
onClick={onClick}
58-
className={props.classes.icon}
59-
style={{ color: '#aaa' }}
60-
data-test-type="ShowChart"
61-
data-test={props.literal.path}
62-
/>
63-
</Tooltip>
64-
)
55+
<Tooltip title="Add to chart panel, not enough data for preview">
56+
<ShowChart
57+
onClick={onClick}
58+
className={props.classes.icon}
59+
style={{ color: '#aaa' }}
60+
data-test-type="ShowChart"
61+
data-test={props.literal.path}
62+
/>
63+
</Tooltip>
64+
)
6565

6666
return (
6767
<span>
6868
{addChartToPanelButton}
6969
<Popper open={open} anchorEl={chartIconRef.current} placement="left-end">
7070
<Fade in={open} timeout={300}>
7171
<Paper style={{ width: '300px' }}>
72-
{open ? <TopicPlot node={props.treeNode} history={props.treeNode.messageHistory} dotPath={props.literal.path} /> : <span />}
72+
{open ? (
73+
<TopicPlot node={props.treeNode} history={props.treeNode.messageHistory} dotPath={props.literal.path} />
74+
) : (
75+
<span />
76+
)}
7377
</Paper>
7478
</Fade>
7579
</Popper>

events/Events.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,10 @@ export const getAppVersion: RpcEvent<void, string> = {
5555
topic: 'getAppVersion',
5656
}
5757

58-
export const writeToFile: RpcEvent<{ filePath: string, data: string, encoding?: string }, void> = {
58+
export const writeToFile: RpcEvent<{ filePath: string; data: string; encoding?: string }, void> = {
5959
topic: 'writeFile',
6060
}
6161

62-
export const readFromFile: RpcEvent<{ filePath: string, encoding?: string }, Buffer> = {
62+
export const readFromFile: RpcEvent<{ filePath: string; encoding?: string }, Buffer> = {
6363
topic: 'readFromFile',
64-
}
64+
}

events/OpenDialogRequest.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,4 @@ export function makeSaveDialogRpc(): RpcEvent<SaveDialogOptions, SaveDialogRetur
1111
return {
1212
topic: 'saveDialog',
1313
}
14-
}
14+
}

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
"test": "yarn test:app && yarn test:backend",
1313
"test:app": "cd app && yarn test",
1414
"test:backend": "cd backend && yarn test",
15+
"test:ui": "tsc && mocha --require source-map-support/register dist/src/spec/ui-tests.spec.js",
1516
"test:mcp": "tsc && node dist/src/spec/testMcpIntrospection.js",
1617
"install": "cd app && yarn && cd ..",
1718
"dev": "npm-run-all --parallel dev:*",

scripts/runUiTests.sh

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
#!/bin/bash
2+
set -e
3+
4+
function finish {
5+
set +e
6+
echo "Exiting, cleaning up.."
7+
8+
if [[ ! -z "$PID_MOSQUITTO" ]]; then
9+
echo "Stopping mosquitto ($PID_MOSQUITTO).."
10+
kill "$PID_MOSQUITTO" || echo "Already stopped"
11+
fi
12+
13+
if [[ ! -z "$PID_XVFB" ]]; then
14+
echo "Stopping XVFB ($PID_XVFB).."
15+
kill "$PID_XVFB" || echo "Already stopped"
16+
fi
17+
}
18+
19+
trap finish EXIT
20+
21+
DIMENSIONS="1024x720"
22+
SCR=99
23+
24+
# Start new window manager
25+
Xvfb :$SCR -screen 0 "$DIMENSIONS"x24 -ac &
26+
export PID_XVFB=$!
27+
sleep 2
28+
29+
# Start mqtt broker
30+
mosquitto &
31+
export PID_MOSQUITTO=$!
32+
sleep 1
33+
34+
# Run UI tests
35+
DISPLAY=:$SCR yarn test:ui
36+
TEST_EXIT_CODE=$?
37+
38+
echo "UI tests exited with $TEST_EXIT_CODE"
39+
exit $TEST_EXIT_CODE

0 commit comments

Comments
 (0)