Refactor config integration tests. #2785
Conversation
…ation-tests' into sea-snake/refactor-config-integration-tests
…ation-tests' into sea-snake/refactor-config-integration-tests
| assert_eq!( | ||
| api::config(env, canister_id)?, | ||
| InternetIdentityInit { | ||
| openid_google: expected_config.openid_google.clone().or(Some(None)), |
There was a problem hiding this comment.
Why is this one necessary like this?
| } | ||
|
|
||
| /// Utility method to upgrade with other config change and check if any other config hasn't changed | ||
| fn unrelated_change_and_assert( |
There was a problem hiding this comment.
unrelated_change to what? To another config? Which other config? One with optionals?
| )?; | ||
| assert_config(env, canister_id, &expected_config)?; | ||
| Ok(canister_id) | ||
| } | ||
|
|
||
| #[test] | ||
| fn should_retain_anchor_on_user_range_change() -> Result<(), CallError> { |
There was a problem hiding this comment.
Wasn't there something missing in this test? which Anchor is retained?
|
|
||
| assert_eq!(api::config(&env, canister_id)?, config); | ||
| let canister_id = install_and_assert(&env, &config)?; | ||
| upgrade_ii_canister_with_arg(&env, canister_id, II_WASM.clone(), Some(config.clone()))?; |
There was a problem hiding this comment.
Shouldn't this upgrade using None?
|
|
||
| assert_eq!(api::config(&env, canister_id)?, config); | ||
| // Check if related origins are disabled and then untouched | ||
| let canister_id_2 = install_and_assert(&env, &config_1)?; |
There was a problem hiding this comment.
Why are there different canister ids? Shouldn't they all be the same?
| assert_eq!(api::config(&env, canister_id)?, config); | ||
| // Check if related origins are disabled and then untouched | ||
| let canister_id_2 = install_and_assert(&env, &config_1)?; | ||
| upgrade_and_assert(&env, canister_id_2, &disabled_config, &disabled_config)?; |
There was a problem hiding this comment.
Wouldn't it be better if the confi passed to upgrade_and_assert had mostly None fields except for the changes we are interested and then we expect the fields to be present from before?
If config logic substituted a None for some value, we wouldn't catch it here, right?
| }; | ||
| // Check if open id config is disabled and then untouched | ||
| let canister_id_2 = install_and_assert(&env, &config_1)?; | ||
| upgrade_and_assert(&env, canister_id_2, &disabled_config, &disabled_config)?; |
Refactor config integration tests to clarify what cases are covered, utility methods are implemented for repetitive checks e.g. assert config with config returned from query.
🟡 Some screens were changed