Skip to content

Commit

Permalink
Auto merge of rust-lang#1686 - jtgeibel:reduce-test-allocations, r=ca…
Browse files Browse the repository at this point in the history
…rols10cents

 Reduce the number of git clones in the test framework

The test framework spends a lot of I/O and allocations initializing and
cloning git repositories, even though most tests do not enqueue a
background job to modify the index.  A `TestApp::full()` method is
added which initializes the `TestApp` with an index, background job
runner, and playback proxy.

It is also now possible to enqueue multiple crate publishes before
running a batch of background jobs.  A `Drop` impl on `TestAppInner`
ensures that all pending migrations are run and that no jobs have been
queued unless `TestApp::full()` was used.

Unlike the `enqueue_publish` helper the `yank` and `unyank` methods
automatically run migrations out of convenience since otherwise the
database is not updated.  This can be changed in the future if a test
cares about controlling exactly when the jobs are run.

Finally, this PR includes several other improvements:

* The initial commit for each remaining upstream index is now made
  directly against the bare repo, rather than doing a clone and push.
* The proxy only initializes a `Client` when recording.
* A playback proxy is not created when calling `app()`

Overall, when running the tests in `all`, these changes reduce the
cumulative allocation size from 3.45 GB to 689 MB.  Allocation counts
are reduced from 4,625,804 to 1,611,351.
  • Loading branch information
bors committed May 22, 2019
2 parents 002e63d + 85ee13e commit 960ced1
Show file tree
Hide file tree
Showing 13 changed files with 330 additions and 308 deletions.
6 changes: 0 additions & 6 deletions src/middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ pub use self::debug::*;
pub use self::ember_index_rewrite::EmberIndexRewrite;
pub use self::head::Head;
use self::log_connection_pool_status::LogConnectionPoolStatus;
use self::run_pending_background_jobs::RunPendingBackgroundJobs;
pub use self::security_headers::SecurityHeaders;
pub use self::static_or_continue::StaticOrContinue;

Expand All @@ -24,7 +23,6 @@ mod head;
mod log_connection_pool_status;
mod log_request;
mod require_user_agent;
mod run_pending_background_jobs;
mod security_headers;
mod static_or_continue;

Expand Down Expand Up @@ -100,9 +98,5 @@ pub fn build_middleware(app: Arc<App>, endpoints: R404) -> MiddlewareBuilder {
m.around(log_request::LogRequests::default());
}

if env == Env::Test {
m.add(RunPendingBackgroundJobs);
}

m
}
55 changes: 0 additions & 55 deletions src/middleware/run_pending_background_jobs.rs

This file was deleted.

13 changes: 8 additions & 5 deletions src/tests/all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,19 +114,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) {
git::init();
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 @@ -295,7 +298,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::cognitive_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
39 changes: 7 additions & 32 deletions src/tests/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@ use std::path::PathBuf;
use std::sync::Once;
use std::thread;

use url::Url;

fn root() -> PathBuf {
env::current_dir()
.unwrap()
.join("tmp")
.join("tests")
.join(thread::current().name().unwrap())
}

Expand All @@ -29,38 +28,14 @@ pub fn init() {
fs::create_dir_all(root().parent().unwrap()).unwrap();
});

// Prepare a bare remote repo
{
let bare = git2::Repository::init_bare(&bare()).unwrap();
let mut config = bare.config().unwrap();
config.set_str("user.name", "name").unwrap();
config.set_str("user.email", "email").unwrap();
}

// Initialize a fresh checkout
let checkout = git2::Repository::init(&checkout()).unwrap();
let url = Url::from_file_path(&*bare()).ok().unwrap().to_string();

// Setup the `origin` remote
checkout.remote_set_url("origin", &url).unwrap();
checkout.remote_set_pushurl("origin", Some(&url)).unwrap();
checkout
.remote_add_push("origin", "refs/heads/master")
.unwrap();

// Create an empty initial commit
let mut config = checkout.config().unwrap();
let bare = git2::Repository::init_bare(&bare()).unwrap();
let mut config = bare.config().unwrap();
config.set_str("user.name", "name").unwrap();
config.set_str("user.email", "email").unwrap();
let mut index = checkout.index().unwrap();
let mut index = bare.index().unwrap();
let id = index.write_tree().unwrap();
let tree = checkout.find_tree(id).unwrap();
let sig = checkout.signature().unwrap();
checkout
.commit(Some("HEAD"), &sig, &sig, "Initial Commit", &tree, &[])
let tree = bare.find_tree(id).unwrap();
let sig = bare.signature().unwrap();
bare.commit(Some("HEAD"), &sig, &sig, "Initial Commit", &tree, &[])
.unwrap();

// Push the commit to the remote repo
let mut origin = checkout.find_remote("origin").unwrap();
origin.push(&["refs/heads/master"], None).unwrap();
}
Loading

0 comments on commit 960ced1

Please sign in to comment.