Writing more integration tests

README.md

@@ -1,6 +1,7 @@
 Note: this project is still in early-development, so expect breaking changes.
 
 # gday
+[![Crates.io Version](https://img.shields.io/crates/v/gday_server)](https://crates.io/crates/gday_server)
 
 A command line tool for sending files.
 

gday/README.md

@@ -1,6 +1,7 @@
 Note: this project is still in early-development, so expect breaking changes.
 
 # gday
+[![Crates.io Version](https://img.shields.io/crates/v/gday_server)](https://crates.io/crates/gday_server)
 
 A command line tool for sending files.
 

gday_file_transfer/src/file_meta.rs

@@ -7,7 +7,7 @@ use std::{
 };
 
 /// Information about an offered file.
-#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
+#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, PartialOrd, Eq, Ord)]
 pub struct FileMeta {
     /// The path offered to the peer
     pub short_path: PathBuf,
@@ -16,7 +16,7 @@ pub struct FileMeta {
 }
 
 /// Information about a locally stored file
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord)]
 pub struct FileMetaLocal {
     /// The shortened path that will be offered to the peer
     pub short_path: PathBuf,
@@ -73,6 +73,8 @@ impl FileMeta {
 
         if number == 0 {
             Ok(None)
+        } else if number == 1 {
+            Ok(Some(path))
         } else {
             suffix_with_number(&mut path, number - 1);
             Ok(Some(path))
@@ -210,12 +212,14 @@ pub fn get_file_metas(paths: &[PathBuf]) -> Result<Vec<FileMetaLocal>, Error> {
             let b = &paths[j];
 
             // we don't want two top-level folders or files with the same name
-            // then we'd run into weird cases with FileMetaLocal.short_path
+            // then we'd run into ambiguity with FileMetaLocal.short_path
             if a.file_name() == b.file_name() && a.is_file() == b.is_file() {
                 let name = a.file_name().unwrap_or(OsStr::new("")).to_os_string();
                 return Err(Error::PathsHaveSameName(name));
             }
 
+            // we don't want one path to be a prefix of another, or we'd
+            // get duplicates
             if a.starts_with(b) {
                 return Err(Error::PathIsPrefix(b.to_path_buf(), a.to_path_buf()));
             }
@@ -263,13 +267,13 @@ fn get_file_metas_helper(
             .to_path_buf();
 
         // get the file's size
-        let size = path.metadata()?.len();
+        let len = path.metadata()?.len();
 
         // insert this file metadata into set
         let meta = FileMetaLocal {
             local_path: path.to_path_buf(),
             short_path,
-            len: size,
+            len,
         };
         files.push(meta);
     }

gday_file_transfer/src/lib.rs

@@ -110,7 +110,11 @@ pub enum Error {
     )]
     PathIsPrefix(PathBuf, PathBuf),
 
-    /// Two folders or files have same name. This would make the offered metadata ambiguous.
-    #[error("Two folders or files have same name: '{0:?}'. This would make the offered metadata ambiguous.")]
+    /// Two of the given folders or files have same name.
+    /// This would make the offered metadata ambiguous.
+    #[error(
+        "Two of the given folders or files have same name: '{0:?}'.
+        This would make the offered metadata ambiguous."
+    )]
     PathsHaveSameName(std::ffi::OsString),
 }

gday_file_transfer/src/offer.rs

@@ -99,8 +99,8 @@ impl FileResponseMsg {
     ) -> Result<Self, Error> {
         let mut response = Vec::with_capacity(offer.files.len());
 
-        for offered in &offer.files {
-            if offered.already_exists(save_dir)? {
+        for file_meta in &offer.files {
+            if file_meta.already_exists(save_dir)? {
                 // reject
                 response.push(None);
             } else {
@@ -168,7 +168,7 @@ impl FileResponseMsg {
 
     /// Returns the number of non-rejected files.
     pub fn get_num_not_rejected(&self) -> usize {
-        self.response.iter().filter_map(|f| *f).count()
+        self.response.iter().filter(|f| f.is_some()).count()
     }
 
     /// Returns the total number of only partially accepted files.

gday_file_transfer/tests/integration_test.rs

@@ -1,12 +1,12 @@
 #![forbid(unsafe_code)]
 #![warn(clippy::all)]
 use gday_file_transfer::{
-    get_file_metas, read_from, receive_files, send_files, write_to, FileMeta, FileMetaLocal,
-    FileOfferMsg, FileResponseMsg,
+    get_file_metas, read_from, receive_files, send_files, write_to, FileMetaLocal, FileOfferMsg,
+    FileResponseMsg,
 };
 use std::fs::{self, create_dir_all};
 use std::io::Write;
-use std::net::{Ipv6Addr, SocketAddr};
+use std::net::SocketAddr;
 use std::{fs::File, path::PathBuf};
 
 /// Returns a temporary directory
@@ -94,9 +94,9 @@ fn test_get_file_metas_1() {
     let test_dir = make_test_dir();
     let dir_path = test_dir.path();
     let dir_name = PathBuf::from(dir_path.file_name().unwrap());
-    let files = gday_file_transfer::get_file_metas(&[dir_path.to_path_buf()]).unwrap();
+    let mut result = gday_file_transfer::get_file_metas(&[dir_path.to_path_buf()]).unwrap();
 
-    let expected = [
+    let mut expected = [
         FileMetaLocal {
             short_path: dir_name.join("file1"),
             local_path: dir_path.join("file1"),
@@ -147,10 +147,10 @@ fn test_get_file_metas_1() {
         },
     ];
 
-    assert_eq!(files.len(), expected.len());
-    for e in expected {
-        assert!(files.contains(&e));
-    }
+    result.sort_unstable();
+    expected.sort_unstable();
+
+    assert_eq!(result, expected);
 }
 
 /// Confirm that [`get_file_metas()`] returns
@@ -160,14 +160,14 @@ fn test_get_file_metas_2() {
     let test_dir = make_test_dir();
     let dir_path = test_dir.path();
 
-    let files = gday_file_transfer::get_file_metas(&[
+    let mut result = gday_file_transfer::get_file_metas(&[
         dir_path.join("dir/subdir1/"),
         dir_path.join("dir/subdir2/file1"),
         dir_path.join("dir/subdir2/file2.txt"),
     ])
     .unwrap();
 
-    let expected = [
+    let mut expected = [
         FileMetaLocal {
             short_path: PathBuf::from("subdir1/file1"),
             local_path: dir_path.join("dir/subdir1/file1"),
@@ -198,17 +198,17 @@ fn test_get_file_metas_2() {
         },
     ];
 
-    assert_eq!(files.len(), expected.len());
-    for e in expected {
-        assert!(files.contains(&e));
-    }
+    result.sort_unstable();
+    expected.sort_unstable();
+
+    assert_eq!(result, expected);
 }
 
 /// Test the file transfer.
 #[test]
 fn file_transfer() {
     // The loopback address that peer_a will connect to.
-    let pipe_addr = SocketAddr::from((Ipv6Addr::LOCALHOST, 2000));
+    let pipe_addr: SocketAddr = "[::1]:2000".parse().unwrap();
 
     // Listens on the loopback address
     let listener = std::net::TcpListener::bind(pipe_addr).unwrap();
@@ -230,9 +230,9 @@ fn file_transfer() {
         ];
         let file_metas = get_file_metas(&paths).unwrap();
         let file_offer = FileOfferMsg::from(file_metas.clone());
-        write_to(file_offer, &mut stream_a).unwrap();
 
-        // read the response from the peer
+        // send offer, and read response
+        write_to(file_offer, &mut stream_a).unwrap();
         let response: FileResponseMsg = read_from(&mut stream_a).unwrap();
 
         // send the files
@@ -314,56 +314,3 @@ fn file_transfer() {
     assert!(fs::read(dir_b.path().join("dir/subdir2/file1")).is_err());
     assert!(fs::read(dir_b.path().join("dir/subdir2/file2.txt")).is_err());
 }
-
-/// Test serializing and deserializing [`FileOfferMsg`] and [`FileResponseMsg`].
-#[test]
-fn sending_messages() {
-    let mut pipe = std::collections::VecDeque::new();
-
-    for msg in get_offer_msg_examples() {
-        write_to(msg, &mut pipe).unwrap();
-    }
-
-    for msg in get_offer_msg_examples() {
-        let deserialized_msg: FileOfferMsg = read_from(&mut pipe).unwrap();
-        assert_eq!(msg, deserialized_msg);
-    }
-
-    for msg in get_response_msg_examples() {
-        write_to(msg, &mut pipe).unwrap();
-    }
-
-    for msg in get_response_msg_examples() {
-        let deserialized_msg: FileResponseMsg = read_from(&mut pipe).unwrap();
-        assert_eq!(msg, deserialized_msg);
-    }
-}
-
-fn get_offer_msg_examples() -> Vec<FileOfferMsg> {
-    vec![
-        FileOfferMsg {
-            files: vec![
-                FileMeta {
-                    short_path: PathBuf::from("example/path"),
-                    len: 43,
-                },
-                FileMeta {
-                    short_path: PathBuf::from("/foo/hello"),
-                    len: 50,
-                },
-            ],
-        },
-        FileOfferMsg { files: Vec::new() },
-    ]
-}
-
-fn get_response_msg_examples() -> Vec<FileResponseMsg> {
-    vec![
-        FileResponseMsg {
-            response: vec![None, Some(0), Some(100)],
-        },
-        FileResponseMsg {
-            response: vec![None, None, None],
-        },
-    ]
-}