Skip to content

test(encrypted maps): CRP-2839 copy and adapt Rust unit tests to canister tests that are also used to test the Motoko canister #192

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.

Sign up for GitHub

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

Jump to bottom
Merged
altkdf merged 6 commits into from
Jul 8, 2025
Merged

test(encrypted maps): CRP-2839 copy and adapt Rust unit tests to canister tests that are also used to test the Motoko canister #192

altkdf merged 6 commits into from
Jul 8, 2025

Conversation

@altkdf
Copy link
Contributor

@altkdf altkdf commented Jul 7, 2025

No description provided.

altkdf added 4 commits July 4, 2025 18:30
@altkdf
copy tests with a lot of effort adapting them
c3221e9 
in reality, Cursor did all the work and I will get the credits \o/
I tried to check though that everything is correct and it looks valid and the tests for the rust canister do pass
@altkdf
fmt
55c9386
@altkdf
fix error message
32a6a92
@altkdf
...
c2b9b7b
@altkdf
Copy link
Contributor Author

altkdf commented Jul 7, 2025

This includes changes from #191, which will disappear once that PR is merged and this one is rebased.

@altkdf altkdf marked this pull request as ready for review July 7, 2025 15:12
@altkdf altkdf requested a review from a team as a code owner July 7, 2025 15:12
fspreiss
fspreiss reviewed Jul 8, 2025
View reviewed changes
Copy link
Contributor

@fspreiss fspreiss left a comment

Choose a reason for hiding this comment

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

Thanks, @altkdf! Here are a few minor comments.

@altkdf
Copy link
Contributor Author

altkdf commented Jul 8, 2025

Thanks for the improvement suggestions @fspreiss ! In this PR I basically ported the existing unit tests to canister tests. But I believe having the unit tests still makes sense as well as long as we keep both in sync s.t. we can also assess the coverage (if that's not too much overhead). If we want to have it in that way, we would need change incorporate your suggestions in both files. If we decide to do that, I would prefer to apply your suggestions in another PR to keep this isolated. WDYT?

@fspreiss
Copy link
Contributor

fspreiss commented Jul 8, 2025

If we want to have it in that way, we would need change incorporate your suggestions in both files. If we decide to do that, I would prefer to apply your suggestions in another PR to keep this isolated. WDYT?

A separate PR that does it for both tests sounds good to me!

@altkdf altkdf merged commit 1c3abd8 into main Jul 8, 2025
36 checks passed
@altkdf altkdf deleted the alex/encrypted-maps-tests branch July 8, 2025 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fspreiss fspreiss fspreiss approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@altkdf @fspreiss