Skip to content

Commit

Permalink
Merge pull request #9 from opencomputeproject/fix/test_incorrect_asserts
Browse files Browse the repository at this point in the history
fix/test incorrect asserts
  • Loading branch information
mimir-d authored Oct 9, 2024
2 parents e164569 + b5f9fe2 commit d7e482d
Show file tree
Hide file tree
Showing 8 changed files with 287 additions and 127 deletions.
40 changes: 35 additions & 5 deletions src/output/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::output::emitter;

/// The configuration repository for the TestRun.
pub struct Config {
pub(crate) timezone: chrono_tz::Tz,
pub(crate) timestamp_provider: Box<dyn TimestampProvider + Send + Sync + 'static>,
pub(crate) writer: emitter::WriterType,
}

Expand All @@ -32,20 +32,28 @@ impl Config {

/// The builder for the [`Config`] object.
pub struct ConfigBuilder {
timezone: Option<chrono_tz::Tz>,
timestamp_provider: Box<dyn TimestampProvider + Send + Sync + 'static>,
writer: Option<emitter::WriterType>,
}

impl ConfigBuilder {
fn new() -> Self {
Self {
timezone: None,
timestamp_provider: Box::new(ConfiguredTzProvider { tz: chrono_tz::UTC }),
writer: Some(emitter::WriterType::Stdout(emitter::StdoutWriter::new())),
}
}

pub fn timezone(mut self, timezone: chrono_tz::Tz) -> Self {
self.timezone = Some(timezone);
self.timestamp_provider = Box::new(ConfiguredTzProvider { tz: timezone });
self
}

pub fn with_timestamp_provider(
mut self,
timestamp_provider: Box<dyn TimestampProvider + Send + Sync + 'static>,
) -> Self {
self.timestamp_provider = timestamp_provider;
self
}

Expand All @@ -68,10 +76,32 @@ impl ConfigBuilder {

pub fn build(self) -> Config {
Config {
timezone: self.timezone.unwrap_or(chrono_tz::UTC),
timestamp_provider: self.timestamp_provider,
writer: self
.writer
.unwrap_or(emitter::WriterType::Stdout(emitter::StdoutWriter::new())),
}
}
}

pub trait TimestampProvider {
fn now(&self) -> chrono::DateTime<chrono_tz::Tz>;
}

struct ConfiguredTzProvider {
tz: chrono_tz::Tz,
}

impl TimestampProvider for ConfiguredTzProvider {
fn now(&self) -> chrono::DateTime<chrono_tz::Tz> {
chrono::Local::now().with_timezone(&self.tz)
}
}

pub struct NullTimestampProvider {}

impl TimestampProvider for NullTimestampProvider {
fn now(&self) -> chrono::DateTime<chrono_tz::Tz> {
chrono::DateTime::from_timestamp_nanos(0).with_timezone(&chrono_tz::UTC)
}
}
25 changes: 17 additions & 8 deletions src/output/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use tokio::fs::File;
use tokio::io::AsyncWriteExt;
use tokio::sync::Mutex;

use crate::output::config;
use crate::spec;

#[derive(Debug, thiserror::Error, derive_more::Display)]
Expand Down Expand Up @@ -84,26 +85,28 @@ impl StdoutWriter {
}

pub struct JsonEmitter {
timezone: chrono_tz::Tz,
// HACK: public for tests, but this should come from config directly to where needed
pub(crate) timestamp_provider: Box<dyn config::TimestampProvider + Send + Sync + 'static>,
writer: WriterType,
seqno: Arc<atomic::AtomicU64>,
}

impl JsonEmitter {
pub(crate) fn new(timezone: chrono_tz::Tz, writer: WriterType) -> Self {
pub(crate) fn new(
timestamp_provider: Box<dyn config::TimestampProvider + Send + Sync + 'static>,
writer: WriterType,
) -> Self {
JsonEmitter {
timezone,
timestamp_provider,
writer,
seqno: Arc::new(atomic::AtomicU64::new(0)),
}
}

fn serialize_artifact(&self, object: &spec::RootImpl) -> serde_json::Value {
let now = chrono::Local::now();
let now_tz = now.with_timezone(&self.timezone);
let root = spec::Root {
artifact: object.clone(),
timestamp: now_tz,
timestamp: self.timestamp_provider.now(),
seqno: self.incr_seqno(),
};
serde_json::json!(root)
Expand Down Expand Up @@ -144,7 +147,10 @@ mod tests {

let buffer = Arc::new(Mutex::new(vec![]));
let writer = BufferWriter::new(buffer.clone());
let emitter = JsonEmitter::new(chrono_tz::UTC, WriterType::Buffer(writer));
let emitter = JsonEmitter::new(
Box::new(config::NullTimestampProvider {}),
WriterType::Buffer(writer),
);

emitter
.emit(&spec::RootImpl::SchemaVersion(
Expand Down Expand Up @@ -179,7 +185,10 @@ mod tests {

let buffer = Arc::new(Mutex::new(vec![]));
let writer = BufferWriter::new(buffer.clone());
let emitter = JsonEmitter::new(chrono_tz::UTC, WriterType::Buffer(writer));
let emitter = JsonEmitter::new(
Box::new(config::NullTimestampProvider {}),
WriterType::Buffer(writer),
);

let version = spec::RootImpl::SchemaVersion(spec::SchemaVersion::default());
emitter.emit(&version).await?;
Expand Down
34 changes: 9 additions & 25 deletions src/output/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ impl ErrorBuilder {
#[cfg(test)]
mod tests {
use anyhow::Result;

use assert_json_diff::assert_json_include;
use assert_json_diff::assert_json_eq;

use super::*;
use crate::output as tv;
Expand Down Expand Up @@ -140,45 +139,30 @@ mod tests {
"message": "message",
"softwareInfoIds": [
{
"computerSystem": null,
"name": "name",
"revision": null,
"softwareInfoId":
"software_id",
"softwareType": null,
"version": null
"softwareInfoId": "software_id",
},
{
"computerSystem": null,
"name": "name",
"revision": null,
"softwareInfoId":
"software_id",
"softwareType": null,
"version": null
"softwareInfoId": "software_id",
}
],
"sourceLocation": {"file": "file.rs", "line": 1},
"sourceLocation": {
"file": "file.rs",
"line": 1
},
"symptom": "symptom"
});
let expected_step = serde_json::json!({
"message": "message",
"softwareInfoIds": [
{
"computerSystem": null,
"name": "name",
"revision": null,
"softwareInfoId": "software_id",
"softwareType": null,
"version": null
},
{
"computerSystem": null,
"name": "name",
"revision": null,
"softwareInfoId": "software_id",
"softwareType": null,
"version": null
}
],
"sourceLocation": {"file":"file.rs","line":1},
Expand All @@ -195,11 +179,11 @@ mod tests {

let spec_error = error.to_artifact();
let actual = serde_json::json!(spec_error);
assert_json_include!(actual: actual, expected: &expected_run);
assert_json_eq!(actual, expected_run);

let spec_error = error.to_artifact();
let actual = serde_json::json!(spec_error);
assert_json_include!(actual: actual, expected: &expected_step);
assert_json_eq!(actual, expected_step);

Ok(())
}
Expand Down
22 changes: 2 additions & 20 deletions src/output/measure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ impl StartedMeasurementSeries {
let element = spec::MeasurementSeriesElement {
index: self.incr_seqno(),
value: value.clone(),
timestamp: chrono::Local::now().with_timezone(&chrono_tz::Tz::UTC),
timestamp: self.parent.emitter.timestamp_provider().now(),
series_id: self.parent.start.series_id.clone(),
metadata: None,
};
Expand Down Expand Up @@ -223,7 +223,7 @@ impl StartedMeasurementSeries {
let element = spec::MeasurementSeriesElement {
index: self.incr_seqno(),
value: value.clone(),
timestamp: chrono::Local::now().with_timezone(&chrono_tz::Tz::UTC),
timestamp: self.parent.emitter.timestamp_provider().now(),
series_id: self.parent.start.series_id.clone(),
metadata: Some(Map::from_iter(
metadata.iter().map(|(k, v)| (k.to_string(), v.clone())),
Expand Down Expand Up @@ -737,24 +737,6 @@ impl MeasurementSeriesStartBuilder {
}
}

// pub struct MeasurementSeriesEmitter {
// series_id: String,
// step_emitter: Arc<step::StepEmitter>,
// }

// impl StepEmitter {
// pub async fn emit(&self, object: &spec::TestStepArtifactImpl) -> Result<(), WriterError> {
// let root = spec::RootImpl::TestStepArtifact(spec::TestStepArtifact {
// id: self.step_id.clone(),
// // TODO: can these copies be avoided?
// artifact: object.clone(),
// });
// self.run_emitter.emit(&root).await?;

// Ok(())
// }
// }

#[cfg(test)]
mod tests {
use super::*;
Expand Down
2 changes: 1 addition & 1 deletion src/output/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ impl TestRunBuilder {

pub fn build(self) -> TestRun {
let config = self.config.unwrap_or(config::Config::builder().build());
let emitter = emitter::JsonEmitter::new(config.timezone, config.writer);
let emitter = emitter::JsonEmitter::new(config.timestamp_provider, config.writer);

TestRun {
name: self.name,
Expand Down
6 changes: 5 additions & 1 deletion src/output/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ use crate::spec::{self, TestStepArtifactImpl};
use tv::measure::MeasurementSeries;
use tv::{emitter, error, log, measure};

use super::JsonEmitter;
use super::WriterError;
use super::{JsonEmitter, TimestampProvider};

/// A single test step in the scope of a [`TestRun`].
///
Expand Down Expand Up @@ -515,4 +515,8 @@ impl StepEmitter {

Ok(())
}

pub fn timestamp_provider(&self) -> &dyn TimestampProvider {
&*self.run_emitter.timestamp_provider
}
}
Loading

0 comments on commit d7e482d

Please sign in to comment.