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

Feat/suite desktop core/convert unaquired device #16603

Merged
merged 4 commits into from
Jan 27, 2025

Conversation

Vere-Grey
Copy link
Contributor

@Vere-Grey Vere-Grey commented Jan 24, 2025

Description

Converts to PW test file unaquired-device

Renamed to multiple-sessions as I added a test that works with two tabs (something that playwright can do :) ). Feel free to challenge the renaming.
Adds bunch of verification. We found out an issue with Stew 👍
Splits test to view only enabled and disabled

Related Issue

Resolve 15606

Screenshots:

Desktop: https://app.currents.dev/instance/OVrwYg0tq2P9Ps6D
Web: https://app.currents.dev/instance/44AQxvrBBBBTzwnx

rename
refactor locator @deviceStatus and few cy tests
split test to viewOnly enabled and false
@Vere-Grey Vere-Grey self-assigned this Jan 24, 2025
Copy link

github-actions bot commented Jan 24, 2025

🚀 Expo preview is ready!

  • Project → trezor-suite-preview
  • Platforms → android, ios
  • Scheme → trezorsuitelite
  • Runtime Version → 23
  • More info

Learn more about 𝝠 Expo Github Action

@Vere-Grey Vere-Grey force-pushed the feat/suite-desktop-core/convert-unaquired-device branch from 81ca301 to 4df18e2 Compare January 24, 2025 21:33
fixes locators, leaves the original one in place until we get rid of cy
@Vere-Grey Vere-Grey force-pushed the feat/suite-desktop-core/convert-unaquired-device branch from 4df18e2 to d243c6a Compare January 27, 2025 08:06
@@ -6,7 +6,7 @@ import { step } from '../common';

export class DevicePromptActions {
readonly confirmOnDevicePrompt: Locator;
private readonly connectDevicePrompt: Locator;
readonly connectDevicePrompt: Locator;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to expect the text in the locator on test level

Comment on lines +8 to +16
await test.step('Steal Bridge session', async () => {
const bridge = new BridgeTransport({ messages, id: 'foo-bar' });
await bridge.init();
const enumerateRes = await bridge.enumerate();
if (!enumerateRes.success) return null;
await bridge.acquire({
input: { path: enumerateRes.payload[0].path, previous: null },
});
});
Copy link
Contributor Author

@Vere-Grey Vere-Grey Jan 27, 2025

Choose a reason for hiding this comment

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

copy pasta from cypress suite

@Vere-Grey Vere-Grey force-pushed the feat/suite-desktop-core/convert-unaquired-device branch from 77f5e6f to a7b9e6d Compare January 27, 2025 08:58
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once we convert all cypress test using these locators, we will simplify that to one statically named locator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once we convert all cypress test using these locators, we will simplify that to one statically named locator. It is not good practice to build your expects around internal state and values of the UI. That way the internal state can be correct but the information shown to user can be still incorrect and test falsely passes.

@Vere-Grey Vere-Grey requested review from mroz22 and STew790 and removed request for bosomt January 27, 2025 09:10

test(
'Overtake session by opening suite new tab',
{ tag: ['@webOnly'] },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mixing electron and web would complicate our CI setup. I don't think it is worth the value. Let me know what you think.
And I assume two desktop apps running on one system is not a thing.

@HajekOndrej HajekOndrej merged commit ba58aa5 into develop Jan 27, 2025
78 checks passed
@HajekOndrej HajekOndrej deleted the feat/suite-desktop-core/convert-unaquired-device branch January 27, 2025 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants