From 2e12ab177a7cf2a290de900167d01f2cdd003aa5 Mon Sep 17 00:00:00 2001 From: mimir-d Date: Sat, 5 Oct 2024 20:18:40 +0100 Subject: [PATCH 1/3] add non_exhaustive to public enums - this is needed in case the spec adds new variants; otherwise we break consumers of these enums that may have matched exhaustively Signed-off-by: mimir-d --- src/spec.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/spec.rs b/src/spec.rs index d60baae..dc91774 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -103,6 +103,7 @@ pub enum TestStepArtifactDescendant { } #[derive(Debug, Serialize, Clone, PartialEq)] +#[non_exhaustive] pub enum ValidatorType { #[serde(rename = "EQUAL")] Equal, @@ -175,6 +176,7 @@ pub enum DiagnosisType { /// schema ref: https://github.com/opencomputeproject/ocp-diag-core/testStatus #[derive(Debug, Serialize, Clone, PartialEq)] #[serde(rename = "testStatus")] +#[non_exhaustive] pub enum TestStatus { #[serde(rename = "COMPLETE")] Complete, @@ -190,6 +192,7 @@ pub enum TestStatus { /// schema ref: https://github.com/opencomputeproject/ocp-diag-core/testRunEnd/$defs/testResult #[derive(Debug, Serialize, Clone, PartialEq)] #[serde(rename = "testResult")] +#[non_exhaustive] pub enum TestResult { #[serde(rename = "PASS")] Pass, @@ -198,11 +201,13 @@ pub enum TestResult { #[serde(rename = "NOT_APPLICABLE")] NotApplicable, } + /// Known log severity variants. /// ref: https://github.com/opencomputeproject/ocp-diag-core/tree/main/json_spec#severity /// schema url: https://github.com/opencomputeproject/ocp-diag-core/blob/main/json_spec/output/log.json /// schema ref: https://github.com/opencomputeproject/ocp-diag-core/log/$defs/severity #[derive(Debug, Serialize, Clone, PartialEq)] +#[non_exhaustive] pub enum LogSeverity { #[serde(rename = "DEBUG")] Debug, From f7a5696e12fbd54f639d5b2330998e28665d4e80 Mon Sep 17 00:00:00 2001 From: mimir-d Date: Sat, 5 Oct 2024 22:04:04 +0100 Subject: [PATCH 2/3] current impl was missing testStepId; add it - note that the tests are not catching the fact that this new field is missing from the expected output, because all the tests are doing an "included" test, rather than the correct equality; fixing this in a later commit Signed-off-by: mimir-d --- src/output/emitter.rs | 8 +- src/output/measurement.rs | 282 +++++++++++++++++--------------------- src/output/run.rs | 15 +- src/output/step.rs | 146 ++++++++++---------- src/spec.rs | 3 + tests/output/runner.rs | 11 +- 6 files changed, 230 insertions(+), 235 deletions(-) diff --git a/src/output/emitter.rs b/src/output/emitter.rs index e26acd2..9bfd8bd 100644 --- a/src/output/emitter.rs +++ b/src/output/emitter.rs @@ -84,9 +84,9 @@ impl StdoutWriter { } pub struct JsonEmitter { - sequence_no: Arc, timezone: chrono_tz::Tz, writer: WriterType, + seqno: Arc, } impl JsonEmitter { @@ -94,7 +94,7 @@ impl JsonEmitter { JsonEmitter { timezone, writer, - sequence_no: Arc::new(atomic::AtomicU64::new(0)), + seqno: Arc::new(atomic::AtomicU64::new(0)), } } @@ -110,8 +110,8 @@ impl JsonEmitter { } fn next_sequence_no(&self) -> u64 { - self.sequence_no.fetch_add(1, atomic::Ordering::SeqCst); - self.sequence_no.load(atomic::Ordering::SeqCst) + self.seqno.fetch_add(1, atomic::Ordering::SeqCst); + self.seqno.load(atomic::Ordering::SeqCst) } pub async fn emit(&self, object: &spec::RootArtifact) -> Result<(), WriterError> { diff --git a/src/output/measurement.rs b/src/output/measurement.rs index 249df84..a874bd9 100644 --- a/src/output/measurement.rs +++ b/src/output/measurement.rs @@ -15,22 +15,25 @@ use tokio::sync::Mutex; use crate::output as tv; use crate::spec; -use tv::{dut, emitter, state}; +use tv::{dut, emitter, step}; /// The measurement series. /// A Measurement Series is a time-series list of measurements. /// /// ref: https://github.com/opencomputeproject/ocp-diag-core/tree/main/json_spec#measurementseriesstart -pub struct MeasurementSeries { - state: Arc>, +pub struct MeasurementSeries<'a> { + // note: intentional design to only allow 1 thread to output; may need + // revisiting in the future, if there's a case for multithreaded writers + emitter: &'a step::StepEmitter, + seq_no: Arc>, start: MeasurementSeriesStart, } -impl MeasurementSeries { - pub(crate) fn new(series_id: &str, name: &str, state: Arc>) -> Self { +impl<'a> MeasurementSeries<'a> { + pub(crate) fn new(series_id: &str, name: &str, emitter: &'a step::StepEmitter) -> Self { Self { - state, + emitter, seq_no: Arc::new(Mutex::new(atomic::AtomicU64::new(0))), start: MeasurementSeriesStart::new(name, series_id), } @@ -38,10 +41,10 @@ impl MeasurementSeries { pub(crate) fn new_with_details( start: MeasurementSeriesStart, - state: Arc>, + emitter: &'a step::StepEmitter, ) -> Self { Self { - state, + emitter, seq_no: Arc::new(Mutex::new(atomic::AtomicU64::new(0))), start, } @@ -78,11 +81,10 @@ impl MeasurementSeries { /// # }); /// ``` pub async fn start(&self) -> Result<(), emitter::WriterError> { - self.state - .lock() - .await - .emitter - .emit(&self.start.to_artifact()) + self.emitter + .emit(&spec::TestStepArtifactDescendant::MeasurementSeriesStart( + self.start.to_artifact(), + )) .await?; Ok(()) } @@ -110,12 +112,12 @@ impl MeasurementSeries { pub async fn end(&self) -> Result<(), emitter::WriterError> { let end = MeasurementSeriesEnd::new(self.start.get_series_id(), self.current_sequence_no().await); - self.state - .lock() - .await - .emitter - .emit(&end.to_artifact()) + self.emitter + .emit(&spec::TestStepArtifactDescendant::MeasurementSeriesEnd( + end.to_artifact(), + )) .await?; + Ok(()) } @@ -147,12 +149,13 @@ impl MeasurementSeries { None, ); self.increment_sequence_no().await; - self.state - .lock() - .await - .emitter - .emit(&element.to_artifact()) + + self.emitter + .emit(&spec::TestStepArtifactDescendant::MeasurementSeriesElement( + element.to_artifact(), + )) .await?; + Ok(()) } @@ -191,12 +194,13 @@ impl MeasurementSeries { )), ); self.increment_sequence_no().await; - self.state - .lock() - .await - .emitter - .emit(&element.to_artifact()) + + self.emitter + .emit(&spec::TestStepArtifactDescendant::MeasurementSeriesElement( + element.to_artifact(), + )) .await?; + Ok(()) } @@ -228,10 +232,10 @@ impl MeasurementSeries { /// # Ok::<(), WriterError>(()) /// # }); /// ``` - pub async fn scope<'a, F, R>(&'a self, func: F) -> Result<(), emitter::WriterError> + pub async fn scope<'s, F, R>(&'s self, func: F) -> Result<(), emitter::WriterError> where R: Future>, - F: std::ops::FnOnce(&'a MeasurementSeries) -> R, + F: std::ops::FnOnce(&'s MeasurementSeries) -> R, { self.start().await?; func(self).await?; @@ -406,27 +410,25 @@ impl Measurement { /// let measurement = Measurement::new("name", 50.into()); /// let _ = measurement.to_artifact(); /// ``` - pub fn to_artifact(&self) -> spec::RootArtifact { - spec::RootArtifact::TestStepArtifact(spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::Measurement(spec::Measurement { - name: self.name.clone(), - unit: self.unit.clone(), - value: self.value.clone(), - validators: self - .validators - .clone() - .map(|vals| vals.iter().map(|val| val.to_spec()).collect()), - hardware_info_id: self - .hardware_info - .as_ref() - .map(|hardware_info| hardware_info.id().to_owned()), - subcomponent: self - .subcomponent - .as_ref() - .map(|subcomponent| subcomponent.to_spec()), - metadata: self.metadata.clone(), - }), - }) + pub fn to_artifact(&self) -> spec::Measurement { + spec::Measurement { + name: self.name.clone(), + unit: self.unit.clone(), + value: self.value.clone(), + validators: self + .validators + .clone() + .map(|vals| vals.iter().map(|val| val.to_spec()).collect()), + hardware_info_id: self + .hardware_info + .as_ref() + .map(|hardware_info| hardware_info.id().to_owned()), + subcomponent: self + .subcomponent + .as_ref() + .map(|subcomponent| subcomponent.to_spec()), + metadata: self.metadata.clone(), + } } } @@ -634,29 +636,25 @@ impl MeasurementSeriesStart { MeasurementSeriesStartBuilder::new(name, series_id) } - pub fn to_artifact(&self) -> spec::RootArtifact { - spec::RootArtifact::TestStepArtifact(spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::MeasurementSeriesStart( - spec::MeasurementSeriesStart { - name: self.name.clone(), - unit: self.unit.clone(), - series_id: self.series_id.clone(), - validators: self - .validators - .clone() - .map(|vals| vals.iter().map(|val| val.to_spec()).collect()), - hardware_info: self - .hardware_info - .as_ref() - .map(|hardware_info| hardware_info.to_spec()), - subcomponent: self - .subcomponent - .as_ref() - .map(|subcomponent| subcomponent.to_spec()), - metadata: self.metadata.clone(), - }, - ), - }) + pub fn to_artifact(&self) -> spec::MeasurementSeriesStart { + spec::MeasurementSeriesStart { + name: self.name.clone(), + unit: self.unit.clone(), + series_id: self.series_id.clone(), + validators: self + .validators + .clone() + .map(|vals| vals.iter().map(|val| val.to_spec()).collect()), + hardware_info: self + .hardware_info + .as_ref() + .map(|hardware_info| hardware_info.to_spec()), + subcomponent: self + .subcomponent + .as_ref() + .map(|subcomponent| subcomponent.to_spec()), + metadata: self.metadata.clone(), + } } pub fn get_series_id(&self) -> &str { @@ -759,15 +757,11 @@ impl MeasurementSeriesEnd { } } - pub fn to_artifact(&self) -> spec::RootArtifact { - spec::RootArtifact::TestStepArtifact(spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::MeasurementSeriesEnd( - spec::MeasurementSeriesEnd { - series_id: self.series_id.clone(), - total_count: self.total_count, - }, - ), - }) + pub fn to_artifact(&self) -> spec::MeasurementSeriesEnd { + spec::MeasurementSeriesEnd { + series_id: self.series_id.clone(), + total_count: self.total_count, + } } } @@ -795,18 +789,14 @@ impl MeasurementSeriesElement { } } - pub fn to_artifact(&self) -> spec::RootArtifact { - spec::RootArtifact::TestStepArtifact(spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::MeasurementSeriesElement( - spec::MeasurementSeriesElement { - index: self.index, - value: self.value.clone(), - timestamp: self.timestamp, - series_id: self.series_id.clone(), - metadata: self.metadata.clone(), - }, - ), - }) + pub fn to_artifact(&self) -> spec::MeasurementSeriesElement { + spec::MeasurementSeriesElement { + index: self.index, + value: self.value.clone(), + timestamp: self.timestamp, + series_id: self.series_id.clone(), + metadata: self.metadata.clone(), + } } } @@ -829,17 +819,15 @@ mod tests { let artifact = measurement.to_artifact(); assert_eq!( artifact, - spec::RootArtifact::TestStepArtifact(spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::Measurement(spec::Measurement { - name: name.to_string(), - unit: None, - value, - validators: None, - hardware_info_id: None, - subcomponent: None, - metadata: None, - }), - }) + spec::Measurement { + name: name.to_string(), + unit: None, + value, + validators: None, + hardware_info_id: None, + subcomponent: None, + metadata: None, + } ); Ok(()) @@ -874,17 +862,15 @@ mod tests { let artifact = measurement.to_artifact(); assert_eq!( artifact, - spec::RootArtifact::TestStepArtifact(spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::Measurement(spec::Measurement { - name, - unit: Some(unit.to_string()), - value, - validators: Some(vec![validator.to_spec(), validator.to_spec()]), - hardware_info_id: Some(hardware_info.to_spec().id.clone()), - subcomponent: Some(subcomponent.to_spec()), - metadata: Some(metadata), - }), - }) + spec::Measurement { + name, + unit: Some(unit.to_string()), + value, + validators: Some(vec![validator.to_spec(), validator.to_spec()]), + hardware_info_id: Some(hardware_info.to_spec().id.clone()), + subcomponent: Some(subcomponent.to_spec()), + metadata: Some(metadata), + } ); Ok(()) @@ -899,19 +885,15 @@ mod tests { let artifact = series.to_artifact(); assert_eq!( artifact, - spec::RootArtifact::TestStepArtifact(spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::MeasurementSeriesStart( - spec::MeasurementSeriesStart { - name: name.to_string(), - unit: None, - series_id: series_id.to_string(), - validators: None, - hardware_info: None, - subcomponent: None, - metadata: None, - } - ), - }) + spec::MeasurementSeriesStart { + name: name.to_string(), + unit: None, + series_id: series_id.to_string(), + validators: None, + hardware_info: None, + subcomponent: None, + metadata: None, + } ); Ok(()) @@ -938,22 +920,18 @@ mod tests { let artifact = series.to_artifact(); assert_eq!( artifact, - spec::RootArtifact::TestStepArtifact(spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::MeasurementSeriesStart( - spec::MeasurementSeriesStart { - name, - unit: Some("unit".to_string()), - series_id: series_id.to_string(), - validators: Some(vec![validator.to_spec(), validator2.to_spec()]), - hardware_info: Some(hw_info.to_spec()), - subcomponent: Some(subcomponent.to_spec()), - metadata: Some(serde_json::Map::from_iter([ - ("key".to_string(), "value".into()), - ("key2".to_string(), "value2".into()) - ])), - } - ), - }) + spec::MeasurementSeriesStart { + name, + unit: Some("unit".to_string()), + series_id: series_id.to_string(), + validators: Some(vec![validator.to_spec(), validator2.to_spec()]), + hardware_info: Some(hw_info.to_spec()), + subcomponent: Some(subcomponent.to_spec()), + metadata: Some(serde_json::Map::from_iter([ + ("key".to_string(), "value".into()), + ("key2".to_string(), "value2".into()) + ])), + } ); Ok(()) @@ -967,14 +945,10 @@ mod tests { let artifact = series.to_artifact(); assert_eq!( artifact, - spec::RootArtifact::TestStepArtifact(spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::MeasurementSeriesEnd( - spec::MeasurementSeriesEnd { - series_id: series_id.to_string(), - total_count: 1, - } - ), - }) + spec::MeasurementSeriesEnd { + series_id: series_id.to_string(), + total_count: 1, + } ); Ok(()) diff --git a/src/output/run.rs b/src/output/run.rs index c3cc844..d468d9f 100644 --- a/src/output/run.rs +++ b/src/output/run.rs @@ -5,6 +5,8 @@ // https://opensource.org/licenses/MIT. use std::env; +use std::sync::atomic; +use std::sync::atomic::Ordering; use std::sync::Arc; use serde_json::Map; @@ -115,7 +117,7 @@ impl TestRun { .emit(&start.to_artifact()) .await?; - Ok(StartedTestRun { run: self }) + Ok(StartedTestRun::new(self)) } // disabling this for the moment so we don't publish api that's unusable. @@ -283,9 +285,17 @@ impl TestRunBuilder { /// ref: https://github.com/opencomputeproject/ocp-diag-core/tree/main/json_spec#testrunstart pub struct StartedTestRun { run: TestRun, + + step_seqno: atomic::AtomicU64, } impl StartedTestRun { + fn new(run: TestRun) -> StartedTestRun { + StartedTestRun { + run, + step_seqno: atomic::AtomicU64::new(0), + } + } /// Ends the test run. /// /// ref: https://github.com/opencomputeproject/ocp-diag-core/tree/main/json_spec#testrunend @@ -505,7 +515,8 @@ impl StartedTestRun { } pub fn step(&self, name: &str) -> TestStep { - TestStep::new(name, self.run.state.clone()) + let step_id = format!("step_{}", self.step_seqno.fetch_add(1, Ordering::AcqRel)); + TestStep::new(&step_id, name, self.run.state.clone()) } } diff --git a/src/output/step.rs b/src/output/step.rs index 20fa755..6b9fa36 100644 --- a/src/output/step.rs +++ b/src/output/step.rs @@ -12,21 +12,27 @@ use tokio::sync::Mutex; use crate::output as tv; use crate::spec; use tv::measurement::MeasurementSeries; -use tv::{emitter, error, log, measurement, state, step}; +use tv::{emitter, error, log, measurement, state}; + +use super::WriterError; /// A single test step in the scope of a [`TestRun`]. /// /// ref: https://github.com/opencomputeproject/ocp-diag-core/tree/main/json_spec#test-step-artifacts pub struct TestStep { name: String, - state: Arc>, + + emitter: StepEmitter, } impl TestStep { - pub(crate) fn new(name: &str, state: Arc>) -> TestStep { + pub(crate) fn new(id: &str, name: &str, state: Arc>) -> TestStep { TestStep { - name: name.to_string(), - state, + name: name.to_owned(), + emitter: StepEmitter { + state, + step_id: id.to_owned(), + }, } } @@ -47,13 +53,8 @@ impl TestStep { /// # }); /// ``` pub async fn start(self) -> Result { - let start = step::TestStepStart::new(&self.name); - self.state - .lock() - .await - .emitter - .emit(&start.to_artifact()) - .await?; + let start = TestStepStart::new(&self.name); + self.emitter.emit(&start.to_artifact()).await?; Ok(StartedTestStep { step: self, @@ -125,14 +126,8 @@ impl StartedTestStep { /// # }); /// ``` pub async fn end(&self, status: spec::TestStatus) -> Result<(), emitter::WriterError> { - let end = step::TestStepEnd::new(status); - self.step - .state - .lock() - .await - .emitter - .emit(&end.to_artifact()) - .await?; + let end = TestStepEnd::new(status); + self.step.emitter.emit(&end.to_artifact()).await?; Ok(()) } @@ -183,13 +178,10 @@ impl StartedTestStep { msg: &str, ) -> Result<(), emitter::WriterError> { let log = log::Log::builder(msg).severity(severity).build(); - let emitter = &self.step.state.lock().await.emitter; - let artifact = spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::Log(log.to_artifact()), - }; - emitter - .emit(&spec::RootArtifact::TestStepArtifact(artifact)) + self.step + .emitter + .emit(&spec::TestStepArtifactDescendant::Log(log.to_artifact())) .await?; Ok(()) @@ -221,13 +213,9 @@ impl StartedTestStep { /// # }); /// ``` pub async fn log_with_details(&self, log: &log::Log) -> Result<(), emitter::WriterError> { - let emitter = &self.step.state.lock().await.emitter; - - let artifact = spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::Log(log.to_artifact()), - }; - emitter - .emit(&spec::RootArtifact::TestStepArtifact(artifact)) + self.step + .emitter + .emit(&spec::TestStepArtifactDescendant::Log(log.to_artifact())) .await?; Ok(()) @@ -273,13 +261,12 @@ impl StartedTestStep { /// ``` pub async fn error(&self, symptom: &str) -> Result<(), emitter::WriterError> { let error = error::Error::builder(symptom).build(); - let emitter = &self.step.state.lock().await.emitter; - let artifact = spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::Error(error.to_artifact()), - }; - emitter - .emit(&spec::RootArtifact::TestStepArtifact(artifact)) + self.step + .emitter + .emit(&spec::TestStepArtifactDescendant::Error( + error.to_artifact(), + )) .await?; Ok(()) @@ -330,13 +317,12 @@ impl StartedTestStep { msg: &str, ) -> Result<(), emitter::WriterError> { let error = error::Error::builder(symptom).message(msg).build(); - let emitter = &self.step.state.lock().await.emitter; - let artifact = spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::Error(error.to_artifact()), - }; - emitter - .emit(&spec::RootArtifact::TestStepArtifact(artifact)) + self.step + .emitter + .emit(&spec::TestStepArtifactDescendant::Error( + error.to_artifact(), + )) .await?; Ok(()) @@ -372,13 +358,11 @@ impl StartedTestStep { &self, error: &error::Error, ) -> Result<(), emitter::WriterError> { - let emitter = &self.step.state.lock().await.emitter; - - let artifact = spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::Error(error.to_artifact()), - }; - emitter - .emit(&spec::RootArtifact::TestStepArtifact(artifact)) + self.step + .emitter + .emit(&spec::TestStepArtifactDescendant::Error( + error.to_artifact(), + )) .await?; Ok(()) @@ -409,13 +393,14 @@ impl StartedTestStep { value: Value, ) -> Result<(), emitter::WriterError> { let measurement = measurement::Measurement::new(name, value); + self.step - .state - .lock() - .await .emitter - .emit(&measurement.to_artifact()) + .emit(&spec::TestStepArtifactDescendant::Measurement( + measurement.to_artifact(), + )) .await?; + Ok(()) } @@ -451,12 +436,12 @@ impl StartedTestStep { measurement: &measurement::Measurement, ) -> Result<(), emitter::WriterError> { self.step - .state - .lock() - .await .emitter - .emit(&measurement.to_artifact()) + .emit(&spec::TestStepArtifactDescendant::Measurement( + measurement.to_artifact(), + )) .await?; + Ok(()) } @@ -487,7 +472,7 @@ impl StartedTestStep { self.measurement_id_no.load(atomic::Ordering::SeqCst) ); - MeasurementSeries::new(&series_id, name, self.step.state.clone()) + MeasurementSeries::new(&series_id, name, &self.step.emitter) } /// Starts a Measurement Series (a time-series list of measurements). @@ -513,7 +498,7 @@ impl StartedTestStep { &self, start: measurement::MeasurementSeriesStart, ) -> MeasurementSeries { - MeasurementSeries::new_with_details(start, self.step.state.clone()) + MeasurementSeries::new_with_details(start, &self.step.emitter) } } @@ -528,11 +513,9 @@ impl TestStepStart { } } - pub fn to_artifact(&self) -> spec::RootArtifact { - spec::RootArtifact::TestStepArtifact(spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::TestStepStart(spec::TestStepStart { - name: self.name.clone(), - }), + pub fn to_artifact(&self) -> spec::TestStepArtifactDescendant { + spec::TestStepArtifactDescendant::TestStepStart(spec::TestStepStart { + name: self.name.clone(), }) } } @@ -546,14 +529,33 @@ impl TestStepEnd { TestStepEnd { status } } - pub fn to_artifact(&self) -> spec::RootArtifact { - spec::RootArtifact::TestStepArtifact(spec::TestStepArtifact { - descendant: spec::TestStepArtifactDescendant::TestStepEnd(spec::TestStepEnd { - status: self.status.clone(), - }), + pub fn to_artifact(&self) -> spec::TestStepArtifactDescendant { + spec::TestStepArtifactDescendant::TestStepEnd(spec::TestStepEnd { + status: self.status.clone(), }) } } +// TODO: move this away from here; extract trait Emitter, dont rely on json +// it will be used in measurement series +pub struct StepEmitter { + // emitter: JsonEmitter, + state: Arc>, + step_id: String, +} + +impl StepEmitter { + pub async fn emit(&self, object: &spec::TestStepArtifactDescendant) -> Result<(), WriterError> { + let root = spec::RootArtifact::TestStepArtifact(spec::TestStepArtifact { + id: self.step_id.clone(), + // TODO: can these copies be avoided? + descendant: object.clone(), + }); + self.state.lock().await.emitter.emit(&root).await?; + + Ok(()) + } +} + #[cfg(test)] mod tests {} diff --git a/src/spec.rs b/src/spec.rs index dc91774..0161cb8 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -494,6 +494,9 @@ pub struct SourceLocation { /// schema ref: https://github.com/opencomputeproject/ocp-diag-core/testStepArtifact #[derive(Debug, Serialize, PartialEq, Clone)] pub struct TestStepArtifact { + #[serde(rename = "testStepId")] + pub id: String, + #[serde(flatten)] pub descendant: TestStepArtifactDescendant, } diff --git a/tests/output/runner.rs b/tests/output/runner.rs index 82426fc..b1f5413 100644 --- a/tests/output/runner.rs +++ b/tests/output/runner.rs @@ -498,6 +498,7 @@ async fn test_testrun_step_error_with_details() -> Result<()> { json!({ "sequenceNumber": 4, "testStepArtifact": { + "testStepId": "step_0", "error": { "message": "Error message", "softwareInfoIds":[{ @@ -579,12 +580,14 @@ async fn test_step_with_measurement() -> Result<()> { json_run_default_start(), json_step_default_start(), json!({ - "sequenceNumber": 4, "testStepArtifact": { + "testStepId": "step_0", "measurement": { - "name": "name", "value": 50 + "name": "name", + "value": 50 } - } + }, + "sequenceNumber": 4 }), json_step_complete(5), json_run_pass(6), @@ -601,6 +604,8 @@ async fn test_step_with_measurement() -> Result<()> { .await } +// TODO: intentionally leaving these tests broken so that it's obvious later that the +// assert_json_includes was not sufficient; this case is missing `testStepId` field #[tokio::test] async fn test_step_with_measurement_builder() -> Result<()> { let expected = [ From e68165df1f7a42d165930bf742b872135d4c8d18 Mon Sep 17 00:00:00 2001 From: mimir-d Date: Sat, 5 Oct 2024 22:42:45 +0100 Subject: [PATCH 3/3] remove a bunch of boilerplate Signed-off-by: mimir-d --- src/output/emitter.rs | 24 ++- src/output/{measurement.rs => measure.rs} | 125 +++-------- src/output/mod.rs | 4 +- src/output/run.rs | 251 +++------------------- src/output/step.rs | 90 +++----- src/spec.rs | 143 ++++++------ 6 files changed, 174 insertions(+), 463 deletions(-) rename src/output/{measurement.rs => measure.rs} (90%) diff --git a/src/output/emitter.rs b/src/output/emitter.rs index 9bfd8bd..01752dc 100644 --- a/src/output/emitter.rs +++ b/src/output/emitter.rs @@ -98,15 +98,15 @@ impl JsonEmitter { } } - fn serialize_artifact(&self, object: &spec::RootArtifact) -> serde_json::Value { + fn serialize_artifact(&self, object: &spec::RootImpl) -> serde_json::Value { let now = chrono::Local::now(); let now_tz = now.with_timezone(&self.timezone); - let out_artifact = spec::Root { + let root = spec::Root { artifact: object.clone(), timestamp: now_tz, seqno: self.next_sequence_no(), }; - serde_json::json!(out_artifact) + serde_json::json!(root) } fn next_sequence_no(&self) -> u64 { @@ -114,7 +114,7 @@ impl JsonEmitter { self.seqno.load(atomic::Ordering::SeqCst) } - pub async fn emit(&self, object: &spec::RootArtifact) -> Result<(), WriterError> { + pub async fn emit(&self, object: &spec::RootImpl) -> Result<(), WriterError> { let serialized = self.serialize_artifact(object); match self.writer { WriterType::File(ref file) => file.write(&serialized.to_string()).await?, @@ -132,8 +132,6 @@ mod tests { use serde_json::json; use super::*; - use crate::output as tv; - use tv::run::SchemaVersion; #[tokio::test] async fn test_emit_using_buffer_writer() -> Result<()> { @@ -149,8 +147,11 @@ mod tests { let writer = BufferWriter::new(buffer.clone()); let emitter = JsonEmitter::new(chrono_tz::UTC, WriterType::Buffer(writer)); - let version = SchemaVersion::new(); - emitter.emit(&version.to_artifact()).await?; + emitter + .emit(&spec::RootImpl::SchemaVersion( + spec::SchemaVersion::default(), + )) + .await?; let deserialized = serde_json::from_str::( buffer.lock().await.first().ok_or(anyhow!("no outputs"))?, @@ -180,9 +181,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 version = SchemaVersion::new(); - emitter.emit(&version.to_artifact()).await?; - emitter.emit(&version.to_artifact()).await?; + + let version = spec::RootImpl::SchemaVersion(spec::SchemaVersion::default()); + emitter.emit(&version).await?; + emitter.emit(&version).await?; let deserialized = serde_json::from_str::( buffer.lock().await.first().ok_or(anyhow!("no outputs"))?, diff --git a/src/output/measurement.rs b/src/output/measure.rs similarity index 90% rename from src/output/measurement.rs rename to src/output/measure.rs index a874bd9..152af80 100644 --- a/src/output/measurement.rs +++ b/src/output/measure.rs @@ -8,7 +8,6 @@ use std::future::Future; use std::sync::atomic; use std::sync::Arc; -use chrono::DateTime; use serde_json::Map; use serde_json::Value; use tokio::sync::Mutex; @@ -82,7 +81,7 @@ impl<'a> MeasurementSeries<'a> { /// ``` pub async fn start(&self) -> Result<(), emitter::WriterError> { self.emitter - .emit(&spec::TestStepArtifactDescendant::MeasurementSeriesStart( + .emit(&spec::TestStepArtifactImpl::MeasurementSeriesStart( self.start.to_artifact(), )) .await?; @@ -110,12 +109,13 @@ impl<'a> MeasurementSeries<'a> { /// # }); /// ``` pub async fn end(&self) -> Result<(), emitter::WriterError> { - let end = - MeasurementSeriesEnd::new(self.start.get_series_id(), self.current_sequence_no().await); + let end = spec::MeasurementSeriesEnd { + series_id: self.start.series_id.clone(), + total_count: self.current_sequence_no().await, + }; + self.emitter - .emit(&spec::TestStepArtifactDescendant::MeasurementSeriesEnd( - end.to_artifact(), - )) + .emit(&spec::TestStepArtifactImpl::MeasurementSeriesEnd(end)) .await?; Ok(()) @@ -142,17 +142,18 @@ impl<'a> MeasurementSeries<'a> { /// # }); /// ``` pub async fn add_measurement(&self, value: Value) -> Result<(), emitter::WriterError> { - let element = MeasurementSeriesElement::new( - self.current_sequence_no().await, - value, - &self.start, - None, - ); + let element = spec::MeasurementSeriesElement { + index: self.current_sequence_no().await, + value: value.clone(), + timestamp: chrono::Local::now().with_timezone(&chrono_tz::Tz::UTC), + series_id: self.start.series_id.clone(), + metadata: None, + }; self.increment_sequence_no().await; self.emitter - .emit(&spec::TestStepArtifactDescendant::MeasurementSeriesElement( - element.to_artifact(), + .emit(&spec::TestStepArtifactImpl::MeasurementSeriesElement( + element, )) .await?; @@ -185,19 +186,20 @@ impl<'a> MeasurementSeries<'a> { value: Value, metadata: Vec<(&str, Value)>, ) -> Result<(), emitter::WriterError> { - let element = MeasurementSeriesElement::new( - self.current_sequence_no().await, - value, - &self.start, - Some(Map::from_iter( + let element = spec::MeasurementSeriesElement { + index: self.current_sequence_no().await, + value: value.clone(), + timestamp: chrono::Local::now().with_timezone(&chrono_tz::Tz::UTC), + series_id: self.start.series_id.clone(), + metadata: Some(Map::from_iter( metadata.iter().map(|(k, v)| (k.to_string(), v.clone())), )), - ); + }; self.increment_sequence_no().await; self.emitter - .emit(&spec::TestStepArtifactDescendant::MeasurementSeriesElement( - element.to_artifact(), + .emit(&spec::TestStepArtifactImpl::MeasurementSeriesElement( + element, )) .await?; @@ -656,10 +658,6 @@ impl MeasurementSeriesStart { metadata: self.metadata.clone(), } } - - pub fn get_series_id(&self) -> &str { - &self.series_id - } } pub struct MeasurementSeriesStartBuilder { @@ -744,62 +742,6 @@ impl MeasurementSeriesStartBuilder { } } -pub struct MeasurementSeriesEnd { - series_id: String, - total_count: u64, -} - -impl MeasurementSeriesEnd { - pub(crate) fn new(series_id: &str, total_count: u64) -> MeasurementSeriesEnd { - MeasurementSeriesEnd { - series_id: series_id.to_string(), - total_count, - } - } - - pub fn to_artifact(&self) -> spec::MeasurementSeriesEnd { - spec::MeasurementSeriesEnd { - series_id: self.series_id.clone(), - total_count: self.total_count, - } - } -} - -pub struct MeasurementSeriesElement { - index: u64, - value: Value, - timestamp: DateTime, - series_id: String, - metadata: Option>, -} - -impl MeasurementSeriesElement { - pub(crate) fn new( - index: u64, - value: Value, - series: &MeasurementSeriesStart, - metadata: Option>, - ) -> MeasurementSeriesElement { - MeasurementSeriesElement { - index, - value: value.clone(), - timestamp: chrono::Local::now().with_timezone(&chrono_tz::Tz::UTC), - series_id: series.series_id.to_string(), - metadata, - } - } - - pub fn to_artifact(&self) -> spec::MeasurementSeriesElement { - spec::MeasurementSeriesElement { - index: self.index, - value: self.value.clone(), - timestamp: self.timestamp, - series_id: self.series_id.clone(), - metadata: self.metadata.clone(), - } - } -} - #[cfg(test)] mod tests { use super::*; @@ -937,23 +879,6 @@ mod tests { Ok(()) } - #[test] - fn test_measurement_series_end_to_artifact() -> Result<()> { - let series_id = "series_id".to_owned(); - let series = MeasurementSeriesEnd::new(&series_id, 1); - - let artifact = series.to_artifact(); - assert_eq!( - artifact, - spec::MeasurementSeriesEnd { - series_id: series_id.to_string(), - total_count: 1, - } - ); - - Ok(()) - } - #[test] fn test_validator() -> Result<()> { let validator = Validator::builder(ValidatorType::Equal, 30.into()) diff --git a/src/output/mod.rs b/src/output/mod.rs index 17cd5f4..e7b1e5c 100644 --- a/src/output/mod.rs +++ b/src/output/mod.rs @@ -10,7 +10,7 @@ mod emitter; mod error; mod log; mod macros; -mod measurement; +mod measure; mod run; mod state; mod step; @@ -25,7 +25,7 @@ pub use dut::*; pub use emitter::*; pub use error::*; pub use log::*; -pub use measurement::*; +pub use measure::*; pub use run::*; pub use step::*; diff --git a/src/output/run.rs b/src/output/run.rs index d468d9f..77e7b77 100644 --- a/src/output/run.rs +++ b/src/output/run.rs @@ -16,7 +16,7 @@ use tokio::sync::Mutex; use crate::output as tv; use crate::spec; use tv::step::TestStep; -use tv::{config, dut, emitter, error, log, run, state}; +use tv::{config, dut, emitter, error, log, state}; /// The outcome of a TestRun. /// It's returned when the scope method of the [`TestRun`] object is used. @@ -33,10 +33,10 @@ pub struct TestRunOutcome { pub struct TestRun { name: String, version: String, - parameters: Map, + parameters: Map, dut: dut::DutInfo, command_line: String, - metadata: Option>, + metadata: Option>, state: Arc>, } @@ -87,35 +87,28 @@ impl TestRun { /// # }); /// ``` pub async fn start(self) -> Result { - let version = SchemaVersion::new(); + // TODO: this likely will go into the emitter since it's not the run's job to emit the schema version self.state .lock() .await .emitter - .emit(&version.to_artifact()) + .emit(&spec::RootImpl::SchemaVersion( + spec::SchemaVersion::default(), + )) .await?; - let mut builder = run::TestRunStart::builder( - &self.name, - &self.version, - &self.command_line, - &self.parameters, - &self.dut, - ); - - if let Some(m) = &self.metadata { - for m in m { - builder = builder.add_metadata(m.0, m.1.clone()) - } - } + let start = spec::RootImpl::TestRunArtifact(spec::TestRunArtifact { + artifact: spec::TestRunArtifactImpl::TestRunStart(spec::TestRunStart { + name: self.name.clone(), + version: self.version.clone(), + command_line: self.command_line.clone(), + parameters: self.parameters.clone(), + metadata: self.metadata.clone(), + dut_info: self.dut.to_spec(), + }), + }); - let start = builder.build(); - self.state - .lock() - .await - .emitter - .emit(&start.to_artifact()) - .await?; + self.state.lock().await.emitter.emit(&start).await?; Ok(StartedTestRun::new(self)) } @@ -317,14 +310,13 @@ impl StartedTestRun { status: spec::TestStatus, result: spec::TestResult, ) -> Result<(), emitter::WriterError> { - let end = run::TestRunEnd::builder() - .status(status) - .result(result) - .build(); + let end = spec::RootImpl::TestRunArtifact(spec::TestRunArtifact { + artifact: spec::TestRunArtifactImpl::TestRunEnd(spec::TestRunEnd { status, result }), + }); let emitter = &self.run.state.lock().await.emitter; - emitter.emit(&end.to_artifact()).await?; + emitter.emit(&end).await?; Ok(()) } @@ -360,10 +352,10 @@ impl StartedTestRun { let emitter = &self.run.state.lock().await.emitter; let artifact = spec::TestRunArtifact { - artifact: spec::TestRunArtifactDescendant::Log(log.to_artifact()), + artifact: spec::TestRunArtifactImpl::Log(log.to_artifact()), }; emitter - .emit(&spec::RootArtifact::TestRunArtifact(artifact)) + .emit(&spec::RootImpl::TestRunArtifact(artifact)) .await?; Ok(()) @@ -396,10 +388,10 @@ impl StartedTestRun { let emitter = &self.run.state.lock().await.emitter; let artifact = spec::TestRunArtifact { - artifact: spec::TestRunArtifactDescendant::Log(log.to_artifact()), + artifact: spec::TestRunArtifactImpl::Log(log.to_artifact()), }; emitter - .emit(&spec::RootArtifact::TestRunArtifact(artifact)) + .emit(&spec::RootImpl::TestRunArtifact(artifact)) .await?; Ok(()) @@ -428,10 +420,10 @@ impl StartedTestRun { let emitter = &self.run.state.lock().await.emitter; let artifact = spec::TestRunArtifact { - artifact: spec::TestRunArtifactDescendant::Error(error.to_artifact()), + artifact: spec::TestRunArtifactImpl::Error(error.to_artifact()), }; emitter - .emit(&spec::RootArtifact::TestRunArtifact(artifact)) + .emit(&spec::RootImpl::TestRunArtifact(artifact)) .await?; Ok(()) @@ -465,10 +457,10 @@ impl StartedTestRun { let emitter = &self.run.state.lock().await.emitter; let artifact = spec::TestRunArtifact { - artifact: spec::TestRunArtifactDescendant::Error(error.to_artifact()), + artifact: spec::TestRunArtifactImpl::Error(error.to_artifact()), }; emitter - .emit(&spec::RootArtifact::TestRunArtifact(artifact)) + .emit(&spec::RootImpl::TestRunArtifact(artifact)) .await?; Ok(()) @@ -505,10 +497,10 @@ impl StartedTestRun { let emitter = &self.run.state.lock().await.emitter; let artifact = spec::TestRunArtifact { - artifact: spec::TestRunArtifactDescendant::Error(error.to_artifact()), + artifact: spec::TestRunArtifactImpl::Error(error.to_artifact()), }; emitter - .emit(&spec::RootArtifact::TestRunArtifact(artifact)) + .emit(&spec::RootImpl::TestRunArtifact(artifact)) .await?; Ok(()) @@ -519,182 +511,3 @@ impl StartedTestRun { TestStep::new(&step_id, name, self.run.state.clone()) } } - -pub struct TestRunStart { - name: String, - version: String, - command_line: String, - parameters: Map, - metadata: Option>, - dut_info: dut::DutInfo, -} - -impl TestRunStart { - pub fn builder( - name: &str, - version: &str, - command_line: &str, - parameters: &Map, - dut_info: &dut::DutInfo, - ) -> TestRunStartBuilder { - TestRunStartBuilder::new(name, version, command_line, parameters, dut_info) - } - - pub fn to_artifact(&self) -> spec::RootArtifact { - spec::RootArtifact::TestRunArtifact(spec::TestRunArtifact { - artifact: spec::TestRunArtifactDescendant::TestRunStart(spec::TestRunStart { - name: self.name.clone(), - version: self.version.clone(), - command_line: self.command_line.clone(), - parameters: self.parameters.clone(), - metadata: self.metadata.clone(), - dut_info: self.dut_info.to_spec(), - }), - }) - } -} - -pub struct TestRunStartBuilder { - name: String, - version: String, - command_line: String, - parameters: Map, - metadata: Option>, - dut_info: dut::DutInfo, -} - -impl TestRunStartBuilder { - pub fn new( - name: &str, - version: &str, - command_line: &str, - parameters: &Map, - dut_info: &dut::DutInfo, - ) -> TestRunStartBuilder { - TestRunStartBuilder { - name: name.to_string(), - version: version.to_string(), - command_line: command_line.to_string(), - parameters: parameters.clone(), - metadata: None, - dut_info: dut_info.clone(), - } - } - - pub fn add_metadata(mut self, key: &str, value: Value) -> TestRunStartBuilder { - self.metadata = match self.metadata { - Some(mut metadata) => { - metadata.insert(key.to_string(), value.clone()); - Some(metadata) - } - None => { - let mut metadata = Map::new(); - metadata.insert(key.to_string(), value.clone()); - Some(metadata) - } - }; - self - } - - pub fn build(self) -> TestRunStart { - TestRunStart { - name: self.name, - version: self.version, - command_line: self.command_line, - parameters: self.parameters, - metadata: self.metadata, - dut_info: self.dut_info, - } - } -} - -pub struct TestRunEnd { - status: spec::TestStatus, - result: spec::TestResult, -} - -impl TestRunEnd { - pub fn builder() -> TestRunEndBuilder { - TestRunEndBuilder::new() - } - - pub fn to_artifact(&self) -> spec::RootArtifact { - spec::RootArtifact::TestRunArtifact(spec::TestRunArtifact { - artifact: spec::TestRunArtifactDescendant::TestRunEnd(spec::TestRunEnd { - status: self.status.clone(), - result: self.result.clone(), - }), - }) - } -} - -#[derive(Debug)] -pub struct TestRunEndBuilder { - status: spec::TestStatus, - result: spec::TestResult, -} - -#[allow(clippy::new_without_default)] -impl TestRunEndBuilder { - pub fn new() -> TestRunEndBuilder { - TestRunEndBuilder { - status: spec::TestStatus::Complete, - result: spec::TestResult::Pass, - } - } - pub fn status(mut self, value: spec::TestStatus) -> TestRunEndBuilder { - self.status = value; - self - } - - pub fn result(mut self, value: spec::TestResult) -> TestRunEndBuilder { - self.result = value; - self - } - - pub fn build(self) -> TestRunEnd { - TestRunEnd { - status: self.status, - result: self.result, - } - } -} - -// TODO: this likely will go into the emitter since it's not the run's job to emit the schema version -pub struct SchemaVersion { - major: i8, - minor: i8, -} - -#[allow(clippy::new_without_default)] -impl SchemaVersion { - pub fn new() -> SchemaVersion { - SchemaVersion { - major: spec::SPEC_VERSION.0, - minor: spec::SPEC_VERSION.1, - } - } - - pub fn to_artifact(&self) -> spec::RootArtifact { - spec::RootArtifact::SchemaVersion(spec::SchemaVersion { - major: self.major, - minor: self.minor, - }) - } -} - -#[cfg(test)] -mod tests { - use anyhow::Result; - - use super::*; - use crate::spec; - - #[test] - fn test_schema_creation_from_builder() -> Result<()> { - let version = SchemaVersion::new(); - assert_eq!(version.major, spec::SPEC_VERSION.0); - assert_eq!(version.minor, spec::SPEC_VERSION.1); - Ok(()) - } -} diff --git a/src/output/step.rs b/src/output/step.rs index 6b9fa36..e754dd6 100644 --- a/src/output/step.rs +++ b/src/output/step.rs @@ -10,9 +10,10 @@ use std::sync::Arc; use tokio::sync::Mutex; use crate::output as tv; -use crate::spec; -use tv::measurement::MeasurementSeries; -use tv::{emitter, error, log, measurement, state}; +use crate::spec::TestStepStart; +use crate::spec::{self, TestStepArtifactImpl}; +use tv::measure::MeasurementSeries; +use tv::{emitter, error, log, measure, state}; use super::WriterError; @@ -53,8 +54,11 @@ impl TestStep { /// # }); /// ``` pub async fn start(self) -> Result { - let start = TestStepStart::new(&self.name); - self.emitter.emit(&start.to_artifact()).await?; + self.emitter + .emit(&TestStepArtifactImpl::TestStepStart(TestStepStart { + name: self.name.clone(), + })) + .await?; Ok(StartedTestStep { step: self, @@ -126,12 +130,13 @@ impl StartedTestStep { /// # }); /// ``` pub async fn end(&self, status: spec::TestStatus) -> Result<(), emitter::WriterError> { - let end = TestStepEnd::new(status); - self.step.emitter.emit(&end.to_artifact()).await?; + let end = TestStepArtifactImpl::TestStepEnd(spec::TestStepEnd { status }); + + self.step.emitter.emit(&end).await?; Ok(()) } - /// Eemits Log message. + /// Emits Log message. /// This method accepts a [`models::LogSeverity`] to define the severity /// and a [`std::string::String`] for the message. /// @@ -181,7 +186,7 @@ impl StartedTestStep { self.step .emitter - .emit(&spec::TestStepArtifactDescendant::Log(log.to_artifact())) + .emit(&TestStepArtifactImpl::Log(log.to_artifact())) .await?; Ok(()) @@ -215,7 +220,7 @@ impl StartedTestStep { pub async fn log_with_details(&self, log: &log::Log) -> Result<(), emitter::WriterError> { self.step .emitter - .emit(&spec::TestStepArtifactDescendant::Log(log.to_artifact())) + .emit(&TestStepArtifactImpl::Log(log.to_artifact())) .await?; Ok(()) @@ -264,9 +269,7 @@ impl StartedTestStep { self.step .emitter - .emit(&spec::TestStepArtifactDescendant::Error( - error.to_artifact(), - )) + .emit(&TestStepArtifactImpl::Error(error.to_artifact())) .await?; Ok(()) @@ -320,9 +323,7 @@ impl StartedTestStep { self.step .emitter - .emit(&spec::TestStepArtifactDescendant::Error( - error.to_artifact(), - )) + .emit(&TestStepArtifactImpl::Error(error.to_artifact())) .await?; Ok(()) @@ -360,9 +361,7 @@ impl StartedTestStep { ) -> Result<(), emitter::WriterError> { self.step .emitter - .emit(&spec::TestStepArtifactDescendant::Error( - error.to_artifact(), - )) + .emit(&TestStepArtifactImpl::Error(error.to_artifact())) .await?; Ok(()) @@ -392,11 +391,11 @@ impl StartedTestStep { name: &str, value: Value, ) -> Result<(), emitter::WriterError> { - let measurement = measurement::Measurement::new(name, value); + let measurement = measure::Measurement::new(name, value); self.step .emitter - .emit(&spec::TestStepArtifactDescendant::Measurement( + .emit(&TestStepArtifactImpl::Measurement( measurement.to_artifact(), )) .await?; @@ -433,11 +432,11 @@ impl StartedTestStep { /// ``` pub async fn add_measurement_with_details( &self, - measurement: &measurement::Measurement, + measurement: &measure::Measurement, ) -> Result<(), emitter::WriterError> { self.step .emitter - .emit(&spec::TestStepArtifactDescendant::Measurement( + .emit(&spec::TestStepArtifactImpl::Measurement( measurement.to_artifact(), )) .await?; @@ -496,46 +495,12 @@ impl StartedTestStep { /// ``` pub fn measurement_series_with_details( &self, - start: measurement::MeasurementSeriesStart, + start: measure::MeasurementSeriesStart, ) -> MeasurementSeries { MeasurementSeries::new_with_details(start, &self.step.emitter) } } -pub struct TestStepStart { - name: String, -} - -impl TestStepStart { - pub fn new(name: &str) -> TestStepStart { - TestStepStart { - name: name.to_string(), - } - } - - pub fn to_artifact(&self) -> spec::TestStepArtifactDescendant { - spec::TestStepArtifactDescendant::TestStepStart(spec::TestStepStart { - name: self.name.clone(), - }) - } -} - -pub struct TestStepEnd { - status: spec::TestStatus, -} - -impl TestStepEnd { - pub fn new(status: spec::TestStatus) -> TestStepEnd { - TestStepEnd { status } - } - - pub fn to_artifact(&self) -> spec::TestStepArtifactDescendant { - spec::TestStepArtifactDescendant::TestStepEnd(spec::TestStepEnd { - status: self.status.clone(), - }) - } -} - // TODO: move this away from here; extract trait Emitter, dont rely on json // it will be used in measurement series pub struct StepEmitter { @@ -545,17 +510,14 @@ pub struct StepEmitter { } impl StepEmitter { - pub async fn emit(&self, object: &spec::TestStepArtifactDescendant) -> Result<(), WriterError> { - let root = spec::RootArtifact::TestStepArtifact(spec::TestStepArtifact { + 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? - descendant: object.clone(), + artifact: object.clone(), }); self.state.lock().await.emitter.emit(&root).await?; Ok(()) } } - -#[cfg(test)] -mod tests {} diff --git a/src/spec.rs b/src/spec.rs index 0161cb8..545d306 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -38,70 +38,6 @@ mod rfc3339_format { } } -#[derive(Debug, Serialize, PartialEq, Clone)] -pub enum TestRunArtifactDescendant { - #[serde(rename = "testRunStart")] - TestRunStart(TestRunStart), - - #[serde(rename = "testRunEnd")] - TestRunEnd(TestRunEnd), - - #[serde(rename = "log")] - Log(Log), - - #[serde(rename = "error")] - Error(Error), -} - -#[derive(Debug, Serialize, PartialEq, Clone)] -pub enum RootArtifact { - #[serde(rename = "schemaVersion")] - SchemaVersion(SchemaVersion), - - #[serde(rename = "testRunArtifact")] - TestRunArtifact(TestRunArtifact), - - #[serde(rename = "testStepArtifact")] - TestStepArtifact(TestStepArtifact), -} - -#[allow(clippy::large_enum_variant)] -#[derive(Debug, Serialize, PartialEq, Clone)] -pub enum TestStepArtifactDescendant { - #[serde(rename = "testStepStart")] - TestStepStart(TestStepStart), - - #[serde(rename = "testStepEnd")] - TestStepEnd(TestStepEnd), - - #[serde(rename = "measurement")] - Measurement(Measurement), - - #[serde(rename = "measurementSeriesStart")] - MeasurementSeriesStart(MeasurementSeriesStart), - - #[serde(rename = "measurementSeriesEnd")] - MeasurementSeriesEnd(MeasurementSeriesEnd), - - #[serde(rename = "measurementSeriesElement")] - MeasurementSeriesElement(MeasurementSeriesElement), - - #[serde(rename = "diagnosis")] - Diagnosis(Diagnosis), - - #[serde(rename = "log")] - Log(Log), - - #[serde(rename = "error")] - Error(Error), - - #[serde(rename = "file")] - File(File), - - #[serde(rename = "extension")] - Extension(Extension), -} - #[derive(Debug, Serialize, Clone, PartialEq)] #[non_exhaustive] pub enum ValidatorType { @@ -241,7 +177,7 @@ pub enum SoftwareType { #[derive(Debug, Serialize, Clone)] pub struct Root { #[serde(flatten)] - pub artifact: RootArtifact, + pub artifact: RootImpl, // TODO : manage different timezones #[serde(rename = "timestamp")] @@ -252,6 +188,18 @@ pub struct Root { pub seqno: u64, } +#[derive(Debug, Serialize, PartialEq, Clone)] +pub enum RootImpl { + #[serde(rename = "schemaVersion")] + SchemaVersion(SchemaVersion), + + #[serde(rename = "testRunArtifact")] + TestRunArtifact(TestRunArtifact), + + #[serde(rename = "testStepArtifact")] + TestStepArtifact(TestStepArtifact), +} + /// Low-level model for the `schemaVersion` spec object. /// Specifies the version that should be used to interpret following json outputs. /// ref: https://github.com/opencomputeproject/ocp-diag-core/tree/main/json_spec#schemaversion @@ -267,6 +215,15 @@ pub struct SchemaVersion { pub minor: i8, } +impl Default for SchemaVersion { + fn default() -> Self { + SchemaVersion { + major: SPEC_VERSION.0, + minor: SPEC_VERSION.1, + } + } +} + /// Low-level model for the `testRunArtifact` spec object. /// Container for the run level artifacts. /// ref: https://github.com/opencomputeproject/ocp-diag-core/tree/main/json_spec#test-run-artifacts @@ -275,7 +232,22 @@ pub struct SchemaVersion { #[derive(Debug, Serialize, PartialEq, Clone)] pub struct TestRunArtifact { #[serde(flatten)] - pub artifact: TestRunArtifactDescendant, + pub artifact: TestRunArtifactImpl, +} + +#[derive(Debug, Serialize, PartialEq, Clone)] +pub enum TestRunArtifactImpl { + #[serde(rename = "testRunStart")] + TestRunStart(TestRunStart), + + #[serde(rename = "testRunEnd")] + TestRunEnd(TestRunEnd), + + #[serde(rename = "log")] + Log(Log), + + #[serde(rename = "error")] + Error(Error), } /// Low-level model for the `testRunStart` spec object. @@ -498,7 +470,44 @@ pub struct TestStepArtifact { pub id: String, #[serde(flatten)] - pub descendant: TestStepArtifactDescendant, + pub artifact: TestStepArtifactImpl, +} + +#[allow(clippy::large_enum_variant)] +#[derive(Debug, Serialize, PartialEq, Clone)] +pub enum TestStepArtifactImpl { + #[serde(rename = "testStepStart")] + TestStepStart(TestStepStart), + + #[serde(rename = "testStepEnd")] + TestStepEnd(TestStepEnd), + + #[serde(rename = "measurement")] + Measurement(Measurement), + + #[serde(rename = "measurementSeriesStart")] + MeasurementSeriesStart(MeasurementSeriesStart), + + #[serde(rename = "measurementSeriesEnd")] + MeasurementSeriesEnd(MeasurementSeriesEnd), + + #[serde(rename = "measurementSeriesElement")] + MeasurementSeriesElement(MeasurementSeriesElement), + + #[serde(rename = "diagnosis")] + Diagnosis(Diagnosis), + + #[serde(rename = "log")] + Log(Log), + + #[serde(rename = "error")] + Error(Error), + + #[serde(rename = "file")] + File(File), + + #[serde(rename = "extension")] + Extension(Extension), } /// Low-level model for the `testStepStart` spec object.