From 6d7fbcadc4ef6f17e9b36436f542cf2c10e4d0af Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sun, 17 Apr 2022 11:56:36 +0200 Subject: [PATCH 1/5] feat(pdb): Mark table rows as serializable --- src/pdb/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pdb/mod.rs b/src/pdb/mod.rs index 8922d28..68c47a4 100644 --- a/src/pdb/mod.rs +++ b/src/pdb/mod.rs @@ -478,9 +478,9 @@ pub struct PlaylistTreeNodeId(pub u32); pub struct HistoryPlaylistId(pub u32); /// Contains the album name, along with an ID of the corresponding artist. -#[binread] +#[binrw] #[derive(Debug, PartialEq, Eq, Clone)] -#[br(little)] +#[brw(little)] pub struct Album { /// Position of start of this row (needed of offset calculations). /// @@ -932,9 +932,9 @@ impl BinWrite for Track { } /// A table row contains the actual data. -#[binread] +#[binrw] #[derive(Debug, PartialEq, Eq, Clone)] -#[br(little)] +#[brw(little)] #[br(import(page_type: PageType))] // The large enum size is unfortunate, but since users of this library will probably use iterators // to consume the results on demand, we can live with this. The alternative of using a `Box` would From faf2ce42327c2d8c3f106ea81cb98fe595ba99f4 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Fri, 22 Apr 2022 11:05:38 +0200 Subject: [PATCH 2/5] test(util): Add helper function for passing args to roundtrip tests --- src/util.rs | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/src/util.rs b/src/util.rs index abedcd0..a4a3ce7 100644 --- a/src/util.rs +++ b/src/util.rs @@ -70,28 +70,51 @@ pub(crate) mod testing { use binrw::{ meta::{ReadEndian, WriteEndian}, prelude::*, + Endian, ReadOptions, WriteOptions, }; - pub fn test_roundtrip(bin: &[u8], obj: T) - where - ::Args: Default, - ::Args: Default, + + pub fn test_roundtrip_with_args( + bin: &[u8], + obj: T, + read_args: ::Args, + write_args: ::Args, + ) where T: BinRead + BinWrite + PartialEq + core::fmt::Debug + ReadEndian + WriteEndian, { + let write_opts = WriteOptions::new(Endian::NATIVE); + let read_opts = ReadOptions::new(Endian::NATIVE); // T->binary let mut writer = binrw::io::Cursor::new(Vec::with_capacity(bin.len())); - obj.write(&mut writer).unwrap(); + obj.write_options(&mut writer, &write_opts, write_args.clone()) + .unwrap(); assert_eq!(bin, writer.get_ref()); // T->binary->T writer.set_position(0); - let parsed = T::read(&mut writer).unwrap(); + let parsed = T::read_options(&mut writer, &read_opts, read_args.clone()).unwrap(); assert_eq!(obj, parsed); // binary->T let mut cursor = binrw::io::Cursor::new(bin); - let parsed = T::read(&mut cursor).unwrap(); + let parsed = T::read_options(&mut cursor, &read_opts, read_args).unwrap(); assert_eq!(obj, parsed); // binary->T->binary writer.set_position(0); - parsed.write(&mut writer).unwrap(); + parsed + .write_options(&mut writer, &write_opts, write_args) + .unwrap(); assert_eq!(bin, writer.get_ref()); } + + pub fn test_roundtrip(bin: &[u8], obj: T) + where + ::Args: Default, + ::Args: Default, + T: BinRead + BinWrite + PartialEq + core::fmt::Debug + ReadEndian + WriteEndian, + { + test_roundtrip_with_args( + bin, + obj, + ::Args::default(), + ::Args::default(), + ); + } } From 202156bea51d8057266650609c0797c084d6e7d0 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Fri, 22 Apr 2022 11:30:36 +0200 Subject: [PATCH 3/5] test(util): Add additional length checks to roundtrip tests --- src/util.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util.rs b/src/util.rs index a4a3ce7..4610df4 100644 --- a/src/util.rs +++ b/src/util.rs @@ -87,6 +87,7 @@ pub(crate) mod testing { let mut writer = binrw::io::Cursor::new(Vec::with_capacity(bin.len())); obj.write_options(&mut writer, &write_opts, write_args.clone()) .unwrap(); + assert_eq!(bin.len(), writer.get_ref().len()); assert_eq!(bin, writer.get_ref()); // T->binary->T writer.set_position(0); @@ -101,6 +102,7 @@ pub(crate) mod testing { parsed .write_options(&mut writer, &write_opts, write_args) .unwrap(); + assert_eq!(bin.len(), writer.get_ref().len()); assert_eq!(bin, writer.get_ref()); } From 3f90b88e32ae2214fec33d7195f7e939262efc56 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 12 Aug 2023 20:04:34 +0200 Subject: [PATCH 4/5] chore(pdb): Use `brw()` for writable types --- src/pdb/mod.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/pdb/mod.rs b/src/pdb/mod.rs index 68c47a4..aa5f44c 100644 --- a/src/pdb/mod.rs +++ b/src/pdb/mod.rs @@ -426,55 +426,55 @@ impl RowGroup { /// Identifies a track. #[binrw] #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] -#[br(little)] +#[brw(little)] pub struct TrackId(pub u32); /// Identifies an artwork item. #[binrw] #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] -#[br(little)] +#[brw(little)] pub struct ArtworkId(pub u32); /// Identifies an album. #[binrw] #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] -#[br(little)] +#[brw(little)] pub struct AlbumId(pub u32); /// Identifies an artist. #[binrw] #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] -#[br(little)] +#[brw(little)] pub struct ArtistId(pub u32); /// Identifies a genre. #[binrw] #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] -#[br(little)] +#[brw(little)] pub struct GenreId(pub u32); /// Identifies a key. #[binrw] #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] -#[br(little)] +#[brw(little)] pub struct KeyId(pub u32); /// Identifies a label. #[binrw] #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] -#[br(little)] +#[brw(little)] pub struct LabelId(pub u32); /// Identifies a playlist tree node. #[binrw] #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] -#[br(little)] +#[brw(little)] pub struct PlaylistTreeNodeId(pub u32); /// Identifies a history playlist. #[binrw] #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] -#[br(little)] +#[brw(little)] pub struct HistoryPlaylistId(pub u32); /// Contains the album name, along with an ID of the corresponding artist. From 0e522e84a30191db214247de99391851077e0ef5 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sun, 13 Aug 2023 21:13:41 +0200 Subject: [PATCH 5/5] test(util): Print useful diffs when `assert_eq!` fails on large blobs --- Cargo.lock | 30 ++++++++++++++++++++++++++++++ Cargo.toml | 4 ++++ src/util.rs | 12 ++++++++++-- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 10a9275..0a54eae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -164,6 +164,12 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "338089f42c427b86394a5ee60ff321da23a5c89c9d89514c829687b26359fcff" +[[package]] +name = "diff" +version = "0.1.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" + [[package]] name = "either" version = "1.9.0" @@ -297,6 +303,22 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "pretty-hex" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c6fa0831dd7cc608c38a5e323422a0077678fa5744aa2be4ad91c4ece8eec8d5" + +[[package]] +name = "pretty_assertions" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af7cee1a6c8a5b9208b3cb1061f10c0cb689087b3d8ce85fb9d2dd7a29b6ba66" +dependencies = [ + "diff", + "yansi", +] + [[package]] name = "proc-macro2" version = "1.0.66" @@ -360,6 +382,8 @@ dependencies = [ "glob", "modular-bitfield", "parse-display", + "pretty-hex", + "pretty_assertions", "thiserror", ] @@ -530,3 +554,9 @@ name = "windows_x86_64_msvc" version = "0.48.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1a515f5799fe4961cb532f983ce2b23082366b898e52ffbce459c86f67c8378a" + +[[package]] +name = "yansi" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09041cd90cf85f7f8b2df60c646f853b7f535ce68f85244eb6731cf89fa498ec" diff --git a/Cargo.toml b/Cargo.toml index 39039ad..0ab2ba7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,10 @@ thiserror = "1.0" [build-dependencies] glob = "0.3" +[dev-dependencies] +pretty-hex = "0.3" +pretty_assertions = "1" + [features] default = ["cli"] cli = ["dep:clap"] diff --git a/src/util.rs b/src/util.rs index 4610df4..e07fc0d 100644 --- a/src/util.rs +++ b/src/util.rs @@ -72,6 +72,14 @@ pub(crate) mod testing { prelude::*, Endian, ReadOptions, WriteOptions, }; + use pretty_assertions::assert_eq; + use pretty_hex::pretty_hex; + + macro_rules! assert_eq_hex { + ($cond:expr, $expected:expr) => { + assert_eq!(pretty_hex($cond), pretty_hex($expected)); + }; + } pub fn test_roundtrip_with_args( bin: &[u8], @@ -88,7 +96,7 @@ pub(crate) mod testing { obj.write_options(&mut writer, &write_opts, write_args.clone()) .unwrap(); assert_eq!(bin.len(), writer.get_ref().len()); - assert_eq!(bin, writer.get_ref()); + assert_eq_hex!(&bin, &writer.get_ref()); // T->binary->T writer.set_position(0); let parsed = T::read_options(&mut writer, &read_opts, read_args.clone()).unwrap(); @@ -103,7 +111,7 @@ pub(crate) mod testing { .write_options(&mut writer, &write_opts, write_args) .unwrap(); assert_eq!(bin.len(), writer.get_ref().len()); - assert_eq!(bin, writer.get_ref()); + assert_eq_hex!(&bin, &writer.get_ref()); } pub fn test_roundtrip(bin: &[u8], obj: T)