Skip to content

e2e test: Check key backup with js-sdk api instead of relying of Security & Privacy tab#29066

Merged
florianduros merged 2 commits into
developfrom
florianduros/check-key-backup-e2e-test
Jan 24, 2025
Merged

e2e test: Check key backup with js-sdk api instead of relying of Security & Privacy tab#29066
florianduros merged 2 commits into
developfrom
florianduros/check-key-backup-e2e-test

Conversation

@florianduros
Copy link
Copy Markdown
Member

@florianduros florianduros commented Jan 22, 2025

Checklist

  • Tests written for new code (and old code if feasible).
  • New or updated public/exported symbols have accurate TSDoc documentation.
  • Linter and other CI checks pass.
  • I have licensed the changes to Element by completing the Contributor License Agreement (CLA)

In #26468, the key backup section of the Security & Privacy tab will be removed.

To check if the key backup is in the expected state, we can directly use the matrix client and the crypto api.

…up with the matrix client and the crypto api instead of relying of the `Security & Privacy` tab.
// Check that the current device is connected to key backup
// For now we don't check that the backup key is in cache because it's a bit flaky,
// as we need to wait for the secret gossiping to happen and the settings dialog doesn't refresh automatically.
await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion, false);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is still not working

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just this test isn't working?

Copy link
Copy Markdown
Member Author

@florianduros florianduros Jan 22, 2025

Choose a reason for hiding this comment

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

The others are working if we check that the key backup is in cache but this one is failing (so I kept the false attribute)

@florianduros florianduros marked this pull request as ready for review January 22, 2025 13:15
@florianduros florianduros requested review from a team as code owners January 22, 2025 13:15
Comment thread playwright/e2e/crypto/utils.ts Outdated
* Also checks that the decryption key is known and cached locally.
*
* @param page - the page to check
* @param app - app page
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So that means that the check on the page is not up-to-date, but the check using the API is ok?
Doesn't that mean that there is a refresh bug on the UI?

Copy link
Copy Markdown
Member Author

@florianduros florianduros Jan 24, 2025

Choose a reason for hiding this comment

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

This UI section will be removed in #26468. The e2e tests were using this section to check if the key storage was in the expected state.

  1. It was not a good practice, we should not look at the ui for this kind of verification (we are not testing this part of the ui)
  2. The UI will be removed so we need to find another way

Copy link
Copy Markdown
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm in principle. A few cleanups

Comment thread playwright/e2e/crypto/utils.ts Outdated
Comment thread playwright/e2e/crypto/utils.ts Outdated
Comment thread playwright/e2e/crypto/utils.ts Outdated
Comment thread playwright/e2e/crypto/utils.ts Outdated
Comment thread playwright/e2e/crypto/utils.ts Outdated
Comment thread playwright/e2e/crypto/utils.ts Outdated
@florianduros florianduros force-pushed the florianduros/check-key-backup-e2e-test branch from 944f661 to 6487612 Compare January 24, 2025 12:25
@florianduros florianduros added this pull request to the merge queue Jan 24, 2025
Merged via the queue into develop with commit 850c1a5 Jan 24, 2025
@florianduros florianduros deleted the florianduros/check-key-backup-e2e-test branch January 24, 2025 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-Task Tasks for the team like planning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants