Skip to content

Add TestApp::init() for tests that don't need HTTP recording #1519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ impl App {
/// Returns a client for making HTTP requests to upload crate files.
///
/// The handle will go through a proxy if the uploader being used has specified one, which
/// is only done in test mode in order to be able to record and inspect the HTTP requests
/// that tests make.
/// is only done in tests with `TestApp::with_proxy()` in order to be able to record and
/// inspect the HTTP requests that tests make.
pub fn http_client(&self) -> CargoResult<reqwest::Client> {
let mut builder = reqwest::Client::builder();
if let Some(proxy) = self.config.uploader.proxy() {
Expand Down
24 changes: 15 additions & 9 deletions src/tests/all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,26 +135,30 @@ fn app() -> (
conduit_middleware::MiddlewareBuilder,
) {
dotenv::dotenv().ok();
git::init();

let (proxy, bomb) = record::proxy();

// When testing we route all API traffic over HTTP so we can
// sniff/record it, but everywhere else we use https
let api_protocol = String::from("http");

let uploader = cargo_registry::Uploader::S3 {
bucket: s3::Bucket::new(
String::from("alexcrichton-test"),
None,
std::env::var("S3_ACCESS_KEY").unwrap_or_default(),
std::env::var("S3_SECRET_KEY").unwrap_or_default(),
&api_protocol,
// When testing we route all API traffic over HTTP so we can
// sniff/record it, but everywhere else we use https
"http",
),
proxy: Some(proxy),
cdn: None,
};

let (app, handler) = simple_app(uploader);
(bomb, app, handler)
}

fn simple_app(
uploader: cargo_registry::Uploader,
) -> (Arc<App>, conduit_middleware::MiddlewareBuilder) {
git::init();
let config = cargo_registry::Config {
uploader,
session_key: "test this has to be over 32 bytes long".to_string(),
Expand All @@ -166,13 +170,15 @@ fn app() -> (
max_upload_size: 3000,
max_unpack_size: 2000,
mirror: Replica::Primary,
api_protocol,
// When testing we route all API traffic over HTTP so we can
// sniff/record it, but everywhere else we use https
api_protocol: String::from("http"),
};
let app = App::new(&config);
t!(t!(app.diesel_database.get()).begin_test_transaction());
let app = Arc::new(app);
let handler = cargo_registry::build_handler(Arc::clone(&app));
(bomb, app, handler)
(app, handler)
}

// Return the environment variable only if it has been defined
Expand Down
6 changes: 3 additions & 3 deletions src/tests/category.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ struct CategoryWithSubcategories {

#[test]
fn index() {
let (app, anon) = TestApp::empty();
let (app, anon) = TestApp::init().empty();
let url = "/api/v1/categories";

// List 0 categories if none exist
Expand All @@ -51,7 +51,7 @@ fn index() {

#[test]
fn show() {
let (app, anon) = TestApp::empty();
let (app, anon) = TestApp::init().empty();
let url = "/api/v1/categories/foo-bar";

// Return not found if a category doesn't exist
Expand Down Expand Up @@ -161,7 +161,7 @@ fn update_crate() {

#[test]
fn category_slugs_returns_all_slugs_in_alphabetical_order() {
let (app, anon) = TestApp::empty();
let (app, anon) = TestApp::init().empty();
app.db(|conn| {
new_category("Foo", "foo", "For crates that foo")
.create_or_update(&conn)
Expand Down
8 changes: 4 additions & 4 deletions src/tests/keyword.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ struct GoodKeyword {
#[test]
fn index() {
let url = "/api/v1/keywords";
let (app, anon) = TestApp::empty();
let (app, anon) = TestApp::init().empty();
let json: KeywordList = anon.get(url).good();
assert_eq!(json.keywords.len(), 0);
assert_eq!(json.meta.total, 0);
Expand All @@ -37,7 +37,7 @@ fn index() {
#[test]
fn show() {
let url = "/api/v1/keywords/foo";
let (app, anon) = TestApp::empty();
let (app, anon) = TestApp::init().empty();
anon.get(url).assert_not_found();

app.db(|conn| {
Expand All @@ -50,7 +50,7 @@ fn show() {
#[test]
fn uppercase() {
let url = "/api/v1/keywords/UPPER";
let (app, anon) = TestApp::empty();
let (app, anon) = TestApp::init().empty();
anon.get(url).assert_not_found();

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

#[test]
fn update_crate() {
let (app, anon) = TestApp::empty();
let (app, anon) = TestApp::init().empty();
let cnt = |kw: &str| {
let json: GoodKeyword = anon.get(&format!("/api/v1/keywords/{}", kw)).good();
json.keyword.crates_cnt as usize
Expand Down
42 changes: 21 additions & 21 deletions src/tests/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl ::util::MockTokenUser {

#[test]
fn index() {
let (app, anon) = TestApp::empty();
let (app, anon) = TestApp::init().empty();
let json = anon.search("");
assert_eq!(json.crates.len(), 0);
assert_eq!(json.meta.total, 0);
Expand All @@ -98,7 +98,7 @@ fn index() {

#[test]
fn index_queries() {
let (app, anon, user) = TestApp::with_user();
let (app, anon, user) = TestApp::init().with_user();
let user = user.as_model();

let (krate, krate2) = app.db(|conn| {
Expand Down Expand Up @@ -179,7 +179,7 @@ fn index_queries() {

#[test]
fn search_includes_crates_where_name_is_stopword() {
let (app, anon, user) = TestApp::with_user();
let (app, anon, user) = TestApp::init().with_user();
let user = user.as_model();
app.db(|conn| {
CrateBuilder::new("which", user.id).expect_build(conn);
Expand All @@ -194,7 +194,7 @@ fn search_includes_crates_where_name_is_stopword() {

#[test]
fn exact_match_first_on_queries() {
let (app, anon, user) = TestApp::with_user();
let (app, anon, user) = TestApp::init().with_user();
let user = user.as_model();

app.db(|conn| {
Expand Down Expand Up @@ -236,7 +236,7 @@ fn exact_match_first_on_queries() {

#[test]
fn index_sorting() {
let (app, anon, user) = TestApp::with_user();
let (app, anon, user) = TestApp::init().with_user();
let user = user.as_model();

app.db(|conn| {
Expand Down Expand Up @@ -318,7 +318,7 @@ fn index_sorting() {

#[test]
fn exact_match_on_queries_with_sort() {
let (app, anon, user) = TestApp::with_user();
let (app, anon, user) = TestApp::init().with_user();
let user = user.as_model();

app.db(|conn| {
Expand Down Expand Up @@ -416,7 +416,7 @@ fn exact_match_on_queries_with_sort() {

#[test]
fn show() {
let (app, anon, user) = TestApp::with_user();
let (app, anon, user) = TestApp::init().with_user();
let user = user.as_model();

let krate = app.db(|conn| {
Expand Down Expand Up @@ -463,7 +463,7 @@ fn show() {

#[test]
fn yanked_versions_are_not_considered_for_max_version() {
let (app, anon, user) = TestApp::with_user();
let (app, anon, user) = TestApp::init().with_user();
let user = user.as_model();

app.db(|conn| {
Expand All @@ -481,7 +481,7 @@ fn yanked_versions_are_not_considered_for_max_version() {

#[test]
fn versions() {
let (app, anon) = TestApp::empty();
let (app, anon) = TestApp::init().empty();
app.db(|conn| {
let u = new_user("foo").create_or_update(conn).unwrap();
CrateBuilder::new("foo_versions", u.id)
Expand Down Expand Up @@ -854,7 +854,7 @@ fn valid_feature_names() {

#[test]
fn new_krate_too_big() {
let (_, _, user) = TestApp::with_user();
let (_, _, user) = TestApp::init().with_user();
let files = [("foo_big-1.0.0/big", &[b'a'; 2000] as &[_])];
let builder = PublishBuilder::new("foo_big").files(&files);

Expand Down Expand Up @@ -1103,13 +1103,13 @@ fn new_krate_with_readme() {

#[test]
fn summary_doesnt_die() {
let (_, anon) = TestApp::empty();
let (_, anon) = TestApp::init().empty();
anon.get::<SummaryResponse>("/api/v1/summary").good();
}

#[test]
fn summary_new_crates() {
let (app, anon, user) = TestApp::with_user();
let (app, anon, user) = TestApp::init().with_user();
let user = user.as_model();
app.db(|conn| {
let krate = CrateBuilder::new("some_downloads", user.id)
Expand Down Expand Up @@ -1176,7 +1176,7 @@ fn summary_new_crates() {
#[test]
fn download() {
use chrono::{Duration, Utc};
let (app, anon, user) = TestApp::with_user();
let (app, anon, user) = TestApp::with_proxy().with_user();
let user = user.as_model();

app.db(|conn| {
Expand Down Expand Up @@ -1260,7 +1260,7 @@ fn dependencies() {

#[test]
fn diesel_not_found_results_in_404() {
let (_, _, user) = TestApp::with_user();
let (_, _, user) = TestApp::init().with_user();

user.get("/api/v1/crates/foo_following/following")
.assert_not_found();
Expand All @@ -1269,7 +1269,7 @@ fn diesel_not_found_results_in_404() {
#[test]
fn following() {
// TODO: Test anon requests as well?
let (app, _, user) = TestApp::with_user();
let (app, _, user) = TestApp::init().with_user();

app.db(|conn| {
CrateBuilder::new("foo_following", user.as_model().id).expect_build(&conn);
Expand Down Expand Up @@ -1399,7 +1399,7 @@ fn yank() {

#[test]
fn yank_not_owner() {
let (app, _, _, token) = TestApp::with_token();
let (app, _, _, token) = TestApp::init().with_token();
app.db(|conn| {
let another_user = new_user("bar").create_or_update(conn).unwrap();
CrateBuilder::new("foo_not", another_user.id).expect_build(conn);
Expand Down Expand Up @@ -2019,7 +2019,7 @@ fn author_license_and_description_required() {
*/
#[test]
fn test_recent_download_count() {
let (app, anon, user) = TestApp::with_user();
let (app, anon, user) = TestApp::init().with_user();
let user = user.as_model();

app.db(|conn| {
Expand Down Expand Up @@ -2057,7 +2057,7 @@ fn test_recent_download_count() {
*/
#[test]
fn test_zero_downloads() {
let (app, anon, user) = TestApp::with_user();
let (app, anon, user) = TestApp::init().with_user();
let user = user.as_model();

app.db(|conn| {
Expand All @@ -2082,7 +2082,7 @@ fn test_zero_downloads() {
*/
#[test]
fn test_default_sort_recent() {
let (app, anon, user) = TestApp::with_user();
let (app, anon, user) = TestApp::init().with_user();
let user = user.as_model();

let (green_crate, potato_crate) = app.db(|conn| {
Expand Down Expand Up @@ -2145,7 +2145,7 @@ fn test_default_sort_recent() {

#[test]
fn block_bad_documentation_url() {
let (app, anon, user) = TestApp::with_user();
let (app, anon, user) = TestApp::init().with_user();
let user = user.as_model();

app.db(|conn| {
Expand All @@ -2163,7 +2163,7 @@ fn block_bad_documentation_url() {
// which call the `PUT /crates/:crate_id/owners` route
#[test]
fn test_cargo_invite_owners() {
let (app, _, owner) = TestApp::with_user();
let (app, _, owner) = TestApp::init().with_user();

let new_user = app.db_new_user("cilantro");
app.db(|conn| {
Expand Down
4 changes: 2 additions & 2 deletions src/tests/owners.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl ::util::MockCookieUser {

#[test]
fn new_crate_owner() {
let (app, _, user1, token) = TestApp::with_token();
let (app, _, user1, token) = TestApp::with_proxy().with_token();

// Create a crate under one user
let crate_to_publish = PublishBuilder::new("foo_owner").version("1.0.0");
Expand Down Expand Up @@ -203,7 +203,7 @@ fn owners_can_remove_self() {
*/
#[test]
fn check_ownership_two_crates() {
let (app, anon, user) = TestApp::with_user();
let (app, anon, user) = TestApp::init().with_user();
let user = user.as_model();

let (krate_owned_by_team, team) = app.db(|conn| {
Expand Down
Loading