-
Notifications
You must be signed in to change notification settings - Fork 959
fix(chrome-extension): add start/stop controls for bridge mode #2177
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
Changes from 2 commits
4bec457
a11b50a
3f8aa0e
3abab8d
29ea3db
df25c06
ea98fd9
6a8253f
05f56dc
c359c94
bfeaae2
06ddc4a
52c8578
495d54f
14fe38c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,8 @@ import { | |
| ClearOutlined, | ||
| DownOutlined, | ||
| LoadingOutlined, | ||
| PauseCircleOutlined, | ||
| PlayCircleOutlined, | ||
| } from '@ant-design/icons'; | ||
| import { Button, Input, List, Spin } from 'antd'; | ||
| import dayjs from 'dayjs'; | ||
|
|
@@ -223,6 +225,40 @@ export default function Bridge() { | |
| }); | ||
| }; | ||
|
|
||
| const isBridgeActive = | ||
| bridgeStatus === 'listening' || bridgeStatus === 'connected'; | ||
|
|
||
| const handleToggleBridge = () => { | ||
| if (isBridgeActive) { | ||
| // Stop bridge | ||
| chrome.runtime.sendMessage( | ||
| { type: workerMessageTypes.BRIDGE_STOP }, | ||
|
Comment on lines
+236
to
+237
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This handler makes Useful? React with 👍 / 👎. |
||
| (response) => { | ||
| if (response?.success) { | ||
| setBridgeStatus('closed'); | ||
| // Reset connection status message so next session gets a new message group | ||
| connectionStatusMessageId.current = null; | ||
| } | ||
| }, | ||
| ); | ||
| } else { | ||
| // Start bridge with optional custom server URL | ||
| const endpoint = | ||
| serverUrl && serverUrl !== DEFAULT_SERVER_URL ? serverUrl : undefined; | ||
| chrome.runtime.sendMessage( | ||
| { | ||
| type: workerMessageTypes.BRIDGE_START, | ||
| payload: { serverEndpoint: endpoint }, | ||
| }, | ||
| (response) => { | ||
| if (response?.success) { | ||
| setBridgeStatus(response.status || 'listening'); | ||
| } | ||
| }, | ||
| ); | ||
| } | ||
| }; | ||
|
|
||
| // check if scrolled to bottom | ||
| const checkIfScrolledToBottom = () => { | ||
| if (messageListRef.current) { | ||
|
|
@@ -286,11 +322,10 @@ export default function Bridge() { | |
|
|
||
| let statusIcon; | ||
| let statusTip: string; | ||
| if ( | ||
| bridgeStatus === 'listening' || | ||
| bridgeStatus === 'disconnected' || | ||
| bridgeStatus === 'closed' | ||
| ) { | ||
| if (bridgeStatus === 'closed') { | ||
| statusIcon = iconForStatus('failed'); | ||
| statusTip = 'Stopped'; | ||
| } else if (bridgeStatus === 'listening' || bridgeStatus === 'disconnected') { | ||
| statusIcon = ( | ||
| <Spin | ||
| className="status-loading-icon" | ||
|
|
@@ -441,6 +476,18 @@ export default function Bridge() { | |
| <span className="bottom-status-icon">{statusIcon}</span> | ||
| <span className="bottom-status-tip">{statusTip}</span> | ||
| </div> | ||
| <div className="bottom-status-divider" /> | ||
| <Button | ||
| type="text" | ||
| size="small" | ||
| className="bridge-toggle-btn" | ||
| icon={ | ||
| isBridgeActive ? <PauseCircleOutlined /> : <PlayCircleOutlined /> | ||
| } | ||
| onClick={handleToggleBridge} | ||
| > | ||
| {isBridgeActive ? 'Stop' : 'Start'} | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,219 @@ | ||
| /** | ||
| * Unit tests for bridge mode start/stop and server URL configuration. | ||
| * | ||
| * Covers: | ||
| * 1. BridgeConnector can be stopped and restarted | ||
| * 2. BridgeConnector can be started with a different server endpoint | ||
| * 3. Worker message handling for BRIDGE_START and BRIDGE_STOP | ||
| * | ||
| * Related issue: https://github.com/web-infra-dev/midscene/issues/2119 | ||
| * - Users cannot stop/start listening in bridge mode | ||
| * - Users cannot change remote server URL because the input is always disabled | ||
| * | ||
| * Run: npx vitest run apps/chrome-extension/tests/bridge-start-stop.test.ts | ||
| */ | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| // ─── BridgeConnector unit tests ────────────────────────────────────────────── | ||
|
|
||
| // We can't import the real BridgeConnector because it depends on | ||
| // ExtensionBridgePageBrowserSide which requires a browser environment. | ||
| // Instead, we test the core state machine logic directly. | ||
|
|
||
| describe('BridgeConnector start/stop state machine', () => { | ||
| type BridgeStatus = 'listening' | 'connected' | 'disconnected' | 'closed'; | ||
|
|
||
| // Minimal state machine that mirrors BridgeConnector logic | ||
| class TestBridgeConnector { | ||
| status: BridgeStatus = 'closed'; | ||
| serverEndpoint?: string; | ||
| private connectLoopRunning = false; | ||
| private statusChanges: BridgeStatus[] = []; | ||
|
|
||
| constructor(serverEndpoint?: string) { | ||
| this.serverEndpoint = serverEndpoint; | ||
| } | ||
|
|
||
| getStatusHistory() { | ||
| return this.statusChanges; | ||
| } | ||
|
|
||
| private setStatus(status: BridgeStatus) { | ||
| if (this.status === status) return; | ||
| this.status = status; | ||
| this.statusChanges.push(status); | ||
| } | ||
|
|
||
| async connect(): Promise<void> { | ||
| if (this.status === 'listening' || this.status === 'connected') { | ||
| return; | ||
| } | ||
| this.setStatus('listening'); | ||
| this.connectLoopRunning = true; | ||
| } | ||
|
|
||
| async disconnect(): Promise<void> { | ||
| if (this.status === 'closed') { | ||
| return; | ||
| } | ||
| this.connectLoopRunning = false; | ||
| this.setStatus('closed'); | ||
| } | ||
|
|
||
| getStatus(): BridgeStatus { | ||
| return this.status; | ||
| } | ||
| } | ||
|
|
||
| it('should start in closed state', () => { | ||
| const connector = new TestBridgeConnector(); | ||
| expect(connector.getStatus()).toBe('closed'); | ||
| }); | ||
|
|
||
| it('should transition to listening when connect() is called', async () => { | ||
| const connector = new TestBridgeConnector(); | ||
| await connector.connect(); | ||
| expect(connector.getStatus()).toBe('listening'); | ||
| }); | ||
|
|
||
| it('should transition to closed when disconnect() is called', async () => { | ||
| const connector = new TestBridgeConnector(); | ||
| await connector.connect(); | ||
| expect(connector.getStatus()).toBe('listening'); | ||
|
|
||
| await connector.disconnect(); | ||
| expect(connector.getStatus()).toBe('closed'); | ||
| }); | ||
|
|
||
| it('should allow restart after disconnect', async () => { | ||
| const connector = new TestBridgeConnector(); | ||
|
|
||
| // First cycle | ||
| await connector.connect(); | ||
| expect(connector.getStatus()).toBe('listening'); | ||
| await connector.disconnect(); | ||
| expect(connector.getStatus()).toBe('closed'); | ||
|
|
||
| // Second cycle | ||
| await connector.connect(); | ||
| expect(connector.getStatus()).toBe('listening'); | ||
|
|
||
| expect(connector.getStatusHistory()).toEqual([ | ||
| 'listening', | ||
| 'closed', | ||
| 'listening', | ||
| ]); | ||
| }); | ||
|
|
||
| it('should allow changing server endpoint after disconnect', async () => { | ||
| const connector1 = new TestBridgeConnector('ws://server1:3766'); | ||
| await connector1.connect(); | ||
| expect(connector1.serverEndpoint).toBe('ws://server1:3766'); | ||
| await connector1.disconnect(); | ||
|
|
||
| // Create new connector with different endpoint (mirrors worker behavior) | ||
| const connector2 = new TestBridgeConnector('ws://server2:3766'); | ||
| await connector2.connect(); | ||
| expect(connector2.serverEndpoint).toBe('ws://server2:3766'); | ||
| expect(connector2.getStatus()).toBe('listening'); | ||
| }); | ||
|
|
||
| it('disconnect() should be idempotent when already closed', async () => { | ||
| const connector = new TestBridgeConnector(); | ||
| expect(connector.getStatus()).toBe('closed'); | ||
| await connector.disconnect(); // should not throw | ||
| expect(connector.getStatus()).toBe('closed'); | ||
| }); | ||
|
|
||
| it('connect() should be idempotent when already listening', async () => { | ||
| const connector = new TestBridgeConnector(); | ||
| await connector.connect(); | ||
| expect(connector.getStatus()).toBe('listening'); | ||
| await connector.connect(); // should not throw or change state | ||
| expect(connector.getStatus()).toBe('listening'); | ||
| expect(connector.getStatusHistory()).toEqual(['listening']); // only one transition | ||
| }); | ||
| }); | ||
|
|
||
| // ─── Worker message handling tests ────────────────────────────────────────── | ||
|
|
||
| describe('Worker bridge message handling', () => { | ||
| it('BRIDGE_START message should accept serverEndpoint parameter', () => { | ||
| // Simulate the worker message handler for BRIDGE_START | ||
| const request = { | ||
| type: 'bridge-start', | ||
| payload: { serverEndpoint: 'ws://remote-server:4000' }, | ||
| }; | ||
|
|
||
| const { serverEndpoint } = request.payload || {}; | ||
| expect(serverEndpoint).toBe('ws://remote-server:4000'); | ||
| }); | ||
|
|
||
| it('BRIDGE_START message should work without serverEndpoint', () => { | ||
| const request = { | ||
| type: 'bridge-start', | ||
| payload: {}, | ||
| }; | ||
|
|
||
| const { serverEndpoint } = request.payload || {}; | ||
| expect(serverEndpoint).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('BRIDGE_STOP message should have correct type', () => { | ||
| const request = { type: 'bridge-stop' }; | ||
| expect(request.type).toBe('bridge-stop'); | ||
| }); | ||
| }); | ||
|
|
||
| // ─── UI state tests ────────────────────────────────────────────────────────── | ||
|
|
||
| describe('Bridge UI state logic', () => { | ||
| it('server URL input should be enabled when status is closed', () => { | ||
| const bridgeStatus = 'closed'; | ||
| const disabled = bridgeStatus !== 'closed'; | ||
| expect(disabled).toBe(false); | ||
| }); | ||
|
|
||
| it('server URL input should be disabled when status is listening', () => { | ||
| const bridgeStatus = 'listening'; | ||
| const disabled = bridgeStatus !== 'closed'; | ||
| expect(disabled).toBe(true); | ||
| }); | ||
|
|
||
| it('server URL input should be disabled when status is connected', () => { | ||
| const bridgeStatus = 'connected'; | ||
| const disabled = bridgeStatus !== 'closed'; | ||
| expect(disabled).toBe(true); | ||
| }); | ||
|
|
||
| it('should determine correct button label based on status', () => { | ||
| // The toggle button should show "Stop" when listening/connected, | ||
| // and "Start Listening" when closed | ||
| function getButtonLabel( | ||
| status: 'listening' | 'connected' | 'disconnected' | 'closed', | ||
| ): string { | ||
| if (status === 'listening' || status === 'connected') { | ||
| return 'Stop'; | ||
| } | ||
| return 'Start Listening'; | ||
| } | ||
|
|
||
| expect(getButtonLabel('closed')).toBe('Start Listening'); | ||
| expect(getButtonLabel('listening')).toBe('Stop'); | ||
| expect(getButtonLabel('connected')).toBe('Stop'); | ||
| expect(getButtonLabel('disconnected')).toBe('Start Listening'); | ||
| }); | ||
|
|
||
| it('should determine if bridge is active for toggle state', () => { | ||
| function isBridgeActive( | ||
| status: 'listening' | 'connected' | 'disconnected' | 'closed', | ||
| ): boolean { | ||
| return status === 'listening' || status === 'connected'; | ||
| } | ||
|
|
||
| expect(isBridgeActive('closed')).toBe(false); | ||
| expect(isBridgeActive('disconnected')).toBe(false); | ||
| expect(isBridgeActive('listening')).toBe(true); | ||
| expect(isBridgeActive('connected')).toBe(true); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a bridge session drops,
BridgeConnectormoves todisconnectedand keeps retrying insideconnect()instead of stopping (apps/chrome-extension/src/utils/bridgeConnector.ts:54-102). BecauseisBridgeActiveonly treatslisteningandconnectedas active, the new control flips toStartduring that reconnect window even though the bridge is still running and the URL field stays disabled. In that state the user cannot actually stop the retry loop without first restarting the bridge, so the new start/stop control is incorrect whenever a connection is lost.Useful? React with 👍 / 👎.