feat(upgrader): support restoring snapshots in disaster recovery via upgrader#526
Conversation
| snapshot_id: snapshot.snapshot_id, | ||
| sender_canister_version: Some(canister_version()), | ||
| }; | ||
| load_canister_snapshot(snapshot_args) |
There was a problem hiding this comment.
Please test this path in unit tests. Since calling directly is not testable, consider adding this method to the InstallCanister trait, and maybe rename it to ManageCanister, and call it through that trait.
There was a problem hiding this comment.
I have integration tests for this code path. Are they not enough?
There was a problem hiding this comment.
Well in this case there is indeed not a lot to test, it's mainly to get full coverage and to continue the existing pattern. Deferrig to your judgement.
There was a problem hiding this comment.
My reasoning was that the existing pattern allows unit testing general disaster recovery properties (using install_code as the actual recovery operation) and the new integration tests would provide coverage of the new code path (snapshots as the actual recovery operation) specifically.
mraszyk
merged commit 4bba380
into
mraszyk/disaster-recovery-via-snapshots
2 participants
No description provided.