ruma / homeserver

A Matrix homeserver written in Rust.
https://www.ruma.io/
1.08k stars 41 forks source link

Use random username in tests #154

Closed anuragsoni closed 7 years ago

anuragsoni commented 7 years ago

This is a first pass at using randomly generated user id's in tests

closes #121

anuragsoni commented 7 years ago

@mujx @jimmycuadra I haven't changed all occurrences of user_id's in tests yet. Updates will be required in all tests and I figured i'd do that if the approach that i've take here is agreeable to you guys.

anuragsoni commented 7 years ago

updated by removing create_access_token, create_access_token_with_username, and by returning the new user struct (by using the new create_user method) in create_fixtures instead of returning just a token.

mujx commented 7 years ago

Turns out there is already a random id generator in ruma-identifiers so we can use that. It is used in registration if a username isn't provided. The other option is to omit the username field and get a random one back.

anuragsoni commented 7 years ago

@mujx Ah, I hadn't seen ruma-identifiers. It looks to be the same thing that I had done in this PR so I replace my function with a call to the one from ruma-identifiers. As for the username, I find it nicer to have in the TestUser. I was generating the username randomly, but there are tests where we need both the username and the id. This field helps for that.

mujx commented 7 years ago

Yes we need to keep it inside TestUser. The registration endpoint will return a random username. So you can do json().find('username') to get it, if you want. Here is the response.

anuragsoni commented 7 years ago

that returns only the user_id?

#[derive(Debug, Serialize)]
struct RegistrationResponse {
    pub access_token: String,
    pub home_server: String,
    pub user_id: String,
}
mujx commented 7 years ago

That will return the fully qualified Matrix ID, ie @zikuoafjyyrh:ruma.test

anuragsoni commented 7 years ago

exactly. My point being that we need the id => @zikuoafjyyrh:ruma.test and the username: zikuoafjyyrh. If I register without the username, I'd need to access the random username that ruma_identifier creates. But I do not see it being returned in the RegistrationResponse struct? Maybe i'm missing something?

mujx commented 7 years ago

There is a helper method for UserId to get the localpart that you are reffering to. You can parse the returned UserId and call user_id.localpart() to the get the username you want. Another way to go is to store the id as UserId instead of String in the TestUser and call alice.localpart() in the tests.

jimmycuadra commented 7 years ago

This should be easier now with #158 merged!

anuragsoni commented 7 years ago

Updated to use the UserId from ruma_identifier, and squashed commits for cleanup

jimmycuadra commented 7 years ago

One minor thing to fix that I just noticed: We organize imports into three sections, separated by blank lines: 1) stdlib imports 2) external crate imports 3) internal imports. The two imports added to src/test.rs should be resorted accordingly. Go ahead and squash the commits again after that and I'll merge this. Thanks very much for doing this work—that was a lot of little things to change!

anuragsoni commented 7 years ago

@jimmycuadra I will keep that in mind for any change I work on in future! Updated with the imports re-ordered.

jimmycuadra commented 7 years ago

We'll probably add some minor style guide information to CONTRIBUTING.md soon, and eventually rustfmt will fix all of this nonsense.