Skip to content

Commit 6d30600

Browse files
authored
[7/n] [sled-agent-zone-images] factor out error PartialEq comparisons (#8189)
Need to use them in other places, and having these wrappers means big `PartialEq` implementations don't have to be written out by hand.
1 parent 02d8cd5 commit 6d30600

File tree

1 file changed

+49
-66
lines changed

1 file changed

+49
-66
lines changed

sled-agent/zone-images/src/mupdate_override.rs

Lines changed: 49 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
99
use std::fs;
1010
use std::fs::FileType;
11+
use std::io;
1112
use std::sync::Arc;
1213

1314
use crate::ZoneImageZpools;
@@ -177,7 +178,7 @@ fn read_mupdate_override(
177178
fs::symlink_metadata(dataset_dir).map_err(|error| {
178179
MupdateOverrideReadError::DatasetDirMetadata {
179180
dataset_dir: dataset_dir.to_owned(),
180-
error: Arc::new(error),
181+
error: ArcIoError::new(error),
181182
}
182183
})?;
183184
if !dir_metadata.is_dir() {
@@ -193,7 +194,7 @@ fn read_mupdate_override(
193194
.map_err(|error| {
194195
MupdateOverrideReadError::Deserialize {
195196
path: override_path.to_owned(),
196-
error: Arc::new(error),
197+
error: ArcSerdeJsonError::new(error),
197198
contents: data,
198199
}
199200
})?;
@@ -210,7 +211,7 @@ fn read_mupdate_override(
210211
} else {
211212
return Err(MupdateOverrideReadError::Read {
212213
path: override_path.to_owned(),
213-
error: Arc::new(error),
214+
error: ArcIoError::new(error),
214215
});
215216
}
216217
}
@@ -398,7 +399,7 @@ impl MupdateOverrideNonBootMismatch {
398399
}
399400
}
400401

401-
#[derive(Clone, Debug, Error)]
402+
#[derive(Clone, Debug, Error, PartialEq)]
402403
enum MupdateOverrideReadError {
403404
#[error(
404405
"error retrieving metadata for install dataset directory \
@@ -407,7 +408,7 @@ enum MupdateOverrideReadError {
407408
DatasetDirMetadata {
408409
dataset_dir: Utf8PathBuf,
409410
#[source]
410-
error: Arc<std::io::Error>,
411+
error: ArcIoError,
411412
},
412413

413414
#[error(
@@ -420,7 +421,7 @@ enum MupdateOverrideReadError {
420421
Read {
421422
path: Utf8PathBuf,
422423
#[source]
423-
error: Arc<std::io::Error>,
424+
error: ArcIoError,
424425
},
425426

426427
#[error(
@@ -431,67 +432,47 @@ enum MupdateOverrideReadError {
431432
path: Utf8PathBuf,
432433
contents: String,
433434
#[source]
434-
error: Arc<serde_json::Error>,
435+
error: ArcSerdeJsonError,
435436
},
436437
}
437438

438-
/// This aids tremendously in testing.
439-
///
440-
/// `MupdateOverrideReadError` forms an equivalence class (i.e. it satisfies the
441-
/// reflexive property `a == a`), so it should in principle be okay to implement
442-
/// `Eq` for it as well. But this algorithm doesn't fully compare for equality,
443-
/// just enough for tests, so we don't implement `Eq`.
444-
///
445-
/// (But if there's a use case for `Eq` in the future, we should consider
446-
/// implementing it.)
447-
impl PartialEq for MupdateOverrideReadError {
439+
/// An `io::Error` wrapper that implements `Clone` and `PartialEq`.
440+
#[derive(Clone, Debug, Error)]
441+
#[error(transparent)]
442+
struct ArcIoError(Arc<io::Error>);
443+
444+
impl ArcIoError {
445+
fn new(error: io::Error) -> Self {
446+
Self(Arc::new(error))
447+
}
448+
}
449+
450+
/// Testing aid.
451+
impl PartialEq for ArcIoError {
448452
fn eq(&self, other: &Self) -> bool {
449-
match (self, other) {
450-
(
451-
Self::DatasetDirMetadata { dataset_dir: dir1, error: error1 },
452-
Self::DatasetDirMetadata { dataset_dir: dir2, error: error2 },
453-
) => {
454-
// Simply comparing io::ErrorKind is good enough for tests.
455-
dir1 == dir2 && error1.kind() == error2.kind()
456-
}
457-
(
458-
Self::DatasetNotDirectory {
459-
dataset_dir: dir1,
460-
file_type: type1,
461-
},
462-
Self::DatasetNotDirectory {
463-
dataset_dir: dir2,
464-
file_type: type2,
465-
},
466-
) => dir1 == dir2 && type1 == type2,
467-
(
468-
Self::Read { path: path1, error: error1 },
469-
Self::Read { path: path2, error: error2 },
470-
) => {
471-
// Simply comparing io::ErrorKind is good enough for tests.
472-
path1 == path2 && error1.kind() == error2.kind()
473-
}
474-
(
475-
Self::Deserialize {
476-
path: path1,
477-
contents: contents1,
478-
error: error1,
479-
},
480-
Self::Deserialize {
481-
path: path2,
482-
contents: contents2,
483-
error: error2,
484-
},
485-
) => {
486-
// Comparing error line/column/category is enough for tests.
487-
path1 == path2
488-
&& contents1 == contents2
489-
&& error1.line() == error2.line()
490-
&& error1.column() == error2.column()
491-
&& error1.classify() == error2.classify()
492-
}
493-
_ => false,
494-
}
453+
// Simply comparing io::ErrorKind is good enough for tests.
454+
self.0.kind() == other.0.kind()
455+
}
456+
}
457+
458+
/// A `serde_json::Error` that implements `Clone` and `PartialEq`.
459+
#[derive(Clone, Debug, Error)]
460+
#[error(transparent)]
461+
struct ArcSerdeJsonError(Arc<serde_json::Error>);
462+
463+
impl ArcSerdeJsonError {
464+
fn new(error: serde_json::Error) -> Self {
465+
Self(Arc::new(error))
466+
}
467+
}
468+
469+
/// Testing aid.
470+
impl PartialEq for ArcSerdeJsonError {
471+
fn eq(&self, other: &Self) -> bool {
472+
// Simply comparing line/column/category is good enough for tests.
473+
self.0.line() == other.0.line()
474+
&& self.0.column() == other.0.column()
475+
&& self.0.classify() == other.0.classify()
495476
}
496477
}
497478

@@ -983,7 +964,9 @@ mod tests {
983964
fn dataset_missing_error(dir_path: &Utf8Path) -> MupdateOverrideReadError {
984965
MupdateOverrideReadError::DatasetDirMetadata {
985966
dataset_dir: dir_path.to_owned(),
986-
error: Arc::new(io::Error::from(io::ErrorKind::NotFound)),
967+
error: ArcIoError(Arc::new(io::Error::from(
968+
io::ErrorKind::NotFound,
969+
))),
987970
}
988971
}
989972

@@ -1007,10 +990,10 @@ mod tests {
1007990
MupdateOverrideReadError::Deserialize {
1008991
path: dir_path.join(json_path),
1009992
contents: contents.to_owned(),
1010-
error: Arc::new(
993+
error: ArcSerdeJsonError(Arc::new(
1011994
serde_json::from_str::<MupdateOverrideInfo>(contents)
1012995
.unwrap_err(),
1013-
),
996+
)),
1014997
}
1015998
}
1016999
}

0 commit comments

Comments
 (0)