From f9ad3d2fcf4dbf50e97fe60892af43643c285aa8 Mon Sep 17 00:00:00 2001 From: Oskar Nyberg Date: Tue, 16 Apr 2024 08:03:02 +0200 Subject: [PATCH 1/7] Add testid to custom bridge delete confirmation button --- gui/src/renderer/components/EditCustomBridge.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gui/src/renderer/components/EditCustomBridge.tsx b/gui/src/renderer/components/EditCustomBridge.tsx index 2692217ea0a7..7cc403aa6ed4 100644 --- a/gui/src/renderer/components/EditCustomBridge.tsx +++ b/gui/src/renderer/components/EditCustomBridge.tsx @@ -95,7 +95,11 @@ function CustomBridgeForm() { {messages.gettext('Cancel')} , - + {messages.gettext('Delete')} , ]} From bc7b8e9fc5f5776df1a2ec0520d1a26656e6c004 Mon Sep 17 00:00:00 2001 From: Oskar Nyberg Date: Tue, 16 Apr 2024 08:03:59 +0200 Subject: [PATCH 2/7] Add aria-label to edit custom bridge button --- .../select-location/SpecialLocationList.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/gui/src/renderer/components/select-location/SpecialLocationList.tsx b/gui/src/renderer/components/select-location/SpecialLocationList.tsx index 9579a6b4b511..a347638b0e28 100644 --- a/gui/src/renderer/components/select-location/SpecialLocationList.tsx +++ b/gui/src/renderer/components/select-location/SpecialLocationList.tsx @@ -108,7 +108,8 @@ export function CustomBridgeLocationRow( const history = useHistory(); const bridgeSettings = useSelector((state) => state.settings.bridgeSettings); - const icon = bridgeSettings.custom !== undefined ? 'icon-edit' : 'icon-add'; + const bridgeConfigured = bridgeSettings.custom !== undefined; + const icon = bridgeConfigured ? 'icon-edit' : 'icon-add'; const selectedRef = props.source.selected ? props.selectedElementRef : undefined; const background = getButtonColor(props.source.selected, 0, props.source.disabled); @@ -135,7 +136,14 @@ export function CustomBridgeLocationRow( 'A custom bridge server can be used to circumvent censorship when regular Mullvad bridge servers don’t work.', )} /> - + Date: Tue, 16 Apr 2024 08:04:22 +0200 Subject: [PATCH 3/7] Fix not resetting to bridge type normal when selecting automatic --- .../renderer/components/select-location/select-location-hooks.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/gui/src/renderer/components/select-location/select-location-hooks.ts b/gui/src/renderer/components/select-location/select-location-hooks.ts index 08ed2dccf3fb..027185e416a6 100644 --- a/gui/src/renderer/components/select-location/select-location-hooks.ts +++ b/gui/src/renderer/components/select-location/select-location-hooks.ts @@ -121,6 +121,7 @@ export function useOnSelectBridgeLocation() { case SpecialBridgeLocationType.closestToExit: return setLocation( bridgeSettingsModifier((bridgeSettings) => { + bridgeSettings.type = 'normal'; bridgeSettings.normal.location = 'any'; return bridgeSettings; }), From a45790318396d5f8b73a55c82d512868968fbe4b Mon Sep 17 00:00:00 2001 From: Oskar Nyberg Date: Tue, 16 Apr 2024 08:06:53 +0200 Subject: [PATCH 4/7] Add GUI test for custom bridge --- .../renderer/components/OpenVpnSettings.tsx | 3 +- gui/src/renderer/components/cell/Selector.tsx | 10 +- .../state-dependent/custom-bridge.spec.ts | 173 ++++++++++++++++++ 3 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 gui/test/e2e/installed/state-dependent/custom-bridge.spec.ts diff --git a/gui/src/renderer/components/OpenVpnSettings.tsx b/gui/src/renderer/components/OpenVpnSettings.tsx index dee1e6408968..5013b158f8eb 100644 --- a/gui/src/renderer/components/OpenVpnSettings.tsx +++ b/gui/src/renderer/components/OpenVpnSettings.tsx @@ -269,6 +269,7 @@ function BridgeModeSelector() { label: messages.gettext('On'), value: 'on', disabled: tunnelProtocol !== 'openvpn' || transportProtocol === 'udp', + 'data-testid': 'bridge-mode-on', }, { label: messages.gettext('Off'), @@ -367,7 +368,7 @@ function BridgeModeSelector() { {messages.gettext('Cancel')} , - + {messages.gettext('Enable')} , ]} diff --git a/gui/src/renderer/components/cell/Selector.tsx b/gui/src/renderer/components/cell/Selector.tsx index e527017435d3..5e5d685a59f7 100644 --- a/gui/src/renderer/components/cell/Selector.tsx +++ b/gui/src/renderer/components/cell/Selector.tsx @@ -16,6 +16,8 @@ export interface SelectorItem { label: string; value: T; disabled?: boolean; + // eslint-disable-next-line @typescript-eslint/naming-convention + 'data-testid'?: string; } // T represents the available values and U represent the value of "Automatic"/"Any" if there is one. @@ -51,7 +53,8 @@ export default function Selector(props: SelectorProps) { isSelected={selected} disabled={props.disabled || item.disabled} forwardedRef={ref} - onSelect={props.onSelect}> + onSelect={props.onSelect} + data-testid={item['data-testid']}> {item.label} ); @@ -133,6 +136,8 @@ interface SelectorCellProps { onSelect: (value: T) => void; children: React.ReactNode | Array; forwardedRef?: React.Ref; + // eslint-disable-next-line @typescript-eslint/naming-convention + 'data-testid'?: string; } function SelectorCell(props: SelectorCellProps) { @@ -150,7 +155,8 @@ function SelectorCell(props: SelectorCellProps) { disabled={props.disabled} role="option" aria-selected={props.isSelected} - aria-disabled={props.disabled}> + aria-disabled={props.disabled} + data-testid={props['data-testid']}> { + ({ page, util } = await startInstalledApp()); +}); + +test.afterAll(async () => { + await page.close(); +}); + +test('App should enable bridge mode', async () => { + await util.waitForNavigation(async () => await page.click('button[aria-label="Settings"]')); + expect( + await util.waitForNavigation(async () => await page.getByText('VPN settings').click()), + ).toBe(RoutePath.vpnSettings); + + await page.getByRole('option', { name: 'OpenVPN' }).click(); + + expect( + await util.waitForNavigation(async () => await page.getByText('OpenVPN settings').click()), + ).toBe(RoutePath.openVpnSettings); + + await page.getByTestId('bridge-mode-on').click(); + await expect(page.getByText('Enable bridge mode?')).toBeVisible(); + + page.getByTestId('enable-confirm').click(); + + await util.waitForNavigation(async () => await page.click('button[aria-label="Back"]')); + await util.waitForNavigation(async () => await page.click('button[aria-label="Back"]')); + expect( + await util.waitForNavigation(async () => await page.click('button[aria-label="Close"]')), + ).toBe(RoutePath.main); +}); + +test('App display disabled custom bridge', async () => { + expect( + await util.waitForNavigation( + async () => await page.click('button[aria-label^="Select location"]'), + ), + ).toBe(RoutePath.selectLocation); + + const title = page.locator('h1') + await expect(title).toHaveText('Select location'); + + await page.getByText(/^Entry$/).click(); + + const customBridgeButton = page.getByText('Custom bridge'); + await expect(customBridgeButton).toBeDisabled(); +}); + +test('App should add new custom bridge', async () => { + expect( + await util.waitForNavigation( + async () => await page.click('button[aria-label="Add new custom bridge"]'), + ), + ).toBe(RoutePath.editCustomBridge); + + const title = page.locator('h1') + await expect(title).toHaveText('Add custom bridge'); + + const inputs = page.locator('input'); + const addButton = page.locator('button:has-text("Add")'); + await expect(addButton).toBeVisible(); + await expect(addButton).toBeDisabled(); + + await inputs.first().fill(process.env.SHADOWSOCKS_SERVER_IP!); + await expect(addButton).toBeDisabled(); + + await inputs.nth(1).fill('443'); + await expect(addButton).toBeEnabled(); + + await inputs.nth(2).fill(process.env.SHADOWSOCKS_SERVER_PASSWORD!); + + await page.getByTestId('ciphers').click(); + await page.getByRole('option', { name: process.env.SHADOWSOCKS_SERVER_CIPHER!, exact: true }).click(); + + expect( + await util.waitForNavigation(async () => await addButton.click()) + ).toEqual(RoutePath.selectLocation); + + const customBridgeButton = page.getByText('Custom bridge'); + await expect(customBridgeButton).toBeEnabled(); + + await expect(page.locator('button[aria-label="Edit custom bridge"]')).toBeVisible(); +}); + +test('App should select custom bridge', async () => { + const customBridgeButton = page.locator('button:has-text("Custom bridge")'); + await expect(customBridgeButton).toHaveCSS('background-color', colors.green); + + const automaticButton = page.getByText('Automatic'); + await automaticButton.click(); + await page.getByText(/^Entry$/).click(); + await expect(customBridgeButton).not.toHaveCSS('background-color', colors.green); + + + await customBridgeButton.click(); + await page.getByText(/^Entry$/).click(); + await expect(customBridgeButton).toHaveCSS('background-color', colors.green); + +}); + +test('App should edit custom bridge', async () => { + const automaticButton = page.getByText('Automatic'); + await automaticButton.click(); + await page.getByText(/^Entry$/).click(); + + expect( + await util.waitForNavigation( + async () => await page.click('button[aria-label="Edit custom bridge"]'), + ), + ).toBe(RoutePath.editCustomBridge); + + const title = page.locator('h1') + await expect(title).toHaveText('Edit custom bridge'); + + const inputs = page.locator('input'); + const saveButton = page.locator('button:has-text("Save")'); + await expect(saveButton).toBeVisible(); + await expect(saveButton).toBeEnabled(); + + await inputs.nth(1).fill(process.env.SHADOWSOCKS_SERVER_PORT!); + await expect(saveButton).toBeEnabled(); + + + expect( + await util.waitForNavigation(async () => await saveButton.click()) + ).toEqual(RoutePath.selectLocation); + + const customBridgeButton = page.locator('button:has-text("Custom bridge")'); + await expect(customBridgeButton).toBeEnabled(); + await expect(customBridgeButton).toHaveCSS('background-color', colors.green); +}); + +test('App should delete custom bridge', async () => { + expect( + await util.waitForNavigation( + async () => await page.click('button[aria-label="Edit custom bridge"]'), + ), + ).toBe(RoutePath.editCustomBridge); + + const deleteButton = page.locator('button:has-text("Delete")'); + await expect(deleteButton).toBeVisible(); + await expect(deleteButton).toBeEnabled(); + + await deleteButton.click(); + await expect(page.getByText('Delete custom bridge?')).toBeVisible(); + + const confirmButton = page.getByTestId('delete-confirm'); + expect( + await util.waitForNavigation(async () => await confirmButton.click()) + ).toEqual(RoutePath.selectLocation); + + const customBridgeButton = page.locator('button:has-text("Custom bridge")'); + await expect(customBridgeButton).toBeDisabled(); + await expect(customBridgeButton).not.toHaveCSS('background-color', colors.green); +}); From 67e772a453c4df480f03e82fb75e44ced89f7679 Mon Sep 17 00:00:00 2001 From: Oskar Nyberg Date: Tue, 16 Apr 2024 09:39:23 +0200 Subject: [PATCH 5/7] Add ensure_logged_in test helper function --- test/test-manager/src/tests/helpers.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/test-manager/src/tests/helpers.rs b/test/test-manager/src/tests/helpers.rs index da50679a26b4..b733939da073 100644 --- a/test/test-manager/src/tests/helpers.rs +++ b/test/test-manager/src/tests/helpers.rs @@ -249,6 +249,21 @@ pub async fn login_with_retries( } } +/// Ensure that the test runner is logged in to an account. +/// +/// This will first check whether we are logged in. If not, it will also try to login +/// on your behalf. If this function returns without any errors, we are logged in to a valid +/// account. +pub async fn ensure_logged_in( + mullvad_client: &mut MullvadProxyClient, +) -> Result<(), mullvad_management_interface::Error> { + if mullvad_client.get_device().await?.is_logged_in() { + return Ok(()); + } + // We are apparently not logged in already.. Try to log in. + login_with_retries(mullvad_client).await +} + /// Try to connect to a Mullvad Tunnel. /// /// # Returns From 1637cdf7c3e1d3fdb087bb94e734bad2d3d8790f Mon Sep 17 00:00:00 2001 From: Oskar Nyberg Date: Tue, 16 Apr 2024 08:57:01 +0200 Subject: [PATCH 6/7] Add gui test to test framework --- test/test-manager/src/tests/ui.rs | 74 ++++++++++++++++++++++++++++--- 1 file changed, 67 insertions(+), 7 deletions(-) diff --git a/test/test-manager/src/tests/ui.rs b/test/test-manager/src/tests/ui.rs index cce7cdd990c8..7adaf432afc6 100644 --- a/test/test-manager/src/tests/ui.rs +++ b/test/test-manager/src/tests/ui.rs @@ -1,4 +1,8 @@ -use super::{config::TEST_CONFIG, helpers, Error, TestContext}; +use super::{ + config::TEST_CONFIG, + helpers::{self, ensure_logged_in}, + Error, TestContext, +}; use mullvad_management_interface::MullvadProxyClient; use mullvad_relay_selector::query::builder::RelayQueryBuilder; use std::{ @@ -125,7 +129,7 @@ pub async fn test_ui_login(_: TestContext, rpc: ServiceClient) -> Result<(), Err Ok(()) } -#[test_function(priority = 1000, must_succeed = true)] +#[test_function(priority = 1000)] async fn test_custom_access_methods_gui( _: TestContext, rpc: ServiceClient, @@ -196,13 +200,69 @@ async fn test_custom_access_methods_gui( assert!(ui_result.success()); - // Reset the `api-override` feature. - tokio::time::timeout( - std::time::Duration::from_secs(60), - rpc.set_daemon_environment(helpers::get_app_env()), + Ok(()) +} + +#[test_function(priority = 1000)] +async fn test_custom_bridge_gui( + _: TestContext, + rpc: ServiceClient, + mut mullvad_client: MullvadProxyClient, +) -> Result<(), Error> { + use mullvad_relay_selector::{RelaySelector, SelectorConfig}; + use talpid_types::net::proxy::CustomProxy; + // For this test to work, we need to supply the following env-variables: + // + // * SHADOWSOCKS_SERVER_IP + // * SHADOWSOCKS_SERVER_PORT + // * SHADOWSOCKS_SERVER_CIPHER + // * SHADOWSOCKS_SERVER_PASSWORD + // + // See `gui/test/e2e/installed/state-dependent/custom-bridge.spec.ts` + // for details. The setup should be the same as in + // `test_manager::tests::access_methods::test_shadowsocks`. + // + // # Note + // The test requires the app to already be logged in. + + ensure_logged_in(&mut mullvad_client) + .await + .expect("ensure_logged_in failed"); + + let gui_test = "custom-bridge.spec"; + let relay_list = mullvad_client.get_relay_locations().await.unwrap(); + let relay_selector = RelaySelector::from_list(SelectorConfig::default(), relay_list); + let custom_proxy = relay_selector + .get_bridge_forced() + .and_then(|proxy| match proxy { + CustomProxy::Shadowsocks(s) => Some(s), + _ => None + }) + .expect("`test_shadowsocks` needs at least one shadowsocks relay to execute. Found none in relay list."); + + let ui_result = run_test_env( + &rpc, + &[gui_test], + [ + ( + "SHADOWSOCKS_SERVER_IP", + custom_proxy.endpoint.ip().to_string().as_ref(), + ), + ( + "SHADOWSOCKS_SERVER_PORT", + custom_proxy.endpoint.port().to_string().as_ref(), + ), + ("SHADOWSOCKS_SERVER_CIPHER", custom_proxy.cipher.as_ref()), + ( + "SHADOWSOCKS_SERVER_PASSWORD", + custom_proxy.password.as_ref(), + ), + ], ) .await - .map_err(|_| Error::DaemonNotRunning)??; + .unwrap(); + + assert!(ui_result.success()); Ok(()) } From 9931f2c602ac9028bba7a05f52b86d707d77043f Mon Sep 17 00:00:00 2001 From: Oskar Nyberg Date: Tue, 16 Apr 2024 16:07:20 +0200 Subject: [PATCH 7/7] Update translations --- gui/locales/messages.pot | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/gui/locales/messages.pot b/gui/locales/messages.pot index 1976d04278ce..9126cb9c998b 100644 --- a/gui/locales/messages.pot +++ b/gui/locales/messages.pot @@ -261,6 +261,10 @@ msgctxt "accessibility" msgid "%(title)s, View loaded" msgstr "" +msgctxt "accessibility" +msgid "Add new custom bridge" +msgstr "" + msgctxt "accessibility" msgid "Close notification" msgstr "" @@ -275,6 +279,10 @@ msgctxt "accessibility" msgid "Copy account number" msgstr "" +msgctxt "accessibility" +msgid "Edit custom bridge" +msgstr "" + msgctxt "accessibility" msgid "Expand %(location)s" msgstr ""