Skip to content

Commit 960ced1

Browse files
committed
Auto merge of #1686 - jtgeibel:reduce-test-allocations, r=carols10cents
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.
2 parents 002e63d + 85ee13e commit 960ced1

13 files changed

+330
-308
lines changed

src/middleware.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ pub use self::debug::*;
1010
pub use self::ember_index_rewrite::EmberIndexRewrite;
1111
pub use self::head::Head;
1212
use self::log_connection_pool_status::LogConnectionPoolStatus;
13-
use self::run_pending_background_jobs::RunPendingBackgroundJobs;
1413
pub use self::security_headers::SecurityHeaders;
1514
pub use self::static_or_continue::StaticOrContinue;
1615

@@ -24,7 +23,6 @@ mod head;
2423
mod log_connection_pool_status;
2524
mod log_request;
2625
mod require_user_agent;
27-
mod run_pending_background_jobs;
2826
mod security_headers;
2927
mod static_or_continue;
3028

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

103-
if env == Env::Test {
104-
m.add(RunPendingBackgroundJobs);
105-
}
106-
107101
m
108102
}

src/middleware/run_pending_background_jobs.rs

Lines changed: 0 additions & 55 deletions
This file was deleted.

src/tests/all.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,19 +114,22 @@ pub struct OkBool {
114114
ok: bool,
115115
}
116116

117-
fn app() -> (
117+
/// Initialize the app and a proxy that can record and playback outgoing HTTP requests
118+
fn app_with_proxy() -> (
118119
record::Bomb,
119120
Arc<App>,
120121
conduit_middleware::MiddlewareBuilder,
121122
) {
122123
let (proxy, bomb) = record::proxy();
123-
let (app, handler) = simple_app(Some(proxy));
124+
let (app, handler) = init_app(Some(proxy));
124125
(bomb, app, handler)
125126
}
126127

127-
fn simple_app(proxy: Option<String>) -> (Arc<App>, conduit_middleware::MiddlewareBuilder) {
128-
git::init();
128+
fn app() -> (Arc<App>, conduit_middleware::MiddlewareBuilder) {
129+
init_app(None)
130+
}
129131

132+
fn init_app(proxy: Option<String>) -> (Arc<App>, conduit_middleware::MiddlewareBuilder) {
130133
let uploader = Uploader::S3 {
131134
bucket: s3::Bucket::new(
132135
String::from("alexcrichton-test"),
@@ -295,7 +298,7 @@ fn logout(req: &mut dyn Request) {
295298
fn multiple_live_references_to_the_same_connection_can_be_checked_out() {
296299
use std::ptr;
297300

298-
let (_bomb, app, _) = app();
301+
let (app, _) = app();
299302
let conn1 = app.diesel_database.get().unwrap();
300303
let conn2 = app.diesel_database.get().unwrap();
301304
let conn1_ref: &PgConnection = &conn1;

src/tests/category.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ fn show() {
7676
#[test]
7777
#[allow(clippy::cognitive_complexity)]
7878
fn update_crate() {
79-
let (_b, app, middle) = app();
79+
let (app, middle) = app();
8080
let mut req = req(Method::Get, "/api/v1/categories/foo");
8181
macro_rules! cnt {
8282
($req:expr, $cat:expr) => {{

src/tests/git.rs

Lines changed: 7 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,11 @@ use std::path::PathBuf;
44
use std::sync::Once;
55
use std::thread;
66

7-
use url::Url;
8-
97
fn root() -> PathBuf {
108
env::current_dir()
119
.unwrap()
1210
.join("tmp")
11+
.join("tests")
1312
.join(thread::current().name().unwrap())
1413
}
1514

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

32-
// Prepare a bare remote repo
33-
{
34-
let bare = git2::Repository::init_bare(&bare()).unwrap();
35-
let mut config = bare.config().unwrap();
36-
config.set_str("user.name", "name").unwrap();
37-
config.set_str("user.email", "email").unwrap();
38-
}
39-
40-
// Initialize a fresh checkout
41-
let checkout = git2::Repository::init(&checkout()).unwrap();
42-
let url = Url::from_file_path(&*bare()).ok().unwrap().to_string();
43-
44-
// Setup the `origin` remote
45-
checkout.remote_set_url("origin", &url).unwrap();
46-
checkout.remote_set_pushurl("origin", Some(&url)).unwrap();
47-
checkout
48-
.remote_add_push("origin", "refs/heads/master")
49-
.unwrap();
50-
51-
// Create an empty initial commit
52-
let mut config = checkout.config().unwrap();
31+
let bare = git2::Repository::init_bare(&bare()).unwrap();
32+
let mut config = bare.config().unwrap();
5333
config.set_str("user.name", "name").unwrap();
5434
config.set_str("user.email", "email").unwrap();
55-
let mut index = checkout.index().unwrap();
35+
let mut index = bare.index().unwrap();
5636
let id = index.write_tree().unwrap();
57-
let tree = checkout.find_tree(id).unwrap();
58-
let sig = checkout.signature().unwrap();
59-
checkout
60-
.commit(Some("HEAD"), &sig, &sig, "Initial Commit", &tree, &[])
37+
let tree = bare.find_tree(id).unwrap();
38+
let sig = bare.signature().unwrap();
39+
bare.commit(Some("HEAD"), &sig, &sig, "Initial Commit", &tree, &[])
6140
.unwrap();
62-
63-
// Push the commit to the remote repo
64-
let mut origin = checkout.find_remote("origin").unwrap();
65-
origin.push(&["refs/heads/master"], None).unwrap();
6641
}

0 commit comments

Comments
 (0)