Convert more tests #1514
Convert more tests #1514
Conversation
| assert_eq!(anon.search("q=foo").meta.total, 2); | ||
| assert_eq!(anon.search("q=kw1").meta.total, 2); | ||
| assert_eq!(anon.search("q=readme").meta.total, 1); | ||
| assert_eq!(anon.search("q=description").meta.total, 1); |
There was a problem hiding this comment.
ugh this is so much nicer i want to cry tears of joy! i can actually see what's being tested!! 😻😻😻
| let query = format!("user_id={}", user.id); | ||
| let mut response = ok_resp!(middle.call(req.with_query(&query))); | ||
| let json: CrateList = ::json(&mut response); | ||
| let user2 = app.db_new_user("user_bar"); |
There was a problem hiding this comment.
hoooo boy this test's setup code was confusing before, with 2 us and a user. much better now 👍
src/tests/owners.rs
Outdated
| let (krate_owned_by_team, team) = app.db(|conn| { | ||
| let t = new_team("team_foo").create_or_update(conn).unwrap(); | ||
| let krate = CrateBuilder::new("foo", user.as_model().id).expect_build(conn); | ||
| add_team_to_crate(&t, &krate, user.as_model(), conn).unwrap(); |
There was a problem hiding this comment.
maybe extract user.as_model() from here because it's used on 210 and 211, and you do that extraction in other tests and it looks nice there? not a huge deal tho
| fn crates_owned_by_user_id(&self, id: i32) -> CrateList { | ||
| let query = format!("user_id={}", id); | ||
| self.get_with_query("/api/v1/crates", &query).good() | ||
| fn search_by_user_id(&self, id: i32) -> CrateList { |
There was a problem hiding this comment.
this is a much better name, because this action is implemented using search. it's a good reminder rather than leading the reader to believe it's completely different 👍
src/tests/krate.rs
Outdated
| let conn = app.diesel_database.get().unwrap(); | ||
| let u = new_user("foo").create_or_update(&conn).unwrap(); | ||
| CrateBuilder::new("foo_bad_doc_url", u.id) | ||
| let _ = app.db(|conn| { |
There was a problem hiding this comment.
is this let _ = still needed?
| download("FOO_DOWNLOAD/1.0.0"); | ||
| assert_dl_count("FOO_DOWNLOAD/1.0.0", None, 1); | ||
| assert_dl_count("FOO_DOWNLOAD", None, 1); | ||
| // TODO: Is the behavior above a bug? |
There was a problem hiding this comment.
shit maybe?? if so it has been since here: 18c5527
| unfollow(); | ||
| unfollow(); | ||
| assert!(!is_following()); | ||
| assert_eq!(user.search("following=1").crates.len(), 0); |
There was a problem hiding this comment.
SO MUCH EASIER TO READ!!!
| sign_in_as(&mut req, &user); | ||
| CrateBuilder::new("foo_following", user.id).expect_build(&conn); | ||
| } | ||
| // TODO: Test anon requests as well? |
There was a problem hiding this comment.
Yeah we probably should. And once there are more tests that exercise following/unfollowing (i'm surprised but not really surprised this is the only one), it'd be nice to extract the closures into functions that take the user and crate name. But I think that can wait.
|
also merge conflicts :) |
Searching for "user_id={}" seem frequent enough to get a dedicated helper.
jtgeibel
force-pushed
the
convert-more-tests
branch
from
f2f10a5 to
db93266
Compare
|
Comments addressed. bors: r+ |
1514: Convert more tests r=jtgeibel a=jtgeibel This converts another batch of tests. The first 3 commits are straightforward conversions. The 4th commit includes 2 tests where I pulled some logic into closures. It makes the tests much easier to follow, but the diff isn't as straightforward. The final commit includes a few minor cleanups. Co-authored-by: Justin Geibel <[email protected]>
Build succeeded |
This converts another batch of tests. The first 3 commits are straightforward conversions. The 4th commit includes 2 tests where I pulled some logic into closures. It makes the tests much easier to follow, but the diff isn't as straightforward. The final commit includes a few minor cleanups.