Skip to content

Commit

Permalink
refactor: simplify Metrics type and update add_issue and add_metric m…
Browse files Browse the repository at this point in the history
…ethods to use EcoString

Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>
  • Loading branch information
simonsan committed Nov 21, 2024
1 parent 9611df5 commit 2265256
Showing 1 changed file with 21 additions and 19 deletions.
40 changes: 21 additions & 19 deletions crates/core/src/error/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,7 @@ pub type IssueIdentifier = EcoString;

pub type Issues = BTreeMap<IssueScope, BTreeMap<IssueIdentifier, CondensedIssue>>;

#[derive(Debug, Clone, derive_more::Display, derive_more::From, serde::Serialize)]
#[display("{metric}")]
pub enum MetricValue {
Int(i64),
Float(f64),
String(EcoString),
}

pub type Metrics = BTreeMap<EcoString, MetricValue>;
pub type Metrics = BTreeMap<EcoString, EcoString>;

#[derive(
Debug,
Expand Down Expand Up @@ -118,7 +110,7 @@ pub struct Summary {

impl Summary {
/// Constructor to create an initial empty Summary
pub fn new(context: &str) -> Self {
pub fn new(context: impl Into<EcoString>) -> Self {
Self {
context: context.into(),

Check warning on line 115 in crates/core/src/error/summary.rs

View check run for this annotation

Codecov / codecov/patch

crates/core/src/error/summary.rs#L115

Added line #L115 was not covered by tests
start_time: Instant::now(),
Expand All @@ -135,27 +127,37 @@ impl Summary {
}

/// Adds a new issue to the summary, condensing similar issues
pub fn add_issue(&mut self, scope: IssueScope, message: &str, root_cause: Option<&str>) {
pub fn add_issue(
&mut self,
scope: IssueScope,
message: impl Into<EcoString>,
root_cause: Option<impl Into<EcoString>>,
) {
let message = message.into();
let root_cause = root_cause.map(Into::into);

Check warning on line 137 in crates/core/src/error/summary.rs

View check run for this annotation

Codecov / codecov/patch

crates/core/src/error/summary.rs#L136-L137

Added lines #L136 - L137 were not covered by tests

_ = self
.issues
.entry(scope)
.or_default()
.entry(message.into())
.entry(message.clone())
.and_modify(|val| {

Check warning on line 144 in crates/core/src/error/summary.rs

View check run for this annotation

Codecov / codecov/patch

crates/core/src/error/summary.rs#L140-L144

Added lines #L140 - L144 were not covered by tests
val.count += 1;
if val.root_cause.is_none() {
val.root_cause = root_cause.map(Into::into);
val.root_cause.clone_from(&root_cause);

Check warning on line 147 in crates/core/src/error/summary.rs

View check run for this annotation

Codecov / codecov/patch

crates/core/src/error/summary.rs#L147

Added line #L147 was not covered by tests
}
})
.or_insert(CondensedIssue {
message: message.into(),
message,
count: 1,

Check warning on line 152 in crates/core/src/error/summary.rs

View check run for this annotation

Codecov / codecov/patch

crates/core/src/error/summary.rs#L152

Added line #L152 was not covered by tests
root_cause: root_cause.map(Into::into),
root_cause,
});
}

/// Adds a custom metric
pub fn add_metric(&mut self, key: &str, value: MetricValue) {
pub fn add_metric(&mut self, key: impl Into<EcoString>, value: impl Into<EcoString>) {
let value = value.into();

Check warning on line 159 in crates/core/src/error/summary.rs

View check run for this annotation

Codecov / codecov/patch

crates/core/src/error/summary.rs#L159

Added line #L159 was not covered by tests

_ = self
.metrics
.entry(key.into())
Expand Down Expand Up @@ -383,7 +385,7 @@ mod tests {
Some("Inconsistent state on disk"),
);

summary.add_metric("execution_time (sec)", 5.into());
summary.add_metric("execution_time", "5s");

summary.complete();

Expand All @@ -407,7 +409,7 @@ mod tests {
DisplayOptionKind::Metrics => {
assert!(display_output.contains("Metrics:"));

assert!(display_output.contains("execution_time (sec): 5"));
assert!(display_output.contains("execution_time: 5s"));

assert_snapshot!(display.to_string(), display_output);
}
Expand All @@ -422,7 +424,7 @@ mod tests {

assert!(display_output.contains("Metrics:"));

assert!(display_output.contains("execution_time (sec): 5"));
assert!(display_output.contains("execution_time: 5s"));
}
}
}
Expand Down

0 comments on commit 2265256

Please sign in to comment.