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

Fix test that will break with matrix-sdk-crypto-wasm 13.0.0 #4635

Merged
merged 2 commits into from
Jan 24, 2025

Conversation

uhoreg
Copy link
Member

@uhoreg uhoreg commented Jan 21, 2025

Remove use of a constructor that never should have been there.
Fixes #4634

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.
  • Sign-off given on the changes (see CONTRIBUTING.md).

@uhoreg
Copy link
Member Author

uhoreg commented Jan 21, 2025

The test runs correctly when I run only the pinCurrentIdentity tests, but when all the spec/unit/rust-crypto/rust-crypto.spec.ts tests are run, it times out for some reason. So I'll have to poke at it some more.

"Cannot pin identity of unknown user",
);
});

it("throws an error for our own user", async () => {
const ownIdentity = new RustSdkCryptoJs.OwnUserIdentity();
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it more simple to just do

 const ownIdentity = { } as unknown as Mocked<RustSdkCryptoJs.OwnUserIdentity>;

Instead of bringing all the key receiver/responder framework?

I added similar tests in this PR https://github.com/matrix-org/matrix-js-sdk/pull/4646/files

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that pinCurrentUserIdentity uses instanceof to check whether it's an OwnUserIdentity, so it needs a real OwnUserIdentity object.

@uhoreg uhoreg requested a review from BillCarsonFr January 23, 2025 13:30
Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@uhoreg uhoreg dismissed BillCarsonFr’s stale review January 24, 2025 16:47

responded to foodback

@uhoreg uhoreg added this pull request to the merge queue Jan 24, 2025
Merged via the queue into matrix-org:develop with commit df49280 Jan 24, 2025
31 checks passed
@uhoreg uhoreg deleted the fix_test_for_wasm_binding_13 branch January 24, 2025 17:08
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.

Constructor for OwnUserIdentity no longer accessible
3 participants