Auto merge of #34 - filsmick:issue-32, r=killercup

Fix #32

#32 (comment)

The old file was only being partially overwritten, because its length didn't shrink along with its contents. This also adds the #33 integration test (I'm closing the corresponding PR).

src/manifest.rs

@@ -3,7 +3,7 @@ use std::collections::btree_map::Entry;
 use std::{env, str};
 use std::error::Error;
 use std::fs::{self, File, OpenOptions};
-use std::io::{Read, Seek, SeekFrom, Write};
+use std::io::{Read, Write};
 use std::path::{Path, PathBuf};
 use toml;
 
@@ -125,8 +125,7 @@ impl Manifest {
     }
 
     /// Overwrite a file with TOML data.
-    pub fn write_to_file<T: Seek + Write>(&self, file: &mut T) -> Result<(), Box<Error>> {
-        try!(file.seek(SeekFrom::Start(0)));
+    pub fn write_to_file(&self, file: &mut File) -> Result<(), Box<Error>> {
         let mut toml = self.data.clone();
 
         let (proj_header, proj_data) = try!(toml.remove("package")
@@ -136,11 +135,19 @@ impl Manifest {
                                                         .map(|data| ("project", data))
                                                 })
                                                 .ok_or(ManifestError::MissingManifest));
-        write!(file,
-               "[{}]\n{}{}",
-               proj_header,
-               proj_data,
-               toml::Value::Table(toml))
+
+        let new_contents = format!(
+          "[{}]\n{}{}",
+          proj_header,
+          proj_data,
+          toml::Value::Table(toml)
+        );
+        let new_contents_bytes = new_contents.as_bytes();
+
+        // We need to truncate the file, otherwise the new contents
+        // will be mixed up with the old ones.
+        try!(file.set_len(new_contents_bytes.len() as u64));
+        file.write_all(new_contents_bytes)
             .map_err(From::from)
     }
 

tests/cargo-add.rs

@@ -1,48 +1,9 @@
 extern crate tempdir;
 extern crate toml;
 
-use std::{fs, process};
-use std::io::prelude::*;
-use std::ffi::OsStr;
-
-fn clone_out_test(source: &str) -> (tempdir::TempDir, String) {
-    let tmpdir = tempdir::TempDir::new("cargo-add-test")
-        .ok().expect("failed to construct temporary directory");
-    fs::copy(source, tmpdir.path().join("Cargo.toml"))
-        .unwrap_or_else(|err| panic!("could not copy test manifest: {}", err));
-    let path = tmpdir.path().join("Cargo.toml").to_str().unwrap().to_string().clone();
-
-    (tmpdir, path)
-}
-
-fn execute_command<S>(command: &[S], manifest: &str) where S: AsRef<OsStr> {
-    let call = process::Command::new("target/debug/cargo-add")
-        .args(command)
-        .arg(format!("--manifest-path={}", manifest))
-        .env("CARGO_IS_TEST", "1")
-        .output().unwrap();
-
-    if !call.status.success() {
-        println!("Status code: {:?}", call.status);
-        println!("STDOUT: {}", String::from_utf8_lossy(&call.stdout));
-        println!("STDERR: {}", String::from_utf8_lossy(&call.stderr));
-        panic!("cargo-add failed to execute")
-    }
-}
-
-fn get_toml(manifest_path: &str) -> toml::Value {
-    let mut f = fs::File::open(manifest_path).unwrap();
-    let mut s = String::new();
-    f.read_to_string(&mut s).unwrap();
-    toml::Value::Table(toml::Parser::new(&s).parse().unwrap())
-}
-
-/// 'failure' dep not present
-fn no_manifest_failures(manifest: &toml::Value) -> bool {
-    manifest.lookup("dependencies.failure").is_none() &&
-    manifest.lookup("dev-dependencies.failure").is_none() &&
-    manifest.lookup("build-dependencies.failure").is_none()
-}
+use std::process;
+mod utils;
+use utils::*;
 
 #[test]
 fn adds_dependencies() {

tests/cargo-rm.rs

@@ -2,6 +2,36 @@ extern crate assert_cli;
 
 use assert_cli::assert_cli_output;
 
+mod utils;
+use utils::*;
+
+// https://github.com/killercup/cargo-edit/issues/32
+#[test]
+fn issue_32() {
+    let (_tmpdir, manifest) = clone_out_test("tests/fixtures/rm/Cargo.toml");
+
+    let toml = get_toml(&manifest);
+    assert!(toml.lookup("dependencies.foo").is_none());
+
+    execute_command(&["add", "foo@1.0"], &manifest);
+    execute_command(&["add", "bar@1.0.7"], &manifest);
+
+    let toml = get_toml(&manifest);
+    assert!(toml.lookup("dependencies.foo").is_some());
+    assert!(toml.lookup("dependencies.bar").is_some());
+
+    execute_command(&["rm", "foo"], &manifest);
+    execute_command(&["rm", "bar"], &manifest);
+
+    let toml = get_toml(&manifest);
+    assert!(toml.lookup("dependencies.foo").is_none());
+    assert!(toml.lookup("dependencies.bar").is_none());
+}
+
+
+
+
+
 #[test]
 #[ignore]
 fn no_argument() {

tests/utils.rs

@@ -0,0 +1,47 @@
+extern crate tempdir;
+extern crate toml;
+
+use std::{fs, process};
+use std::io::prelude::*;
+use std::ffi::OsStr;
+
+pub fn clone_out_test(source: &str) -> (tempdir::TempDir, String) {
+    let tmpdir = tempdir::TempDir::new("cargo-add-test")
+        .ok().expect("failed to construct temporary directory");
+    fs::copy(source, tmpdir.path().join("Cargo.toml"))
+        .unwrap_or_else(|err| panic!("could not copy test manifest: {}", err));
+    let path = tmpdir.path().join("Cargo.toml").to_str().unwrap().to_string().clone();
+
+    (tmpdir, path)
+}
+
+pub fn execute_command<S>(command: &[S], manifest: &str) where S: AsRef<OsStr> {
+    let subcommand_name = &command[0].as_ref().to_str().unwrap();
+
+    let call = process::Command::new(&format!("target/debug/cargo-{}", subcommand_name))
+        .args(command)
+        .arg(format!("--manifest-path={}", manifest))
+        .env("CARGO_IS_TEST", "1")
+        .output().unwrap();
+
+    if !call.status.success() {
+        println!("Status code: {:?}", call.status);
+        println!("STDOUT: {}", String::from_utf8_lossy(&call.stdout));
+        println!("STDERR: {}", String::from_utf8_lossy(&call.stderr));
+        panic!("cargo-add failed to execute")
+    }
+}
+
+pub fn get_toml(manifest_path: &str) -> toml::Value {
+    let mut f = fs::File::open(manifest_path).unwrap();
+    let mut s = String::new();
+    f.read_to_string(&mut s).unwrap();
+    toml::Value::Table(toml::Parser::new(&s).parse().unwrap())
+}
+
+/// 'failure' dep not present
+pub fn no_manifest_failures(manifest: &toml::Value) -> bool {
+    manifest.lookup("dependencies.failure").is_none() &&
+    manifest.lookup("dev-dependencies.failure").is_none() &&
+    manifest.lookup("build-dependencies.failure").is_none()
+}