Skip to content

Commit 93b42fb

Browse files
committed
Fix our git tests to actually test that we pushed
Our git tests aren't actually testing very much at the moment. We can replace the entire body of `commit_and_push` with `Ok(())`, and the test suite would pass. Right now all our tests do is check that we manipulated files in a given directory, which is more of an implementation detail than anything else. Aside from not actually testing what we want, these tests also break with the setup I want to use for async index updates. Once the index updates are off the web server, there's no reason we need to keep a local checkout of the index. We can afford to just do a full clone every time, which simplifies our code. Once we do that though, our tests can't observe the directory the commit was made in. Pushing to a remote repository doesn't actually update the files you changed on disk. Instead, to observe the changes that were committed, we create a new clone of the repo and look at the files there. Almost all of the tests are easy to change to this structure. One of our tests manipulated a file in the location that it expected the server to manipulate though. We'd have to push that for the test to be legit. We could write a `commit_and_push` function for our tests, but that function will be a pain in the ass. Instead we can just go though a full crate upload twice to test what we want.
1 parent 27842db commit 93b42fb

File tree

4 files changed

+114
-23
lines changed

4 files changed

+114
-23
lines changed

Cargo.lock

Lines changed: 6 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ itertools = "0.6.0"
5959
scheduled-thread-pool = "0.2.0"
6060
derive_deref = "1.0.0"
6161
reqwest = "0.9.1"
62+
tempdir = "0.3.7"
6263

6364
lettre = "0.8"
6465
lettre_email = "0.8"

src/tests/http-data/krate_new_krate_git_upload_appends

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,75 @@
11
[
2+
{
3+
"request": {
4+
"uri": "http://alexcrichton-test.s3.amazonaws.com/crates/FPP/FPP-0.0.1.crate",
5+
"method": "PUT",
6+
"headers": [
7+
[
8+
"content-type",
9+
"application/x-tar"
10+
],
11+
[
12+
"accept-encoding",
13+
"gzip"
14+
],
15+
[
16+
"host",
17+
"alexcrichton-test.s3.amazonaws.com"
18+
],
19+
[
20+
"accept",
21+
"*/*"
22+
],
23+
[
24+
"user-agent",
25+
"reqwest/0.9.1"
26+
],
27+
[
28+
"authorization",
29+
"AWS AKIAICL5IWUZYWWKA7JA:UgUqqHJ9cQAZDdbcsxpnC0BI2eE="
30+
],
31+
[
32+
"content-length",
33+
"35"
34+
],
35+
[
36+
"date",
37+
"Fri, 15 Sep 2017 07:53:06 -0700"
38+
]
39+
],
40+
"body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA="
41+
},
42+
"response": {
43+
"status": 200,
44+
"headers": [
45+
[
46+
"content-length",
47+
"0"
48+
],
49+
[
50+
"ETag",
51+
"\"f9016ad360cebb4fe2e6e96e5949f022\""
52+
],
53+
[
54+
"Server",
55+
"AmazonS3"
56+
],
57+
[
58+
"date",
59+
"Fri, 15 Sep 2017 14:53:07 GMT"
60+
],
61+
[
62+
"x-amz-request-id",
63+
"949E6183C81895B0"
64+
],
65+
[
66+
"x-amz-id-2",
67+
"JWMemhtsWvAdoKxFS4Cp2RnI8fOqriCnXdHGFakRPw6k4JALJsToegxrkl9qzNoF9rhPAGKti4w="
68+
]
69+
],
70+
"body": ""
71+
}
72+
},
273
{
374
"request": {
475
"uri": "http://alexcrichton-test.s3.amazonaws.com/crates/FPP/FPP-1.0.0.crate",
@@ -70,4 +141,4 @@
70141
"body": ""
71142
}
72143
}
73-
]
144+
]

src/tests/krate.rs

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
extern crate diesel;
2+
extern crate tempdir;
23

34
use std::collections::HashMap;
45
use std::fs::{self, File};
56
use std::io;
67
use std::io::prelude::*;
78

89
use self::diesel::prelude::*;
10+
use self::tempdir::TempDir;
911
use chrono::Utc;
1012
use conduit::{Handler, Method};
1113
use diesel::dsl::*;
@@ -729,7 +731,8 @@ fn new_krate_with_dependency() {
729731
let mut response = ok_resp!(middle.call(&mut req));
730732
::json::<GoodCrate>(&mut response);
731733

732-
let path = ::git::checkout().join("ne/w_/new_dep");
734+
let remote_contents = clone_remote_repo();
735+
let path = remote_contents.path().join("ne/w_/new_dep");
733736
assert!(path.exists());
734737
let mut contents = String::new();
735738
File::open(&path)
@@ -767,7 +770,8 @@ fn new_renamed_crate() {
767770
let mut response = ok_resp!(middle.call(&mut req));
768771
::json::<GoodCrate>(&mut response);
769772

770-
let path = ::git::checkout().join("ne/w-/new-krate");
773+
let remote_contents = clone_remote_repo();
774+
let path = remote_contents.path().join("ne/w-/new-krate");
771775
assert!(path.exists());
772776
let mut contents = String::new();
773777
File::open(&path)
@@ -1068,7 +1072,8 @@ fn new_krate_git_upload() {
10681072
let mut response = ok_resp!(middle.call(&mut req));
10691073
::json::<GoodCrate>(&mut response);
10701074

1071-
let path = ::git::checkout().join("3/f/fgt");
1075+
let remote_contents = clone_remote_repo();
1076+
let path = remote_contents.path().join("3/f/fgt");
10721077
assert!(path.exists());
10731078
let mut contents = String::new();
10741079
File::open(&path)
@@ -1088,25 +1093,18 @@ fn new_krate_git_upload() {
10881093
#[test]
10891094
fn new_krate_git_upload_appends() {
10901095
let (_b, app, middle) = app();
1091-
let path = ::git::checkout().join("3/f/fpp");
1092-
fs::create_dir_all(path.parent().unwrap()).unwrap();
1093-
File::create(&path)
1094-
.unwrap()
1095-
.write_all(
1096-
br#"{"name":"FPP","vers":"0.0.1","deps":[],"features":{},"cksum":"3j3"}
1097-
"#,
1098-
).unwrap();
10991096

1097+
let mut req = new_req("FPP", "0.0.1");
1098+
let user = sign_in(&mut req, &app);
1099+
ok_resp!(middle.call(&mut req));
11001100
let mut req = new_req("FPP", "1.0.0");
1101-
sign_in(&mut req, &app);
1101+
sign_in_as(&mut req, &user);
11021102
let mut response = ok_resp!(middle.call(&mut req));
11031103
::json::<GoodCrate>(&mut response);
11041104

1105-
let mut contents = String::new();
1106-
File::open(&path)
1107-
.unwrap()
1108-
.read_to_string(&mut contents)
1109-
.unwrap();
1105+
let remote_contents = clone_remote_repo();
1106+
let path = remote_contents.path().join("3/f/fpp");
1107+
let contents = fs::read_to_string(&path).unwrap();
11101108
let mut lines = contents.lines();
11111109
let p1: git::Crate = serde_json::from_str(lines.next().unwrap().trim()).unwrap();
11121110
let p2: git::Crate = serde_json::from_str(lines.next().unwrap().trim()).unwrap();
@@ -1430,13 +1428,15 @@ fn yank() {
14301428
version: EncodableVersion,
14311429
}
14321430
let (_b, app, middle) = app();
1433-
let path = ::git::checkout().join("3/f/fyk");
14341431

14351432
// Upload a new crate, putting it in the git index
14361433
let mut req = new_req("fyk", "1.0.0");
14371434
sign_in(&mut req, &app);
14381435
let mut response = ok_resp!(middle.call(&mut req));
14391436
::json::<GoodCrate>(&mut response);
1437+
1438+
let remote_contents = clone_remote_repo();
1439+
let path = remote_contents.path().join("3/f/fyk");
14401440
let mut contents = String::new();
14411441
File::open(&path)
14421442
.unwrap()
@@ -1461,6 +1461,9 @@ fn yank() {
14611461
)
14621462
);
14631463
assert!(::json::<OkBool>(&mut r).ok);
1464+
1465+
let remote_contents = clone_remote_repo();
1466+
let path = remote_contents.path().join("3/f/fyk");
14641467
let mut contents = String::new();
14651468
File::open(&path)
14661469
.unwrap()
@@ -1483,6 +1486,9 @@ fn yank() {
14831486
)
14841487
);
14851488
assert!(::json::<OkBool>(&mut r).ok);
1489+
1490+
let remote_contents = clone_remote_repo();
1491+
let path = remote_contents.path().join("3/f/fyk");
14861492
let mut contents = String::new();
14871493
File::open(&path)
14881494
.unwrap()
@@ -2361,3 +2367,14 @@ fn new_krate_hard_links() {
23612367
let body = new_crate_to_body_with_tarball(&new_crate("foo", "1.1.0"), &tarball);
23622368
bad_resp!(middle.call(req.with_body(&body)));
23632369
}
2370+
2371+
/// We want to observe the contents of our push, but we can't do that in a
2372+
/// bare repo so we need to clone it to some random directory.
2373+
fn clone_remote_repo() -> TempDir {
2374+
use url::Url;
2375+
2376+
let tempdir = TempDir::new("tests").unwrap();
2377+
let url = Url::from_file_path(::git::bare()).unwrap();
2378+
git2::Repository::clone(url.as_str(), tempdir.path()).unwrap();
2379+
tempdir
2380+
}

0 commit comments

Comments
 (0)