Skip to content

Commit 6144edf

Browse files
committed
Add TestApp::simple() for tests that don't need HTTP recording
The unused `Uploader::NoOp` option is now `Uploader::Panic` which panics on any usage. The panic message directs users to initialize the app with `TestApp::full()`. While this adds more panics to the main lib, the application configuration isn't dynamic and it should be obvious that `Uploader::Panic` is not to be used to configure a production application.
1 parent ab0163c commit 6144edf

File tree

10 files changed

+136
-96
lines changed

10 files changed

+136
-96
lines changed

src/app.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ impl App {
103103
/// Returns a client for making HTTP requests to upload crate files.
104104
///
105105
/// The handle will go through a proxy if the uploader being used has specified one, which
106-
/// is only done in test mode in order to be able to record and inspect the HTTP requests
107-
/// that tests make.
106+
/// is only done in tests with `TestApp::full()` in order to be able to record and inspect
107+
/// the HTTP requests that tests make.
108108
pub fn http_client(&self) -> CargoResult<reqwest::Client> {
109109
let mut builder = reqwest::Client::builder();
110110
if let Some(proxy) = self.config.uploader.proxy() {

src/tests/all.rs

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -134,26 +134,30 @@ fn app() -> (
134134
conduit_middleware::MiddlewareBuilder,
135135
) {
136136
dotenv::dotenv().ok();
137-
git::init();
138137

139138
let (proxy, bomb) = record::proxy();
140-
141-
// When testing we route all API traffic over HTTP so we can
142-
// sniff/record it, but everywhere else we use https
143-
let api_protocol = String::from("http");
144-
145139
let uploader = cargo_registry::Uploader::S3 {
146140
bucket: s3::Bucket::new(
147141
String::from("alexcrichton-test"),
148142
None,
149143
std::env::var("S3_ACCESS_KEY").unwrap_or_default(),
150144
std::env::var("S3_SECRET_KEY").unwrap_or_default(),
151-
&api_protocol,
145+
// When testing we route all API traffic over HTTP so we can
146+
// sniff/record it, but everywhere else we use https
147+
"http",
152148
),
153149
proxy: Some(proxy),
154150
cdn: None,
155151
};
156152

153+
let (app, handler) = simple_app(uploader);
154+
(bomb, app, handler)
155+
}
156+
157+
fn simple_app(
158+
uploader: cargo_registry::Uploader,
159+
) -> (Arc<App>, conduit_middleware::MiddlewareBuilder) {
160+
git::init();
157161
let config = cargo_registry::Config {
158162
uploader,
159163
session_key: "test this has to be over 32 bytes long".to_string(),
@@ -165,13 +169,15 @@ fn app() -> (
165169
max_upload_size: 3000,
166170
max_unpack_size: 2000,
167171
mirror: Replica::Primary,
168-
api_protocol,
172+
// When testing we route all API traffic over HTTP so we can
173+
// sniff/record it, but everywhere else we use https
174+
api_protocol: String::from("http"),
169175
};
170176
let app = App::new(&config);
171177
t!(t!(app.diesel_database.get()).begin_test_transaction());
172178
let app = Arc::new(app);
173179
let handler = cargo_registry::build_handler(Arc::clone(&app));
174-
(bomb, app, handler)
180+
(app, handler)
175181
}
176182

177183
// Return the environment variable only if it has been defined

src/tests/keyword.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ struct GoodKeyword {
1919
#[test]
2020
fn index() {
2121
let url = "/api/v1/keywords";
22-
let (app, anon) = TestApp::empty();
22+
let (app, anon) = TestApp::simple().empty();
2323
let json: KeywordList = anon.get(url).good();
2424
assert_eq!(json.keywords.len(), 0);
2525
assert_eq!(json.meta.total, 0);
@@ -37,7 +37,7 @@ fn index() {
3737
#[test]
3838
fn show() {
3939
let url = "/api/v1/keywords/foo";
40-
let (app, anon) = TestApp::empty();
40+
let (app, anon) = TestApp::simple().empty();
4141
anon.get(url).assert_not_found();
4242

4343
app.db(|conn| {
@@ -50,7 +50,7 @@ fn show() {
5050
#[test]
5151
fn uppercase() {
5252
let url = "/api/v1/keywords/UPPER";
53-
let (app, anon) = TestApp::empty();
53+
let (app, anon) = TestApp::simple().empty();
5454
anon.get(url).assert_not_found();
5555

5656
app.db(|conn| {
@@ -62,7 +62,7 @@ fn uppercase() {
6262

6363
#[test]
6464
fn update_crate() {
65-
let (app, anon) = TestApp::empty();
65+
let (app, anon) = TestApp::simple().empty();
6666
let cnt = |kw: &str| {
6767
let json: GoodKeyword = anon.get(&format!("/api/v1/keywords/{}", kw)).good();
6868
json.keyword.crates_cnt as usize

src/tests/krate.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ impl ::util::MockTokenUser {
7878
#[test]
7979
fn index() {
8080
let url = "/api/v1/crates";
81-
let (app, anon) = TestApp::empty();
81+
let (app, anon) = TestApp::simple().empty();
8282
let json: CrateList = anon.get(url).good();
8383
assert_eq!(json.crates.len(), 0);
8484
assert_eq!(json.meta.total, 0);
@@ -551,7 +551,7 @@ fn yanked_versions_are_not_considered_for_max_version() {
551551

552552
#[test]
553553
fn versions() {
554-
let (app, anon) = TestApp::empty();
554+
let (app, anon) = TestApp::simple().empty();
555555
app.db(|conn| {
556556
let u = new_user("foo").create_or_update(conn).unwrap();
557557
CrateBuilder::new("foo_versions", u.id)
@@ -923,7 +923,7 @@ fn valid_feature_names() {
923923

924924
#[test]
925925
fn new_krate_too_big() {
926-
let (_, _, user) = TestApp::with_user();
926+
let (_, _, user) = TestApp::simple().with_user();
927927
let files = [("foo_big-1.0.0/big", &[b'a'; 2000] as &[_])];
928928
let builder = PublishBuilder::new("foo_big").files(&files);
929929

@@ -1358,7 +1358,7 @@ fn dependencies() {
13581358

13591359
#[test]
13601360
fn diesel_not_found_results_in_404() {
1361-
let (_, _, user) = TestApp::with_user();
1361+
let (_, _, user) = TestApp::simple().with_user();
13621362

13631363
user.get("/api/v1/crates/foo_following/following")
13641364
.assert_not_found();
@@ -1500,7 +1500,7 @@ fn yank() {
15001500

15011501
#[test]
15021502
fn yank_not_owner() {
1503-
let (app, _, _, token) = TestApp::with_token();
1503+
let (app, _, _, token) = TestApp::simple().with_token();
15041504
app.db(|conn| {
15051505
let another_user = new_user("bar").create_or_update(conn).unwrap();
15061506
CrateBuilder::new("foo_not", another_user.id).expect_build(conn);

src/tests/owners.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ impl ::util::MockCookieUser {
6363

6464
#[test]
6565
fn new_crate_owner() {
66-
let (app, _, user1, token) = TestApp::with_token();
66+
let (app, _, user1, token) = TestApp::full().with_token();
6767

6868
// Create a crate under one user
6969
let crate_to_publish = PublishBuilder::new("foo_owner").version("1.0.0");

src/tests/token.rs

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -36,20 +36,20 @@ static NEW_BAR: &[u8] = br#"{ "api_token": { "name": "bar" } }"#;
3636

3737
#[test]
3838
fn list_logged_out() {
39-
let (_, anon) = TestApp::empty();
39+
let (_, anon) = TestApp::simple().empty();
4040
anon.get(URL).assert_forbidden();
4141
}
4242

4343
#[test]
4444
fn list_empty() {
45-
let (_, _, user) = TestApp::with_user();
45+
let (_, _, user) = TestApp::simple().with_user();
4646
let json: ListResponse = user.get(URL).good();
4747
assert_eq!(json.api_tokens.len(), 0);
4848
}
4949

5050
#[test]
5151
fn list_tokens() {
52-
let (app, _, user) = TestApp::with_user();
52+
let (app, _, user) = TestApp::simple().with_user();
5353
let id = user.as_model().id;
5454
let tokens = app.db(|conn| {
5555
vec![
@@ -71,13 +71,13 @@ fn list_tokens() {
7171

7272
#[test]
7373
fn create_token_logged_out() {
74-
let (_, anon) = TestApp::empty();
74+
let (_, anon) = TestApp::simple().empty();
7575
anon.put(URL, NEW_BAR).assert_forbidden();
7676
}
7777

7878
#[test]
7979
fn create_token_invalid_request() {
80-
let (_, _, user) = TestApp::with_user();
80+
let (_, _, user) = TestApp::simple().with_user();
8181
let invalid = br#"{ "name": "" }"#;
8282
let json = user.put::<()>(URL, invalid).bad_with_status(400);
8383

@@ -86,7 +86,7 @@ fn create_token_invalid_request() {
8686

8787
#[test]
8888
fn create_token_no_name() {
89-
let (_, _, user) = TestApp::with_user();
89+
let (_, _, user) = TestApp::simple().with_user();
9090
let empty_name = br#"{ "api_token": { "name": "" } }"#;
9191
let json = user.put::<()>(URL, empty_name).bad_with_status(400);
9292

@@ -95,7 +95,7 @@ fn create_token_no_name() {
9595

9696
#[test]
9797
fn create_token_long_body() {
98-
let (_, _, user) = TestApp::with_user();
98+
let (_, _, user) = TestApp::simple().with_user();
9999
let too_big = &[5; 5192]; // Send a request with a 5kB body of 5's
100100
let json = user.put::<()>(URL, too_big).bad_with_status(400);
101101

@@ -104,7 +104,7 @@ fn create_token_long_body() {
104104

105105
#[test]
106106
fn create_token_exceeded_tokens_per_user() {
107-
let (app, _, user) = TestApp::with_user();
107+
let (app, _, user) = TestApp::simple().with_user();
108108
let id = user.as_model().id;
109109
app.db(|conn| {
110110
for i in 0..1000 {
@@ -118,7 +118,7 @@ fn create_token_exceeded_tokens_per_user() {
118118

119119
#[test]
120120
fn create_token_success() {
121-
let (app, _, user) = TestApp::with_user();
121+
let (app, _, user) = TestApp::simple().with_user();
122122

123123
let json: NewResponse = user.put(URL, NEW_BAR).good();
124124
assert_eq!(json.api_token.name, "bar");
@@ -133,7 +133,7 @@ fn create_token_success() {
133133

134134
#[test]
135135
fn create_token_multiple_have_different_values() {
136-
let (_, _, user) = TestApp::with_user();
136+
let (_, _, user) = TestApp::simple().with_user();
137137
let first: NewResponse = user.put(URL, NEW_BAR).good();
138138
let second: NewResponse = user.put(URL, NEW_BAR).good();
139139

@@ -142,7 +142,7 @@ fn create_token_multiple_have_different_values() {
142142

143143
#[test]
144144
fn create_token_multiple_users_have_different_values() {
145-
let (app, _, user1) = TestApp::with_user();
145+
let (app, _, user1) = TestApp::simple().with_user();
146146
let first_token: NewResponse = user1.put(URL, NEW_BAR).good();
147147

148148
let user2 = app.db_new_user("bar");
@@ -153,7 +153,7 @@ fn create_token_multiple_users_have_different_values() {
153153

154154
#[test]
155155
fn cannot_create_token_with_token() {
156-
let (_, _, _, token) = TestApp::with_token();
156+
let (_, _, _, token) = TestApp::simple().with_token();
157157
let json = token
158158
.put::<()>(
159159
"/api/v1/me/tokens",
@@ -168,13 +168,13 @@ fn cannot_create_token_with_token() {
168168

169169
#[test]
170170
fn revoke_token_non_existing() {
171-
let (_, _, user) = TestApp::with_user();
171+
let (_, _, user) = TestApp::simple().with_user();
172172
let _json: RevokedResponse = user.delete("/api/v1/me/tokens/5").good();
173173
}
174174

175175
#[test]
176176
fn revoke_token_doesnt_revoke_other_users_token() {
177-
let (app, _, user1, token) = TestApp::with_token();
177+
let (app, _, user1, token) = TestApp::simple().with_token();
178178
let user1 = user1.as_model();
179179
let token = token.as_model();
180180
let user2 = app.db_new_user("baz");
@@ -201,7 +201,7 @@ fn revoke_token_doesnt_revoke_other_users_token() {
201201

202202
#[test]
203203
fn revoke_token_success() {
204-
let (app, _, user, token) = TestApp::with_token();
204+
let (app, _, user, token) = TestApp::simple().with_token();
205205

206206
// List tokens contains the token
207207
app.db(|conn| {
@@ -227,7 +227,7 @@ fn revoke_token_success() {
227227
#[test]
228228
fn token_gives_access_to_me() {
229229
let url = "/api/v1/me";
230-
let (_, anon, user, token) = TestApp::with_token();
230+
let (_, anon, user, token) = TestApp::simple().with_token();
231231

232232
anon.get(url).assert_forbidden();
233233

@@ -238,7 +238,7 @@ fn token_gives_access_to_me() {
238238
#[test]
239239
fn using_token_updates_last_used_at() {
240240
let url = "/api/v1/me";
241-
let (app, anon, user, token) = TestApp::with_token();
241+
let (app, anon, user, token) = TestApp::simple().with_token();
242242

243243
anon.get(url).assert_forbidden();
244244
user.get::<EncodableMe>(url).good();

src/tests/user.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,22 +29,22 @@ pub struct UserShowPrivateResponse {
2929

3030
#[test]
3131
fn auth_gives_a_token() {
32-
let (_, anon) = TestApp::empty();
32+
let (_, anon) = TestApp::simple().empty();
3333
let json: AuthResponse = anon.get("/authorize_url").good();
3434
assert!(json.url.contains(&json.state));
3535
}
3636

3737
#[test]
3838
fn access_token_needs_data() {
39-
let (_, anon) = TestApp::empty();
39+
let (_, anon) = TestApp::simple().empty();
4040
let json = anon.get::<()>("/authorize").bad_with_status(200); // Change endpoint to 400?
4141
assert!(json.errors[0].detail.contains("invalid state"));
4242
}
4343

4444
#[test]
4545
fn me() {
4646
let url = "/api/v1/me";
47-
let (app, anon) = TestApp::empty();
47+
let (app, anon) = TestApp::simple().empty();
4848
anon.get(url).assert_forbidden();
4949

5050
let user = app.db_new_user("foo");
@@ -115,7 +115,7 @@ fn show_latest_user_case_insensitively() {
115115

116116
#[test]
117117
fn crates_by_user_id() {
118-
let (app, _, user) = TestApp::with_user();
118+
let (app, _, user) = TestApp::simple().with_user();
119119
let id = user.as_model().id;
120120
app.db(|conn| {
121121
CrateBuilder::new("foo_my_packages", id).expect_build(conn);

0 commit comments

Comments
 (0)