e2e test: Check key backup with js-sdk api instead of relying of Security & Privacy tab
#29066
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…up with the matrix client and the crypto api instead of relying of the `Security & Privacy` tab.
florianduros
force-pushed
the
florianduros/check-key-backup-e2e-test
branch
from
cb0d855 to
44c6d1d
Compare
florianduros
commented
Jan 22, 2025
| @@ -68,8 +68,8 @@ test.describe("Device verification", { tag: "@no-webkit" }, () => { | |||
|
|
|||
| // 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); | |||
There was a problem hiding this comment.
This is still not working
There was a problem hiding this comment.
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)
dbkr
approved these changes
Jan 22, 2025
BillCarsonFr
requested changes
Jan 24, 2025
playwright/e2e/crypto/utils.ts
Outdated
| @@ -139,12 +139,12 @@ export async function checkDeviceIsCrossSigned(app: ElementAppPage): Promise<voi | |||
| * Check that the current device is connected to the expected key backup. | |||
| * Also checks that the decryption key is known and cached locally. | |||
| * | |||
| * @param page - the page to check | |||
| * @param app - app page | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
- 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)
- The UI will be removed so we need to find another way
richvdh
reviewed
Jan 24, 2025
BillCarsonFr
approved these changes
Jan 24, 2025
florianduros
force-pushed
the
florianduros/check-key-backup-e2e-test
branch
from
944f661 to
6487612
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
public/exportedsymbols have accurate TSDoc documentation.In #26468, the key backup section of the
Security & Privacytab 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.