Move test helper to make it useable in other tests #1762
Conversation
Creating new identities based off authn_methods will be requiered in almost all v2 API integration tests. This PR moves it into a helper file where it can be used from any other test.
| pub fn create_identity_with_authn_method( | ||
| env: &StateMachine, | ||
| canister_id: CanisterId, | ||
| authn_method: &AuthnMethodData, |
There was a problem hiding this comment.
Why is this now passed by reference while before it wasn't?
There was a problem hiding this comment.
Having the function only borrow the argument allows tests to use it multiple times. This moves the call to clone into the function making the tests less cluttered.
There was a problem hiding this comment.
None of the tests seem to be using the argument multiple times, nor calling clone. Am I missing something?
Is this change related to the moving the helper or orthogonal?
Is it needed/used by your follow-up/main PR?
There was a problem hiding this comment.
None of the tests seem to be using the argument multiple times, nor calling clone. Am I missing something?
I think so? There is a line diff in src/internet_identity/tests/integration/v2_api/authn_method_add.rs that changes
let identity_number = create_identity_with_authn_method(&env, canister_id, authn_method.clone());
to
let identity_number = create_identity_with_authn_method(&env, canister_id, &authn_method);
Similarly in the upcoming changeset.
Is this change related to the moving the helper or orthogonal?
It does not directly have anything to do with moving, but goes in the same vein as it also makes the helper more useful (imho).
There was a problem hiding this comment.
Ah, yes, I missed it.
It does mean that clone is now called more often than before, right? Does that go against the spirit or Rust? Anyway it's fine.
There seems to be some flakyness regarding the screenshots. I will talk to @nmattia about that, since there have been a lot of changes to that infrastructure since I went on PTO. |
| pub fn create_identity_with_authn_method( | ||
| env: &StateMachine, | ||
| canister_id: CanisterId, | ||
| authn_method: &AuthnMethodData, |
There was a problem hiding this comment.
Ah, yes, I missed it.
It does mean that clone is now called more often than before, right? Does that go against the spirit or Rust? Anyway it's fine.
Potentially yes. However, in most cases |
Creating new identities based off authn_methods will be requiered in almost all v2 API integration tests. This PR moves it into a helper file where it can be used from any other test.
🟡 Some screens were changed