Merge branch 'develop' into feature/unit_tests

- use `unwrap` while testing

src/file/checked.rs

@@ -32,7 +32,8 @@ impl AsRef<[u8]> for Checked {
 }
 
 impl Checked {
-    pub(super) fn new_direct(path: PathBuf, size: u64, hash: Option<String>) -> Self {
+    /// create this directly, without any checks
+    pub(super) unsafe fn new_unchecked(path: PathBuf, size: u64, hash: Option<String>) -> Self {
         Self { path, size, hash }
     }
 
@@ -93,9 +94,7 @@ impl Checked {
     ) -> crate::Result<Uploading> {
         let file_id = client.file_create(uri, alias_id, share_id, &self)?;
 
-        Ok(Uploading::new_direct(
-            self.path, self.size, self.hash, file_id,
-        ))
+        Ok(unsafe { Uploading::new_unchecked(self.path, self.size, self.hash, file_id) })
     }
 }
 
@@ -117,16 +116,19 @@ impl FileTrait for Checked {
 mod tests {
     use tempfile::{NamedTempFile, TempDir};
 
-    use crate::test_util::{
+    use crate::{
+        sharry::{Client, json::NewShareRequest},
+        test_util::{
             MockClient, check_trait, create_file,
             data::{HASHES_STD_GOOD, cases, data},
+        },
     };
 
     use super::*;
 
     fn create_checked(content: &[u8]) -> (Checked, NamedTempFile) {
         let file = create_file(content);
-        let chk = Checked::new(file.path()).expect("creating `Checked` should succeed");
+        let chk = Checked::new(file.path()).unwrap();
 
         // return both, so the `NamedTempFile` is not auto-deleted here
         (chk, file)
@@ -136,21 +138,14 @@ mod tests {
     fn new_on_existing_file_works() {
         for (content, size) in cases() {
             let (chk, file) = create_checked(content);
-
-            let path = file
-                .path()
-                .canonicalize()
-                .expect("the file should have a canonical path");
+            let path = file.path().canonicalize().unwrap();
 
             assert_eq!(chk.path, path);
             assert_eq!(chk.size, size);
             assert!(chk.hash.is_none());
 
             // `FileTrait`
-            assert_eq!(
-                chk.get_name(),
-                file.path().file_name().expect("`file_name` should succeed")
-            );
+            assert_eq!(chk.get_name(), file.path().file_name().unwrap());
             assert_eq!(chk.get_size(), size);
 
             check_trait(
@@ -160,8 +155,8 @@ mod tests {
                 "Checked",
             );
 
-            // new_direct
-            let chk = Checked::new_direct(chk.path, chk.size, chk.hash);
+            // new_unchecked
+            let chk = unsafe { Checked::new_unchecked(chk.path, chk.size, chk.hash) };
             assert_eq!(chk.path, path);
             assert_eq!(chk.size, size);
             assert!(chk.hash.is_none());
@@ -170,13 +165,13 @@ mod tests {
 
     #[test]
     fn new_on_dir_errors() {
-        let tempdir = TempDir::new().expect("creating temp dir");
+        let tempdir = TempDir::new().unwrap();
         let fs_root = PathBuf::from("/");
 
         let dirs = [tempdir.path(), fs_root.as_path()];
 
         for p in dirs {
-            let err = Checked::new(p).expect_err("creating `Checked` should fail");
+            let err = Checked::new(p).unwrap_err();
             assert_eq!(err.kind(), io::ErrorKind::InvalidInput);
             #[cfg(target_os = "linux")]
             assert_eq!(err.to_string(), "Not a regular file");
@@ -185,11 +180,11 @@ mod tests {
 
     #[test]
     fn new_on_nex_errors() {
-        let tempdir = TempDir::new().expect("creating temp dir");
+        let tempdir = TempDir::new().unwrap();
         let nex_paths = [0, 1, 2, 3, 4].map(|i| tempdir.path().join(format!("nex_{i}.ext")));
 
         for p in nex_paths {
-            let err = Checked::new(p).expect_err("creating `Checked` should fail");
+            let err = Checked::new(p).unwrap_err();
             assert_eq!(err.kind(), io::ErrorKind::NotFound);
             #[cfg(target_os = "linux")]
             assert_eq!(err.to_string(), "No such file or directory (os error 2)");
@@ -201,9 +196,9 @@ mod tests {
         for (content, hash) in data().zip(HASHES_STD_GOOD) {
             let (mut chk, _file) = create_checked(content);
 
-            chk.hash(drop).expect("`hash` should succeed");
+            chk.hash(drop).unwrap();
             // `FileTrait`
-            chk.check_hash(drop).expect("`check_hash` should succeed");
+            chk.check_hash(drop).unwrap();
 
             assert_eq!(chk.hash, Some(hash.to_string()));
         }
@@ -216,7 +211,7 @@ mod tests {
 
             // fake hash
             chk.hash = Some(String::default());
-            let err = chk.hash(drop).expect_err("`hash` twice should fail");
+            let err = chk.hash(drop).unwrap_err();
 
             assert!(err.is_mismatch("unhashed file", chk.path.display().to_string()));
         }
@@ -225,18 +220,18 @@ mod tests {
     #[test]
     fn start_upload_works() {
         let client = MockClient::default();
-        let share_id = client.add_share();
+        let uri = sharry::Uri::from(true);
+        let alias_id = sharry::AliasID::from(true);
+
+        let share_id = client
+            .share_create(&uri, &alias_id, NewShareRequest::new("share", 0))
+            .unwrap();
 
         for content in data() {
             let (chk, _file) = create_checked(content);
 
             assert!(
-                chk.start_upload(
-                    &client,
-                    &sharry::Uri::from(true),
-                    &sharry::AliasID::from(true),
-                    &share_id
-                )
+                chk.start_upload(&client, &uri, &alias_id, &share_id)
                     .is_ok()
             );
         }

src/file/chunk.rs

@@ -40,7 +40,12 @@ where
 }
 
 impl<'t> Chunk<'t> {
-    pub(super) fn new_direct(file_id: sharry::FileID, offset: u64, data: &'t [u8]) -> Self {
+    /// create this directly, without any checks
+    pub(super) unsafe fn new_unchecked(
+        file_id: sharry::FileID,
+        offset: u64,
+        data: &'t [u8],
+    ) -> Self {
         Self {
             file_id,
             offset,
@@ -83,9 +88,10 @@ mod tests {
 
     #[test]
     fn basic_tests() {
+        // items from `DATA_LENGTHS_BAD` used as mock offsets
         for (data, len, mock_offset) in cases_with(DATA_LENGTHS_BAD) {
             let fid = sharry::FileID::default();
-            let chunk = Chunk::new_direct(fid, mock_offset, data);
+            let chunk = unsafe { Chunk::new_unchecked(fid, mock_offset, data) };
 
             let repr_expect = format!(
                 "Chunk {{ file_id: {:?}, offset: {:?}, data.len(): {:?}, .. }}",

src/file/mod.rs

@@ -139,8 +139,7 @@ mod tests {
             // to capture progress updates from `compute_hash`
             let mut read_total = 0;
             let callback = |n| read_total += n;
-
-            let hash = compute_hash(file.path(), size, callback).expect("hash should succeed");
+            let hash = compute_hash(file.path(), size, callback).unwrap();
 
             assert_eq!(hash, expected_hash);
             assert_eq!(read_total, size);
@@ -151,21 +150,16 @@ mod tests {
     fn hash_size_mismatch() {
         for (content, good_size, bad_size) in cases_with(DATA_LENGTHS_BAD) {
             let file = create_file(content);
-            let callback = drop;
 
             {
-                let err = compute_hash(file.path(), bad_size, callback)
-                    .expect_err("compute_hash should report a mismatch");
-
-                // check error
+                // `compute_hash` with bad size
+                let err = compute_hash(file.path(), bad_size, drop).unwrap_err();
                 assert!(err.is_mismatch(bad_size.to_string(), good_size.to_string()));
             }
 
             {
-                let err = check_hash(file.path(), bad_size, Some("foobar"), callback)
-                    .expect_err("check_hash should report a mismatch");
-
-                // check error
+                // `check_hash` with bad size
+                let err = check_hash(file.path(), bad_size, Some("foobar"), drop).unwrap_err();
                 assert!(err.is_mismatch(bad_size.to_string(), good_size.to_string()));
             }
         }
@@ -175,12 +169,9 @@ mod tests {
     fn hash_value_none() {
         for (content, size) in cases() {
             let file = create_file(content);
-            let callback = drop;
 
-            let err = check_hash(file.path(), size, None, callback)
-                .expect_err("check_hash should report a mismatch");
-
-            // check error
+            // `check_hash` with no hash
+            let err = check_hash(file.path(), size, None, drop).unwrap_err();
             assert!(err.is_mismatch("hash", file.path().display().to_string()));
         }
     }
@@ -191,12 +182,9 @@ mod tests {
             cases_with(HASHES_STD_GOOD).zip(HASHES_STD_BAD)
         {
             let file = create_file(content);
-            let callback = drop;
 
-            let err = check_hash(file.path(), size, Some(bad_hash), callback)
-                .expect_err("check_hash should report a mismatch");
-
-            // check error
+            // `check_hash` with bad hash
+            let err = check_hash(file.path(), size, Some(bad_hash), drop).unwrap_err();
             assert!(err.is_mismatch(bad_hash, good_hash));
         }
     }

src/file/uploading.rs

@@ -14,7 +14,6 @@ use super::{Checked, Chunk, FileTrait};
 /// Description of a `file::Checked` that is actively being uploaded
 ///
 /// - impl `serde` for cachefile handling
-/// - impl `Into<Checked>` to abort an upload
 #[derive(Serialize, Deserialize, Debug)]
 pub struct Uploading {
     /// canonical path to a regular file
@@ -33,7 +32,8 @@ pub struct Uploading {
 }
 
 impl Uploading {
-    pub(super) fn new_direct(
+    /// create this directly, without any checks
+    pub(super) unsafe fn new_unchecked(
         path: PathBuf,
         size: u64,
         hash: Option<String>,
@@ -93,7 +93,8 @@ impl Uploading {
             ));
         }
 
-        let chunk = Chunk::new_direct(self.file_id.clone(), self.offset, &buf[..read_len]);
+        let chunk =
+            unsafe { Chunk::new_unchecked(self.file_id.clone(), self.offset, &buf[..read_len]) };
         self.previous_offset = Some(self.offset);
         self.offset += chunk.get_length();
 
@@ -117,7 +118,7 @@ impl Uploading {
     ///
     /// - consume self, returning as a `file::Checked`
     pub fn stop(self) -> Checked {
-        Checked::new_direct(self.path, self.size, self.hash)
+        unsafe { Checked::new_unchecked(self.path, self.size, self.hash) }
     }
 }
 
@@ -134,3 +135,82 @@ impl FileTrait for Uploading {
         super::check_hash(&self.path, self.size, self.hash.as_deref(), on_progress)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use tempfile::NamedTempFile;
+
+    use crate::{
+        sharry::{Client, json::NewShareRequest},
+        test_util::{MockClient, create_file, data::cases},
+    };
+
+    use super::*;
+
+    fn create_uploading(content: &[u8]) -> (Uploading, sharry::ShareID, NamedTempFile) {
+        let client = MockClient::default();
+        let uri = sharry::Uri::from(true);
+        let alias_id = sharry::AliasID::from(true);
+
+        let share_id = client
+            .share_create(&uri, &alias_id, NewShareRequest::new("share", 0))
+            .expect("");
+
+        let file = create_file(content);
+        let upl = Checked::new(file.path())
+            .unwrap()
+            .start_upload(&client, &uri, &alias_id, &share_id)
+            .unwrap();
+
+        // return all, so the `NamedTempFile` is not auto-deleted here
+        (upl, share_id, file)
+    }
+
+    #[test]
+    fn basic_tests() {
+        fn check_members(upl: &Uploading, path: &PathBuf, size: u64) {
+            assert_eq!(&upl.path, path);
+            assert_eq!(upl.size, size);
+            assert_eq!(upl.offset, 0);
+            assert!(upl.previous_offset.is_none());
+            assert!(upl.hash.is_none());
+        }
+
+        for (content, size) in cases() {
+            let (upl, _share_id, file) = create_uploading(content);
+            let path = file.path().canonicalize().unwrap();
+            check_members(&upl, &path, size);
+
+            // `get_offset`
+            assert_eq!(upl.get_offset(), upl.offset);
+
+            // `FileTrait`
+            assert_eq!(upl.get_name(), file.path().file_name().unwrap());
+            assert_eq!(upl.get_size(), size);
+            assert!(upl.check_hash(drop).is_err());
+
+            // `new_unchecked`
+            let upl =
+                unsafe { Uploading::new_unchecked(upl.path, upl.size, upl.hash, upl.file_id) };
+            check_members(&upl, &path, size);
+
+            // TODO into separate test
+            // // `check_eof`
+            // let upl = if size == 0 {
+            //     upl
+            // } else {
+            //     let eof = upl.check_eof();
+            //     assert!(eof.is_ok());
+            //     let upl = eof.unwrap();
+            //     check_members(&upl, &path, size);
+
+            //     upl
+            // };
+
+            // `stop`
+            let chk = upl.stop();
+            assert_eq!(chk.get_name(), path.file_name().unwrap().to_str().unwrap());
+            assert_eq!(chk.get_size(), size);
+        }
+    }
+}

src/lib.rs

@@ -1,15 +1,16 @@
 // TODO fix with documentation
 #![allow(clippy::missing_errors_doc)]
+#![allow(clippy::missing_panics_doc)]
 
 mod appstate;
 mod cachefile;
 mod cli;
 mod error;
 mod file;
-mod impl_ureq;
 pub mod output;
 mod sharry;
 mod test_util;
+mod ureq_client;
 
 pub use appstate::AppState;
 pub use cli::Cli;

src/output.rs

@@ -7,7 +7,7 @@ use log::{info, warn};
 
 type StaticStyled<'t> = LazyLock<StyledObject<&'t str>>;
 
-pub static SHRUPL: StaticStyled = LazyLock::new(|| style("ShrUpl").yellow().bold());
+pub const SHRUPL: StaticStyled = LazyLock::new(|| style("ShrUpl").yellow().bold());
 
 #[must_use]
 pub fn prompt_continue() -> bool {
@@ -61,7 +61,6 @@ where
 }
 
 #[must_use]
-#[allow(clippy::missing_panics_doc)]
 pub fn new_progressbar() -> ProgressBar {
     ProgressBar::no_length().with_style(
         ProgressStyle::with_template(&format!(

src/sharry/ids.rs

@@ -71,10 +71,6 @@ impl fmt::Display for FileID {
     }
 }
 
-impl TryFrom<String> for FileID {
-    type Error = crate::Error;
-
-    fn try_from(value: String) -> crate::Result<Self> {
 /// Pattern breakdown:
 /// - `^([^:/?#]+)://`          - scheme (anything but `:/?#`) + `"://"`
 /// - `([^/?#]+)`               - authority/host (anything but `/?#`)
@@ -83,15 +79,17 @@ impl TryFrom<String> for FileID {
 /// - `/files/tus/`             - literal path segment
 /// - `(?P<fid>[^/]+)`          - capture FID (one or more non-slash chars)
 /// - `$`                       - end of string
-        static UPLOAD_URL_RE: LazyLock<Regex> = LazyLock::new(|| {
+const UPLOAD_URL_RE: LazyLock<Regex> = LazyLock::new(|| {
     trace!("compiling UPLOAD_URL_RE");
 
-            Regex::new(
-                r"^([^:/?#]+)://([^/?#]+)/api/v2/alias/upload/[^/]+/files/tus/(?P<fid>[^/]+)$",
-            )
+    Regex::new(r"^([^:/?#]+)://([^/?#]+)/api/v2/alias/upload/[^/]+/files/tus/(?P<fid>[^/]+)$")
         .expect("Regex compilation failed")
 });
 
+impl TryFrom<String> for FileID {
+    type Error = crate::Error;
+
+    fn try_from(value: String) -> crate::Result<Self> {
         trace!("TryFrom {value:?}");
 
         if let Some(fid) = UPLOAD_URL_RE
@@ -171,12 +169,8 @@ mod tests {
         ];
 
         for (good, expected_fid) in cases {
-            let file_id =
-                FileID::try_from(good.to_string()).expect("URL should parse successfully");
-            assert_eq!(
-                file_id.0, expected_fid,
-                "Expected `{good}` → FileID({expected_fid}), got {file_id:?}",
-            );
+            let file_id = FileID::try_from(good.to_string()).unwrap();
+            assert_eq!(file_id.0, expected_fid);
         }
     }
 
@@ -196,7 +190,7 @@ mod tests {
         ];
 
         for bad in bad_inputs {
-            let err = FileID::try_from(bad.to_string()).expect_err("URL should not parse");
+            let err = FileID::try_from(bad.to_string()).unwrap_err();
             // make sure it's the Mismatch variant, and that it contains the original input
             assert!(err.is_mismatch(
                 "<proto>://<host>/api/v2/alias/upload/<share>/files/tus/<file>",

src/sharry/uri.rs

@@ -1,7 +1,7 @@
 use std::{fmt, sync::LazyLock};
 
 use log::{debug, trace};
-use regex::Regex;
+use regex::{Captures, Regex};
 use serde::{Deserialize, Serialize};
 
 /// ID of a file in a Sharry share
@@ -26,33 +26,33 @@ impl AsRef<[u8]> for Uri {
     }
 }
 
-impl From<String> for Uri {
-    fn from(value: String) -> Self {
-        fn parse_url(value: &str) -> Option<(String, String)> {
+fn captured(caps: &Captures, name: &str) -> String {
+    caps.name(name)
+        .unwrap_or_else(|| panic!("{name} not captured"))
+        .as_str()
+        .to_string()
+}
+
 /// Pattern breakdown:
 /// - `^(?P<scheme>[^:/?#]+)://`    - capture scheme (anything but `:/?#`) + `"://"`
 /// - `(?P<host>[^/?#]+)`           - capture authority/host (anything but `/?#`)
 /// - `(/.*)?`                      - maybe trailing slash and some path
 /// - `$`                           - end of string
-            static SHARRY_URI_RE: LazyLock<Regex> = LazyLock::new(|| {
+const SHARRY_URI_RE: LazyLock<Regex> = LazyLock::new(|| {
     trace!("compiling SHARRY_URI_RE");
 
     Regex::new(r"^(?P<scheme>[^:/?#]+)://(?P<host>[^/?#]+)(/.*)?$")
         .expect("Regex compilation failed")
 });
 
-            SHARRY_URI_RE.captures(value).map(|caps| {
-                let captured = |name| {
-                    caps.name(name)
-                        .unwrap_or_else(|| panic!("{name} not captured"))
-                        .as_str()
-                        .to_string()
-                };
-
-                (captured("scheme"), captured("host"))
-            })
+fn parse_url(value: &str) -> Option<(String, String)> {
+    SHARRY_URI_RE
+        .captures(value)
+        .map(|caps| (captured(&caps, "scheme"), captured(&caps, "host")))
 }
 
+impl From<String> for Uri {
+    fn from(value: String) -> Self {
         trace!("TryFrom {value:?}");
 
         if let Some((scheme, host)) = parse_url(&value) {

src/test_util/mock_client.rs

@@ -71,7 +71,7 @@ impl MockClient {
             .ok_or_else(|| error_response!("can't find share {share_id:?}!"))?;
 
         Ok(RefMut::map(shares, |shares| {
-            shares.get_mut(share_id).expect("checked but None!")
+            shares.get_mut(share_id).unwrap()
         }))
     }
 
@@ -90,17 +90,9 @@ impl MockClient {
             .ok_or_else(|| error_response!("can't find file {file_id:?}!"))?;
 
         Ok(RefMut::map(share, move |share| {
-            share.files.get_mut(file_id).expect("checked but None!")
+            share.files.get_mut(file_id).unwrap()
         }))
     }
-
-    pub fn add_share(&self) -> ShareID {
-        let share_id = ShareID::from(true);
-        self.insert_share(&share_id, MockShare::default())
-            .expect("should never fail");
-
-        share_id
-    }
 }
 
 impl Client for MockClient {

src/test_util/mock_ids.rs

@@ -142,8 +142,8 @@ mod tests {
     #[test]
     fn false_makes_invalids() {
         fn test_check(value: impl CheckID, callback: impl FnOnce(&Parameter) -> bool) {
-            let check = value.check().expect_err("should be invalid");
-            let p = check.get_invalid_param().expect("should be InvalidParam");
+            let check = value.check().unwrap_err();
+            let p = check.get_invalid_param().unwrap();
             assert!(callback(p));
         }
 

src/test_util/mod.rs

@@ -24,7 +24,7 @@ where
 
 /// Helper to create a temp file from `data`
 pub fn create_file(data: &[u8]) -> NamedTempFile {
-    let mut tmp = NamedTempFile::new().expect("creating temp file");
-    tmp.write_all(data).expect("writing to tempfile");
+    let mut tmp = NamedTempFile::new().unwrap();
+    tmp.write_all(data).unwrap();
     tmp
 }