Merge #1532

1532: Fix our git tests to actually test that we pushed r=jtgeibel a=sgrif

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.

Co-authored-by: Sean Griffin <[email protected]>

Author: bors-voyager[bot]

Cargo.lock

@@ -59,6 +59,7 @@ itertools = "0.6.0"
 scheduled-thread-pool = "0.2.0"
 derive_deref = "1.0.0"
 reqwest = "0.9.1"
+tempdir = "0.3.7"
 
 lettre = "0.8"
 lettre_email = "0.8"

Cargo.toml

@@ -1,4 +1,75 @@
 [
+  {
+    "request": {
+      "uri": "http://alexcrichton-test.s3.amazonaws.com/crates/FPP/FPP-0.0.1.crate",
+      "method": "PUT",
+      "headers": [
+        [
+          "content-type",
+          "application/x-tar"
+        ],
+        [
+          "accept-encoding",
+          "gzip"
+        ],
+        [
+          "host",
+          "alexcrichton-test.s3.amazonaws.com"
+        ],
+        [
+          "accept",
+          "*/*"
+        ],
+        [
+          "user-agent",
+          "reqwest/0.9.1"
+        ],
+        [
+          "authorization",
+          "AWS AKIAICL5IWUZYWWKA7JA:UgUqqHJ9cQAZDdbcsxpnC0BI2eE="
+        ],
+        [
+          "content-length",
+          "35"
+        ],
+        [
+          "date",
+          "Fri, 15 Sep 2017 07:53:06 -0700"
+        ]
+      ],
+      "body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA="
+    },
+    "response": {
+      "status": 200,
+      "headers": [
+        [
+          "content-length",
+          "0"
+        ],
+        [
+          "ETag",
+          "\"f9016ad360cebb4fe2e6e96e5949f022\""
+        ],
+        [
+          "Server",
+          "AmazonS3"
+        ],
+        [
+          "date",
+          "Fri, 15 Sep 2017 14:53:07 GMT"
+        ],
+        [
+          "x-amz-request-id",
+          "949E6183C81895B0"
+        ],
+        [
+          "x-amz-id-2",
+          "JWMemhtsWvAdoKxFS4Cp2RnI8fOqriCnXdHGFakRPw6k4JALJsToegxrkl9qzNoF9rhPAGKti4w="
+        ]
+      ],
+      "body": ""
+    }
+  },
   {
     "request": {
       "uri": "http://alexcrichton-test.s3.amazonaws.com/crates/FPP/FPP-1.0.0.crate",
@@ -70,4 +141,4 @@
       "body": ""
     }
   }
-]
+]

src/tests/http-data/krate_new_krate_git_upload_appends

@@ -1,11 +1,13 @@
 extern crate diesel;
+extern crate tempdir;
 
 use std::collections::HashMap;
 use std::fs::{self, File};
 use std::io;
 use std::io::prelude::*;
 
 use self::diesel::prelude::*;
+use self::tempdir::TempDir;
 use chrono::Utc;
 use conduit::{Handler, Method};
 use diesel::dsl::*;
@@ -729,7 +731,8 @@ fn new_krate_with_dependency() {
     let mut response = ok_resp!(middle.call(&mut req));
     ::json::<GoodCrate>(&mut response);
 
-    let path = ::git::checkout().join("ne/w_/new_dep");
+    let remote_contents = clone_remote_repo();
+    let path = remote_contents.path().join("ne/w_/new_dep");
     assert!(path.exists());
     let mut contents = String::new();
     File::open(&path)
@@ -767,7 +770,8 @@ fn new_renamed_crate() {
     let mut response = ok_resp!(middle.call(&mut req));
     ::json::<GoodCrate>(&mut response);
 
-    let path = ::git::checkout().join("ne/w-/new-krate");
+    let remote_contents = clone_remote_repo();
+    let path = remote_contents.path().join("ne/w-/new-krate");
     assert!(path.exists());
     let mut contents = String::new();
     File::open(&path)
@@ -1068,7 +1072,8 @@ fn new_krate_git_upload() {
     let mut response = ok_resp!(middle.call(&mut req));
     ::json::<GoodCrate>(&mut response);
 
-    let path = ::git::checkout().join("3/f/fgt");
+    let remote_contents = clone_remote_repo();
+    let path = remote_contents.path().join("3/f/fgt");
     assert!(path.exists());
     let mut contents = String::new();
     File::open(&path)
@@ -1088,25 +1093,18 @@ fn new_krate_git_upload() {
 #[test]
 fn new_krate_git_upload_appends() {
     let (_b, app, middle) = app();
-    let path = ::git::checkout().join("3/f/fpp");
-    fs::create_dir_all(path.parent().unwrap()).unwrap();
-    File::create(&path)
-        .unwrap()
-        .write_all(
-            br#"{"name":"FPP","vers":"0.0.1","deps":[],"features":{},"cksum":"3j3"}
-"#,
-        ).unwrap();
 
+    let mut req = new_req("FPP", "0.0.1");
+    let user = sign_in(&mut req, &app);
+    ok_resp!(middle.call(&mut req));
     let mut req = new_req("FPP", "1.0.0");
-    sign_in(&mut req, &app);
+    sign_in_as(&mut req, &user);
     let mut response = ok_resp!(middle.call(&mut req));
     ::json::<GoodCrate>(&mut response);
 
-    let mut contents = String::new();
-    File::open(&path)
-        .unwrap()
-        .read_to_string(&mut contents)
-        .unwrap();
+    let remote_contents = clone_remote_repo();
+    let path = remote_contents.path().join("3/f/fpp");
+    let contents = fs::read_to_string(&path).unwrap();
     let mut lines = contents.lines();
     let p1: git::Crate = serde_json::from_str(lines.next().unwrap().trim()).unwrap();
     let p2: git::Crate = serde_json::from_str(lines.next().unwrap().trim()).unwrap();
@@ -1430,13 +1428,15 @@ fn yank() {
         version: EncodableVersion,
     }
     let (_b, app, middle) = app();
-    let path = ::git::checkout().join("3/f/fyk");
 
     // Upload a new crate, putting it in the git index
     let mut req = new_req("fyk", "1.0.0");
     sign_in(&mut req, &app);
     let mut response = ok_resp!(middle.call(&mut req));
     ::json::<GoodCrate>(&mut response);
+
+    let remote_contents = clone_remote_repo();
+    let path = remote_contents.path().join("3/f/fyk");
     let mut contents = String::new();
     File::open(&path)
         .unwrap()
@@ -1461,6 +1461,9 @@ fn yank() {
         )
     );
     assert!(::json::<OkBool>(&mut r).ok);
+
+    let remote_contents = clone_remote_repo();
+    let path = remote_contents.path().join("3/f/fyk");
     let mut contents = String::new();
     File::open(&path)
         .unwrap()
@@ -1483,6 +1486,9 @@ fn yank() {
         )
     );
     assert!(::json::<OkBool>(&mut r).ok);
+
+    let remote_contents = clone_remote_repo();
+    let path = remote_contents.path().join("3/f/fyk");
     let mut contents = String::new();
     File::open(&path)
         .unwrap()
@@ -2361,3 +2367,14 @@ fn new_krate_hard_links() {
     let body = new_crate_to_body_with_tarball(&new_crate("foo", "1.1.0"), &tarball);
     bad_resp!(middle.call(req.with_body(&body)));
 }
+
+/// We want to observe the contents of our push, but we can't do that in a
+/// bare repo so we need to clone it to some random directory.
+fn clone_remote_repo() -> TempDir {
+    use url::Url;
+
+    let tempdir = TempDir::new("tests").unwrap();
+    let url = Url::from_file_path(::git::bare()).unwrap();
+    git2::Repository::clone(url.as_str(), tempdir.path()).unwrap();
+    tempdir
+}