Skip to content

feat(PocketIC): support for specifying subnet cost schedule#9419

Open
mraszyk wants to merge 2 commits intomasterfrom
mraszyk/pic-subnet-cost-schedule
Open

feat(PocketIC): support for specifying subnet cost schedule#9419
mraszyk wants to merge 2 commits intomasterfrom
mraszyk/pic-subnet-cost-schedule

Conversation

@mraszyk
Copy link
Contributor

@mraszyk mraszyk commented Mar 17, 2026

This PR adds support for specifying subnet cost schedule in PocketIC.

Note: backward-compatibility is ensured by using #[serde(default)] (commit).

@github-actions github-actions bot added the feat label Mar 17, 2026
@mraszyk mraszyk marked this pull request as ready for review March 17, 2026 11:11
@mraszyk mraszyk requested a review from a team as a code owner March 17, 2026 11:11
Comment on lines +3321 to +3342
async fn rental_subnet_with_subnet_admins() {
// Create a PocketIC instance with a single rental subnet.
let admin = Principal::anonymous();
let subnet_spec = SubnetSpec::default()
.with_subnet_admins(vec![admin])
.with_cost_schedule(CanisterCyclesCostSchedule::Free);
let config = ExtendedSubnetConfigSet {
application: vec![subnet_spec],
..Default::default()
};
let mut pic = PocketIcBuilder::new_with_config(config).build_async().await;

// Derive the rental subnet's subnet ID.
let topology = pic.topology().await;
let rental_subnet = topology.get_app_subnets()[0];

// Run the test.
rental_subnet_or_cloud_engine_test(&mut pic, rental_subnet).await;
}

#[tokio::test]
async fn cloud_engine_with_subnet_admins() {
Copy link
Contributor

Choose a reason for hiding this comment

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

These two tests still are almost entirely duplicated. Maybe rental_subnet_or_cloud_engine_test can take a closure SubnetSpec -> ExtendedSubnetConfigSet or something and have that be the only difference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reusable test function would also need to take a closure Topology -> Principal abstracting over

    let rental_subnet = topology.get_app_subnets()[0];

and

    let cloud_engine = topology.get_cloud_engines()[0];

respectively (we should test that the PocketIC builder created the appropriate subnet kind and thus enumerating all subnets and taking the first one would reduce test coverage).

If we want to make the test more compact, I'd suggest to introduce a private enum:

enum FreeSubnet {
  RentalSubnet,
  CloudEngine,
}

and do stuff like

    let subnet_id = match free_subnet {
      FreeSubnet::RentalSubnet => topology.get_app_subnets()[0],
      FreeSubnet::CloudEngine => topology.get_cloud_engines()[0],
    };

What do you think?

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.

2 participants