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

refactor: Move statesync_test canister to Rust CDK #2959

Merged
merged 8 commits into from
Dec 10, 2024

Conversation

dsarlis
Copy link
Member

@dsarlis dsarlis commented Dec 4, 2024

dfn_core is considered deprecated and the plan is to eventually remove it. Therefore, we need to move any canisters in the ic repo that still depend on it to instead use the public CDK. This PR migrates the statesync_test canister.

@dsarlis dsarlis requested a review from a team as a code owner December 4, 2024 09:10
@dsarlis dsarlis added this pull request to the merge queue Dec 5, 2024
@dsarlis dsarlis removed this pull request from the merge queue due to a manual request Dec 5, 2024
@derlerd-dfinity
Copy link
Contributor

Given that we ran into quite some trouble with the XNet test canister port, would it be possible to trigger all the tests that typically also run on an RC on this one?

@dsarlis
Copy link
Member Author

dsarlis commented Dec 6, 2024

Given that we ran into quite some trouble with the XNet test canister port, would it be possible to trigger all the tests that typically also run on an RC on this one?

I have asked the team to let me know what I need to run. I have confirmed that both rejoin_test_large_state and state_sync_malicious_chunk_test are successful on the latest commit. If there are more I need to run, let me know.

@derlerd-dfinity
Copy link
Contributor

I have asked the team to let me know what I need to run. I have confirmed that both rejoin_test_large_state and state_sync_malicious_chunk_test are successful on the latest commit. If there are more I need to run, let me know.

Please trigger the pipelines that usually run on an RC. My concern is that -- similar to last time -- running selective tests will make us miss some subtle connections.

@dsarlis dsarlis requested review from a team as code owners December 9, 2024 08:28
@dsarlis dsarlis added CI_ALL_BAZEL_TARGETS Runs all bazel targets and uploads them to S3 and removed CI_ALL_BAZEL_TARGETS Runs all bazel targets and uploads them to S3 labels Dec 9, 2024
Copy link
Contributor

@ShuoWangNSL ShuoWangNSL left a comment

Choose a reason for hiding this comment

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

Thanks. The refactoring looks good to me.

@dsarlis
Copy link
Member Author

dsarlis commented Dec 10, 2024

Release testing pipeline is green on latest commit.

cc @derlerd-dfinity

@derlerd-dfinity
Copy link
Contributor

Release testing pipeline is green on latest commit.

cc @derlerd-dfinity

Thanks a lot @dsarlis for running these tests! Given that Shuo has already approved the code changes feel free to go ahead with merging right away.

@dsarlis dsarlis added this pull request to the merge queue Dec 10, 2024
Merged via the queue into master with commit a3df71e Dec 10, 2024
45 checks passed
@dsarlis dsarlis deleted the dimitris/state-sync-test branch December 10, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants