Skip to content

Reduce the number of git clones in the test framework #1686

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 5 commits into from
May 22, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Switch to no proxy by default for tests
The remaining 12 tests that call `app()` directly do not need a proxy.
  • Loading branch information
jtgeibel committed May 1, 2019
commit 3fd1bfb72bf82804772b6048f609d7dd5c7063fa
13 changes: 9 additions & 4 deletions src/tests/all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,17 +108,22 @@ pub struct OkBool {
ok: bool,
}

fn app() -> (
/// Initialize the app and a proxy that can record and playback outgoing HTTP requests
fn app_with_proxy() -> (
record::Bomb,
Arc<App>,
conduit_middleware::MiddlewareBuilder,
) {
let (proxy, bomb) = record::proxy();
let (app, handler) = simple_app(Some(proxy));
let (app, handler) = init_app(Some(proxy));
(bomb, app, handler)
}

fn simple_app(proxy: Option<String>) -> (Arc<App>, conduit_middleware::MiddlewareBuilder) {
fn app() -> (Arc<App>, conduit_middleware::MiddlewareBuilder) {
init_app(None)
}

fn init_app(proxy: Option<String>) -> (Arc<App>, conduit_middleware::MiddlewareBuilder) {
let uploader = Uploader::S3 {
bucket: s3::Bucket::new(
String::from("alexcrichton-test"),
Expand Down Expand Up @@ -287,7 +292,7 @@ fn logout(req: &mut dyn Request) {
fn multiple_live_references_to_the_same_connection_can_be_checked_out() {
use std::ptr;

let (_bomb, app, _) = app();
let (app, _) = app();
let conn1 = app.diesel_database.get().unwrap();
let conn2 = app.diesel_database.get().unwrap();
let conn1_ref: &PgConnection = &conn1;
Expand Down
2 changes: 1 addition & 1 deletion src/tests/category.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ fn show() {
#[test]
#[allow(clippy::cyclomatic_complexity)]
fn update_crate() {
let (_b, app, middle) = app();
let (app, middle) = app();
let mut req = req(Method::Get, "/api/v1/categories/foo");
macro_rules! cnt {
($req:expr, $cat:expr) => {{
Expand Down
4 changes: 2 additions & 2 deletions src/tests/owners.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ fn test_accept_invitation() {
crate_owner_invitation: InvitationResponse,
}

let (_b, app, middle) = app();
let (app, middle) = app();
let mut req = req(Method::Get, "/api/v1/me/crate_owner_invitations");
let (krate, user) = {
let conn = app.diesel_database.get().unwrap();
Expand Down Expand Up @@ -313,7 +313,7 @@ fn test_decline_invitation() {
crate_owner_invitation: InvitationResponse,
}

let (_b, app, middle) = app();
let (app, middle) = app();
let mut req = req(Method::Get, "/api/v1/me/crate_owner_invitations");
let (krate, user) = {
let conn = app.diesel_database.get().unwrap();
Expand Down
16 changes: 8 additions & 8 deletions src/tests/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ fn updating_existing_user_doesnt_change_api_token() {
*/
#[test]
fn test_github_login_does_not_overwrite_email() {
let (_b, app, middle) = app();
let (app, middle) = app();
let mut req = req(Method::Get, "/api/v1/me");
let user = {
let conn = app.diesel_database.get().unwrap();
Expand Down Expand Up @@ -343,7 +343,7 @@ fn test_github_login_does_not_overwrite_email() {
*/
#[test]
fn test_email_get_and_put() {
let (_b, app, middle) = app();
let (app, middle) = app();
let mut req = req(Method::Get, "/api/v1/me");
let user = {
let conn = app.diesel_database.get().unwrap();
Expand Down Expand Up @@ -386,7 +386,7 @@ fn test_email_get_and_put() {
*/
#[test]
fn test_empty_email_not_added() {
let (_b, app, middle) = app();
let (app, middle) = app();
let mut req = req(Method::Get, "/api/v1/me");
let user = {
let conn = app.diesel_database.get().unwrap();
Expand Down Expand Up @@ -434,7 +434,7 @@ fn test_empty_email_not_added() {
*/
#[test]
fn test_this_user_cannot_change_that_user_email() {
let (_b, app, middle) = app();
let (app, middle) = app();
let mut req = req(Method::Get, "/api/v1/me");

let not_signed_in_user = {
Expand Down Expand Up @@ -471,7 +471,7 @@ fn test_this_user_cannot_change_that_user_email() {
*/
#[test]
fn test_insert_into_email_table() {
let (_b, app, middle) = app();
let (app, middle) = app();
let mut req = req(Method::Get, "/me");
let user = {
let conn = app.diesel_database.get().unwrap();
Expand Down Expand Up @@ -518,7 +518,7 @@ fn test_insert_into_email_table() {
*/
#[test]
fn test_insert_into_email_table_with_email_change() {
let (_b, app, middle) = app();
let (app, middle) = app();
let mut req = req(Method::Get, "/me");
let user = {
let conn = app.diesel_database.get().unwrap();
Expand Down Expand Up @@ -581,7 +581,7 @@ fn test_insert_into_email_table_with_email_change() {
fn test_confirm_user_email() {
use cargo_registry::schema::emails;

let (_b, app, middle) = app();
let (app, middle) = app();
let mut req = req(Method::Get, "/me");
let user = {
let conn = app.diesel_database.get().unwrap();
Expand Down Expand Up @@ -627,7 +627,7 @@ fn test_existing_user_email() {
use chrono::NaiveDateTime;
use diesel::update;

let (_b, app, middle) = app();
let (app, middle) = app();
let mut req = req(Method::Get, "/me");
{
let conn = app.diesel_database.get().unwrap();
Expand Down
10 changes: 5 additions & 5 deletions src/tests/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
//! to the underlying database model value (`User` and `ApiToken` respectively).

use crate::{
app, builders::PublishBuilder, record, CrateList, CrateResponse, GoodCrate, OkBool,
VersionResponse,
app, app_with_proxy, builders::PublishBuilder, record, CrateList, CrateResponse, GoodCrate,
OkBool, VersionResponse,
};
use cargo_registry::{
background_jobs::Environment,
Expand Down Expand Up @@ -93,7 +93,7 @@ pub struct TestApp(Rc<TestAppInner>);
impl TestApp {
/// Initialize an application with an `Uploader` that panics
pub fn init() -> TestAppBuilder {
let (app, middle) = crate::simple_app(None);
let (app, middle) = app();
let inner = Rc::new(TestAppInner {
app,
_bomb: None,
Expand All @@ -106,7 +106,7 @@ impl TestApp {

/// Initialize the app and a proxy that can record and playback outgoing HTTP requests
pub fn with_proxy() -> TestAppBuilder {
let (bomb, app, middle) = app();
let (bomb, app, middle) = app_with_proxy();
let inner = Rc::new(TestAppInner {
app,
_bomb: Some(bomb),
Expand All @@ -121,7 +121,7 @@ impl TestApp {
pub fn full() -> TestAppBuilder {
use crate::git;

let (bomb, app, middle) = app();
let (bomb, app, middle) = app_with_proxy();
git::init();

let thread_local_path = git::bare();
Expand Down