Skip to content

Commit

Permalink
Merge branch 'elias/pic-bugfixes' into 'master'
Browse files Browse the repository at this point in the history
fix(PocketIC): Fix bugs related to PocketIC time and `canister_exists()`

- Fixed a bug where `canister_exists()` would return wrong results
- Fixed a bug related to `PocketIc`s internal time being set to the current time, which lead to non-deterministic behavior
- Cycles consumption is now more appropriately scaled according to the size of the subnet
- When the PocketIC binary is not found, the error now points to the PocketIC repo instead of the download link 

See merge request dfinity-lab/public/ic!16292
  • Loading branch information
fxgst committed Nov 23, 2023
2 parents a53a2fa + 57c7768 commit 69e1408
Show file tree
Hide file tree
Showing 13 changed files with 176 additions and 150 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/pocket-ic/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ rust_library(
name = "pocket-ic",
srcs = glob(["src/**"]),
proc_macro_deps = MACRO_DEPENDENCIES,
version = "2.0.0",
version = "2.0.1",
deps = DEPENDENCIES,
)

Expand Down
11 changes: 9 additions & 2 deletions packages/pocket-ic/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,17 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleases

## 2.0.1 - 2023-11-23

### Added
- Support cycle scaling according to subnet size.
- Support for PocketIC server version 2.0.1


### Changed
- When the PocketIC binary is not found, the error now points to the PocketIC repo instead of the download link



## 2.0.0 - 2023-11-21

Expand Down
2 changes: 1 addition & 1 deletion packages/pocket-ic/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "pocket-ic"
version = "2.0.0"
version = "2.0.1"
license = "Apache-2.0"
description = "PocketIC: A Canister Smart Contract Testing Platform"
repository = "https://github.com/dfinity/ic"
Expand Down
50 changes: 17 additions & 33 deletions packages/pocket-ic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
use crate::common::rest::{
ApiResponse, BlobCompression, BlobId, CreateInstanceResponse, InstanceId, RawAddCycles,
RawCanisterCall, RawCanisterId, RawCanisterResult, RawCycles, RawSetStableMemory,
RawStableMemory, RawTime, RawWasmResult, SubnetKind,
RawStableMemory, RawTime, RawWasmResult,
};
use candid::{
decode_args, encode_args,
Expand Down Expand Up @@ -445,36 +445,20 @@ impl PocketIc {
}

/// Creates a canister with a specific canister ID and optional custom settings.
/// The canister ID must be contained in the Bitcoin, Fiduciary, II, SNS or NNS subnet.
/// Panics if the canister ID is not contained in any of the subnets, of if it is already in use.
/// Returns an error if the canister ID is already in use.
/// Panics if the canister ID is not contained in any of the subnets.
///
/// The canister ID must be contained in the Bitcoin, Fiduciary, II, SNS or NNS
/// subnet range, it is not intended to be used on regular app or system subnets,
/// where it can lead to conflicts on which the function panics.
#[instrument(ret, skip(self), fields(instance_id=self.instance_id, sender = %sender.unwrap_or(Principal::anonymous()).to_string(), settings = ?settings, canister_id = %canister_id.to_string()))]
pub fn create_canister_with_id(
&self,
sender: Option<Principal>,
settings: Option<CanisterSettings>,
canister_id: CanisterId,
) -> Result<CanisterId, String> {
use SubnetKind::{Application, System};

let subnet_id = self.get_subnet(canister_id);
if subnet_id.is_none() {
return Err(format!(
"Canister ID {} is not contained in any subnet",
canister_id
));
}
if let Some(subnet_id) = subnet_id {
let config = self.topology.0.get(&subnet_id).unwrap();
if config.subnet_kind == Application || config.subnet_kind == System {
return Err(
"Create canister with ID is only supported for Bitcoin, Fiduciary, II, SNS and NNS subnets".into()
);
}
}

let CanisterIdRecord {
canister_id: actual_canister_id,
} = call_candid_as(
let res = call_candid_as(
self,
Principal::management_canister(),
RawEffectivePrincipal::CanisterId(canister_id.as_slice().to_vec()),
Expand All @@ -485,9 +469,13 @@ impl PocketIc {
specified_id: Some(canister_id),
},),
)
.map(|(x,)| x)
.unwrap();
Ok(actual_canister_id)
.map(|(x,)| x);
match res {
Ok(CanisterIdRecord {
canister_id: actual_canister_id,
}) => Ok(actual_canister_id),
Err(e) => Err(format!("{:?}", e)),
}
}

/// Create a canister on a specific subnet with optional custom settings.
Expand Down Expand Up @@ -1084,12 +1072,8 @@ Could not find the PocketIC binary.
The PocketIC binary could not be found at {:?}. Please specify the path to the binary with the POCKET_IC_BIN environment variable, \
or place it in your current working directory (you are running PocketIC from {:?}).
Run the following commands to get the binary:
curl -sLO https://download.dfinity.systems/ic/29ec86dc9f9ca4691d4d4386c8b2aa41e14d9d16/openssl-static-binaries/x86_64-$platform/pocket-ic.gz
gzip -d pocket-ic.gz
chmod +x pocket-ic
where $platform is 'linux' for Linux and 'darwin' for Intel/rosetta-enabled Darwin.
", &bin_path, &std::env::current_dir().map(|x| x.display().to_string()).unwrap_or_else(|_| "an unknown directory".to_string()));
To download the binary, please visit https://github.com/dfinity/pocketic."
, &bin_path, &std::env::current_dir().map(|x| x.display().to_string()).unwrap_or_else(|_| "an unknown directory".to_string()));
}

// Use the parent process ID to find the PocketIC server port for this `cargo test` run.
Expand Down
48 changes: 19 additions & 29 deletions packages/pocket-ic/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ fn test_create_canister_with_id() {
}

#[test]
fn test_create_canister_after_create_canister_with_id_occupied_next_canister_id() {
fn test_create_canister_after_create_canister_with_id() {
let pic = PocketIcBuilder::new().with_nns_subnet().build();

let canister_id = Principal::from_text("rwlgt-iiaaa-aaaaa-aaaaa-cai").unwrap();
Expand All @@ -205,35 +205,20 @@ fn test_create_canister_after_create_canister_with_id_occupied_next_canister_id(
}

#[test]
#[should_panic(expected = "only supported for Bitcoin, Fiduciary, II, SNS and NNS subnets")]
fn test_create_canister_with_id_on_app_subnet_fails() {
let pic = PocketIc::new();

let valid_canister_id = pic.create_canister();
let _ = pic
.create_canister_with_id(None, None, valid_canister_id)
.unwrap();
}

#[test]
#[should_panic(expected = "only supported for Bitcoin, Fiduciary, II, SNS and NNS subnets")]
fn test_create_canister_with_id_on_system_subnet_fails() {
let pic = PocketIcBuilder::new().with_system_subnet().build();

let valid_canister_id = pic.create_canister();
let _ = pic
.create_canister_with_id(None, None, valid_canister_id)
.unwrap();
fn test_create_canister_with_used_id_fails() {
let pic = PocketIcBuilder::new().with_nns_subnet().build();
let canister_id = Principal::from_text("rwlgt-iiaaa-aaaaa-aaaaa-cai").unwrap();
let res = pic.create_canister_with_id(None, None, canister_id);
assert!(res.is_ok());
let res = pic.create_canister_with_id(None, None, canister_id);
assert!(res.is_err());
}

#[test]
#[should_panic(expected = "CanisterAlreadyInstalled")]
fn test_create_canister_with_used_id_should_panic() {
let pic = PocketIcBuilder::new().with_nns_subnet().build();

let canister_id = Principal::from_text("rwlgt-iiaaa-aaaaa-aaaaa-cai").unwrap();
let _ = pic.create_canister_with_id(None, None, canister_id);
let _ = pic.create_canister_with_id(None, None, canister_id);
#[should_panic(expected = "not contained on any subnet")]
fn test_create_canister_with_not_contained_id_panics() {
let pic = PocketIc::new();
let _ = pic.create_canister_with_id(None, None, Principal::anonymous());
}

#[test]
Expand Down Expand Up @@ -571,9 +556,14 @@ fn test_new_pocket_ic_without_subnets_panics() {
fn test_canister_exists() {
let pic = PocketIc::new();
let canister_id = pic.create_canister();
pic.add_cycles(canister_id, INIT_CYCLES);
assert!(pic.canister_exists(canister_id));
let nonexistent_canister_id = Principal::anonymous();
assert!(!pic.canister_exists(nonexistent_canister_id));
pic.stop_canister(canister_id, None).unwrap();
pic.delete_canister(canister_id, None).unwrap();
assert!(!pic.canister_exists(canister_id));

let pic = PocketIc::new();
assert!(!pic.canister_exists(canister_id));
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion rs/pocket_ic_server/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ rust_library(
]),
crate_name = "pocket_ic_server",
proc_macro_deps = MACRO_DEPENDENCIES,
version = "2.0.0",
version = "2.0.1",
deps = LIB_DEPENDENCIES,
)

Expand Down
2 changes: 1 addition & 1 deletion rs/pocket_ic_server/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "pocket-ic-server"
version = "2.0.0"
version = "2.0.1"
edition = "2021"

[dependencies]
Expand Down
5 changes: 2 additions & 3 deletions rs/pocket_ic_server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,11 @@ mod tests {

match res {
UpdateReply::Output(OpOut::CanisterResult(Ok(WasmResult::Reply(bytes)))) => {
println!("wasm result bytes {:?}", bytes);
let (CanisterIdRecord { canister_id },) = decode_args(&bytes).unwrap();
println!("result: {}", canister_id);
assert!(!canister_id.to_text().is_empty());
}
UpdateReply::Output(OpOut::CanisterResult(Ok(WasmResult::Reject(x)))) => {
println!("wasm reject {:?}", x);
panic!("unexpected reject: {:?}", x);
}
e => {
panic!("unexpected result: {:?}", e);
Expand Down
2 changes: 1 addition & 1 deletion rs/pocket_ic_server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const LOG_DIR_LEVELS_ENV_NAME: &str = "POCKET_IC_LOG_DIR_LEVELS";

/// The PocketIC server hosts and manages IC instances.
#[derive(Parser)]
#[clap(version = "2.0.0")]
#[clap(version = "2.0.1")]
struct Args {
/// If you use PocketIC from the command line, you should not use this flag.
/// Client libraries use this flag to provide a common identifier (the process ID of the test
Expand Down
Loading

0 comments on commit 69e1408

Please sign in to comment.