Merge pull request #457 from sftse/tests

fix broken tests

.github/workflows/rust.yml

@@ -27,7 +27,7 @@ jobs:
         cargo build
     - name: Run tests
       run: |
-        cargo test --features dates
+        cargo test --all-features
     - name: Install rustfmt
       uses: dtolnay/rust-toolchain@master
       if: ${{ matrix.toolchain == 'stable' }}

Cargo.toml

@@ -29,7 +29,7 @@ chrono = { version = "0.4", features = [
 glob = "0.3"
 env_logger = "0.11"
 serde_derive = "1.0"
-sha256 = "1.3"
+sha2 = "0.10.8"
 
 [features]
 default = []

fuzz/fuzz_targets/fuzz_all.rs

@@ -16,7 +16,7 @@ fuzz_target!(|data: &[u8]| {
         Err(_) => return,
     };
     for worksheet in workbook.worksheets() {
-        if let Some(Ok(range)) = workbook.worksheet_range(&worksheet.0) {
+        if let Ok(range) = workbook.worksheet_range(&worksheet.0) {
            let _ = range.get_size().0 * range.get_size().1;
            range.used_cells().count();
         }
@@ -33,7 +33,7 @@ fuzz_target!(|data: &[u8]| {
     }
     let sheets = workbook.sheet_names().to_owned();
     for s in sheets {
-        if let Some(Ok(formula)) = workbook.worksheet_formula(&s) {
+        if let Ok(formula) = workbook.worksheet_formula(&s) {
             formula.rows().flat_map(|r| r.iter().filter(|f| !f.is_empty())).count();
         }
     }

src/cfb.rs

@@ -2,7 +2,6 @@
 
 use std::borrow::Cow;
 use std::cmp::min;
-use std::convert::TryInto;
 use std::io::Read;
 
 use log::debug;

src/datatype.rs

@@ -3,7 +3,7 @@ use std::fmt;
 use std::sync::OnceLock;
 
 use serde::de::Visitor;
-use serde::{self, Deserialize};
+use serde::Deserialize;
 
 use super::CellErrorType;
 
@@ -596,8 +596,7 @@ pub trait DataType {
         if self.is_datetime_iso() {
             self.as_datetime().map(|dt| dt.date()).or_else(|| {
                 self.get_datetime_iso()
-                    .map(|s| chrono::NaiveDate::from_str(&s).ok())
-                    .flatten()
+                    .and_then(|s| chrono::NaiveDate::from_str(s).ok())
             })
         } else {
             self.as_datetime().map(|dt| dt.date())
@@ -611,13 +610,11 @@ pub trait DataType {
         if self.is_datetime_iso() {
             self.as_datetime().map(|dt| dt.time()).or_else(|| {
                 self.get_datetime_iso()
-                    .map(|s| chrono::NaiveTime::from_str(&s).ok())
-                    .flatten()
+                    .and_then(|s| chrono::NaiveTime::from_str(s).ok())
             })
         } else if self.is_duration_iso() {
             self.get_duration_iso()
-                .map(|s| chrono::NaiveTime::parse_from_str(&s, "PT%HH%MM%S%.fS").ok())
-                .flatten()
+                .and_then(|s| chrono::NaiveTime::parse_from_str(s, "PT%HH%MM%S%.fS").ok())
         } else {
             self.as_datetime().map(|dt| dt.time())
         }
@@ -629,7 +626,7 @@ pub trait DataType {
         use chrono::Timelike;
 
         if self.is_datetime() {
-            self.get_datetime().map(|dt| dt.as_duration()).flatten()
+            self.get_datetime().and_then(|dt| dt.as_duration())
         } else if self.is_duration_iso() {
             // need replace in the future to smth like chrono::Duration::from_str()
             // https://github.com/chronotope/chrono/issues/579
@@ -655,7 +652,7 @@ pub trait DataType {
             self.get_datetime().map(|d| d.as_datetime())
         } else if self.is_datetime_iso() {
             self.get_datetime_iso()
-                .map(|s| chrono::NaiveDateTime::from_str(&s).ok())
+                .map(|s| chrono::NaiveDateTime::from_str(s).ok())
         } else {
             None
         }

src/de.rs

@@ -1,6 +1,6 @@
 use serde::de::value::BorrowedStrDeserializer;
 use serde::de::{self, DeserializeOwned, DeserializeSeed, SeqAccess, Visitor};
-use serde::{self, forward_to_deserialize_any, Deserialize, Deserializer};
+use serde::{forward_to_deserialize_any, Deserialize, Deserializer};
 use std::marker::PhantomData;
 use std::{fmt, slice, str};
 

src/lib.rs

@@ -468,7 +468,7 @@ impl<T: CellType> Range<T> {
             // search bounds
             let row_start = cells.first().unwrap().pos.0;
             let row_end = cells.last().unwrap().pos.0;
-            let mut col_start = std::u32::MAX;
+            let mut col_start = u32::MAX;
             let mut col_end = 0;
             for c in cells.iter().map(|c| c.pos.1) {
                 if c < col_start {

src/ods.rs

@@ -284,7 +284,7 @@ fn parse_content<RS: Read + Seek>(mut zip: ZipArchive<RS>) -> Result<Content, Od
                     .map(|x| x.to_string())
             }
             Ok(Event::Start(ref e))
-                if style_name.clone().is_some() && e.name() == QName(b"style:table-properties") =>
+                if style_name.is_some() && e.name() == QName(b"style:table-properties") =>
             {
                 let visible = match e.try_get_attribute(b"table:display")? {
                     Some(a) => match a
@@ -309,7 +309,7 @@ fn parse_content<RS: Read + Seek>(mut zip: ZipArchive<RS>) -> Result<Content, Od
                             .map_err(OdsError::Xml)?
                             .map(|x| x.to_string()),
                     )
-                    .map(|v| v.to_owned())
+                    .cloned()
                     .unwrap_or(SheetVisible::Visible);
                 if let Some(ref a) = e
                     .attributes()
@@ -699,20 +699,20 @@ fn read_pictures<RS: Read + Seek>(
     let mut pics = Vec::new();
     for i in 0..zip.len() {
         let mut zfile = zip.by_index(i)?;
-        let zname = zfile.name().to_owned();
+        let zname = zfile.name();
         // no Thumbnails
         if zname.starts_with("Pictures") {
-            let name_ext: Vec<&str> = zname.split(".").collect();
-            if let Some(ext) = name_ext.last() {
+            if let Some(ext) = zname.split('.').last() {
                 if [
                     "emf", "wmf", "pict", "jpeg", "jpg", "png", "dib", "gif", "tiff", "eps", "bmp",
                     "wpg",
                 ]
-                .contains(ext)
+                .contains(&ext)
                 {
+                    let ext = ext.to_string();
                     let mut buf: Vec<u8> = Vec::new();
                     zfile.read_to_end(&mut buf)?;
-                    pics.push((ext.to_string(), buf));
+                    pics.push((ext, buf));
                 }
             }
         }

src/utils.rs

@@ -1,7 +1,5 @@
 //! Internal module providing handy function
 
-use std::convert::TryInto;
-
 macro_rules! from_err {
     ($from:ty, $to:tt, $var:tt) => {
         impl From<$from> for $to {

src/xls.rs

@@ -1,7 +1,6 @@
 use std::borrow::Cow;
 use std::cmp::min;
 use std::collections::BTreeMap;
-use std::convert::TryInto;
 use std::fmt::Write;
 use std::io::{Read, Seek, SeekFrom};
 use std::marker::PhantomData;
@@ -575,14 +574,8 @@ fn parse_sheet_metadata(
         }
     };
     r.data = &r.data[6..];
-    let name = parse_short_string(r, encoding, biff)?;
-    let sheet_name = name
-        .as_bytes()
-        .iter()
-        .cloned()
-        .filter(|b| *b != 0)
-        .collect::<Vec<_>>();
-    let name = String::from_utf8(sheet_name).unwrap();
+    let mut name = parse_short_string(r, encoding, biff)?;
+    name.retain(|c| c != '\0');
     Ok((pos, Sheet { name, visible, typ }))
 }
 

src/xlsb/mod.rs

@@ -5,7 +5,6 @@ pub use cells_reader::XlsbCellsReader;
 use std::borrow::Cow;
 use std::collections::BTreeMap;
 use std::io::{BufReader, Read, Seek};
-use std::string::String;
 
 use log::debug;
 
@@ -410,19 +409,19 @@ impl<RS: Read + Seek> Xlsb<RS> {
         let mut pics = Vec::new();
         for i in 0..self.zip.len() {
             let mut zfile = self.zip.by_index(i)?;
-            let zname = zfile.name().to_owned();
+            let zname = zfile.name();
             if zname.starts_with("xl/media") {
-                let name_ext: Vec<&str> = zname.split(".").collect();
-                if let Some(ext) = name_ext.last() {
+                if let Some(ext) = zname.split('.').last() {
                     if [
                         "emf", "wmf", "pict", "jpeg", "jpg", "png", "dib", "gif", "tiff", "eps",
                         "bmp", "wpg",
                     ]
-                    .contains(ext)
+                    .contains(&ext)
                     {
+                        let ext = ext.to_string();
                         let mut buf: Vec<u8> = Vec::new();
                         zfile.read_to_end(&mut buf)?;
-                        pics.push((ext.to_string(), buf));
+                        pics.push((ext, buf));
                     }
                 }
             }

src/xlsx/mod.rs

@@ -507,11 +507,8 @@ impl<RS: Read + Seek> Xlsx<RS> {
                                     // this is an incomplete implementation, but should be good enough for excel
                                     let new_index =
                                         base_folder.rfind('/').expect("Must be a parent folder");
-                                    let full_path = format!(
-                                        "{}{}",
-                                        base_folder[..new_index].to_owned(),
-                                        target[2..].to_owned()
-                                    );
+                                    let full_path =
+                                        format!("{}{}", &base_folder[..new_index], &target[2..]);
                                     table_locations.push(full_path);
                                 } else if target.is_empty() { // do nothing
                                 } else {
@@ -622,19 +619,19 @@ impl<RS: Read + Seek> Xlsx<RS> {
         let mut pics = Vec::new();
         for i in 0..self.zip.len() {
             let mut zfile = self.zip.by_index(i)?;
-            let zname = zfile.name().to_owned();
+            let zname = zfile.name();
             if zname.starts_with("xl/media") {
-                let name_ext: Vec<&str> = zname.split(".").collect();
-                if let Some(ext) = name_ext.last() {
+                if let Some(ext) = zname.split('.').last() {
                     if [
                         "emf", "wmf", "pict", "jpeg", "jpg", "png", "dib", "gif", "tiff", "eps",
                         "bmp", "wpg",
                     ]
-                    .contains(ext)
+                    .contains(&ext)
                     {
+                        let ext = ext.to_string();
                         let mut buf: Vec<u8> = Vec::new();
                         zfile.read_to_end(&mut buf)?;
-                        pics.push((ext.to_string(), buf));
+                        pics.push((ext, buf));
                     }
                 }
             }

tests/test.rs

@@ -1097,7 +1097,7 @@ fn merged_regions_xlsx() {
 
 #[test]
 fn issue_252() {
-    let path = format!("issue252.xlsx");
+    let path = "issue252.xlsx";
 
     // should err, not panic
     assert!(open_workbook::<Xls<_>, _>(&path).is_err());
@@ -1227,25 +1227,34 @@ fn issue_305_merge_cells_xls() {
     );
 }
 
+#[cfg(feature = "picture")]
+fn digest(data: &[u8]) -> [u8; 32] {
+    use sha2::digest::Digest;
+    let mut hasher = sha2::Sha256::new();
+    hasher.update(data);
+    hasher.finalize().into()
+}
+
 // cargo test --features picture
 #[test]
 #[cfg(feature = "picture")]
 fn pictures() -> Result<(), calamine::Error> {
-    let jpg_path = format!("picture.jpg");
-    let png_path = format!("picture.png");
+    let path = |name: &str| format!("{}/tests/{name}", env!("CARGO_MANIFEST_DIR"));
+    let jpg_path = path("picture.jpg");
+    let png_path = path("picture.png");
 
-    let xlsx_path = format!("picture.xlsx");
-    let xlsb_path = format!("picture.xlsb");
-    let xls_path = format!("picture.xls");
-    let ods_path = format!("picture.ods");
+    let xlsx_path = "picture.xlsx";
+    let xlsb_path = "picture.xlsb";
+    let xls_path = "picture.xls";
+    let ods_path = "picture.ods";
 
-    let jpg_hash = sha256::digest(&*std::fs::read(&jpg_path)?);
-    let png_hash = sha256::digest(&*std::fs::read(&png_path)?);
+    let jpg_hash = digest(&std::fs::read(jpg_path)?);
+    let png_hash = digest(&std::fs::read(png_path)?);
 
-    let xlsx: Xlsx<_> = wb(xlsx_path)?;
-    let xlsb: Xlsb<_> = wb(xlsb_path)?;
-    let xls: Xls<_> = wb(xls_path)?;
-    let ods: Ods<_> = wb(ods_path)?;
+    let xlsx: Xlsx<_> = wb(xlsx_path);
+    let xlsb: Xlsb<_> = wb(xlsb_path);
+    let xls: Xls<_> = wb(xls_path);
+    let ods: Ods<_> = wb(ods_path);
 
     let mut pictures = Vec::with_capacity(8);
     let mut pass = 0;
@@ -1263,7 +1272,7 @@ fn pictures() -> Result<(), calamine::Error> {
         pictures.extend(pics);
     }
     for (ext, data) in pictures {
-        let pic_hash = sha256::digest(&*data);
+        let pic_hash = digest(&data);
         if ext == "jpg" || ext == "jpeg" {
             assert_eq!(jpg_hash, pic_hash);
         } else if ext == "png" {