From 74294a045b7e3ad75310c950aa86e7d4174a62f9 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Amiel Date: Wed, 31 Jul 2024 17:36:14 +0200 Subject: [PATCH] Remove filename from LoggerOptions in favor of parameter in hurl_file::run public function. --- packages/hurl/src/cli/options/mod.rs | 3 +- packages/hurl/src/parallel/worker.rs | 3 +- packages/hurl/src/run.rs | 13 +++++-- packages/hurl/src/runner/hurl_file.rs | 53 +++++++++++++++++---------- packages/hurl/src/util/logger.rs | 36 ++++++++---------- packages/hurl/tests/sample.rs | 13 +++++-- 6 files changed, 71 insertions(+), 50 deletions(-) diff --git a/packages/hurl/src/cli/options/mod.rs b/packages/hurl/src/cli/options/mod.rs index d7bcda423..ed9ee7373 100644 --- a/packages/hurl/src/cli/options/mod.rs +++ b/packages/hurl/src/cli/options/mod.rs @@ -482,12 +482,11 @@ impl CliOptions { .build() } - pub fn to_logger_options(&self, filename: &Input) -> LoggerOptions { + pub fn to_logger_options(&self) -> LoggerOptions { let verbosity = Verbosity::from(self.verbose, self.very_verbose); LoggerOptionsBuilder::new() .color(self.color) .error_format(self.error_format.into()) - .filename(&filename.to_string()) .verbosity(verbosity) .build() } diff --git a/packages/hurl/src/parallel/worker.rs b/packages/hurl/src/parallel/worker.rs index e2a464ee5..2a923badf 100644 --- a/packages/hurl/src/parallel/worker.rs +++ b/packages/hurl/src/parallel/worker.rs @@ -102,7 +102,7 @@ impl Worker { let hurl_file = match hurl_file { Ok(h) => h, Err(e) => { - logger.error_parsing_rich(&content, &e); + logger.error_parsing_rich(&content, Some(&job.filename), &e); let msg = ParsingErrorMsg::new(worker_id, &job, &logger.stderr); return tx.send(WorkerMessage::ParsingError(msg)); } @@ -112,6 +112,7 @@ impl Worker { let result = runner::run_entries( &hurl_file.entries, &content, + Some(&job.filename), &job.runner_options, &job.variables, &mut stdout, diff --git a/packages/hurl/src/run.rs b/packages/hurl/src/run.rs index dd48f62be..cc5c9031b 100644 --- a/packages/hurl/src/run.rs +++ b/packages/hurl/src/run.rs @@ -59,13 +59,18 @@ pub fn run_seq( }; let variables = &options.variables; let runner_options = options.to_runner_options(&filename, current_dir); - let logger_options = options.to_logger_options(&filename); + let logger_options = options.to_logger_options(); // Run our Hurl file now, we can only fail if there is a parsing error. // The parsing error is displayed in the `execute` call, that's why we gobble the error // string. - let Ok(hurl_result) = runner::run(&content, &runner_options, variables, &logger_options) - else { + let Ok(hurl_result) = runner::run( + &content, + Some(&filename), + &runner_options, + variables, + &logger_options, + ) else { return Err(CliError::Parsing); }; @@ -181,7 +186,7 @@ pub fn run_par( .enumerate() .map(|(seq, input)| { let runner_options = options.to_runner_options(input, current_dir); - let logger_options = options.to_logger_options(input); + let logger_options = options.to_logger_options(); Job::new(input, seq, &runner_options, variables, &logger_options) }) .collect::>(); diff --git a/packages/hurl/src/runner/hurl_file.rs b/packages/hurl/src/runner/hurl_file.rs index e3c852ee7..38e0ea2d7 100644 --- a/packages/hurl/src/runner/hurl_file.rs +++ b/packages/hurl/src/runner/hurl_file.rs @@ -25,6 +25,7 @@ use hurl_core::ast::{ Body, Bytes, Entry, MultilineString, OptionKind, Request, Response, SourceInfo, }; use hurl_core::error::DisplaySourceError; +use hurl_core::input::Input; use hurl_core::parser; use hurl_core::typing::Count; @@ -39,9 +40,11 @@ use crate::util::term::{Stderr, Stdout, WriteMode}; /// /// If `content` is a syntactically correct Hurl file, an [`HurlResult`] is always returned on /// run completion. The success or failure of the run (due to assertions checks, runtime failures -/// etc..) can be read in the [`HurlResult`] `success` field. If `content` is not syntactically +/// etc...) can be read in the [`HurlResult`] `success` field. If `content` is not syntactically /// correct, a parsing error is returned. This is the only possible way for this function to fail. /// +/// `filename` indicates an optional file source, used when displaying errors. +/// /// # Example /// /// ``` @@ -49,6 +52,7 @@ use crate::util::term::{Stderr, Stdout, WriteMode}; /// use hurl::runner; /// use hurl::runner::{Value, RunnerOptionsBuilder}; /// use hurl::util::logger::{LoggerOptionsBuilder, Verbosity}; +/// use hurl_core::input::Input; /// /// // A simple Hurl sample /// let content = r#" @@ -56,6 +60,8 @@ use crate::util::term::{Stderr, Stdout, WriteMode}; /// HTTP 200 /// "#; /// +/// let filename = Input::new("sample.hurl"); +/// /// // Define runner and logger options /// let runner_opts = RunnerOptionsBuilder::new() /// .follow_location(true) @@ -71,6 +77,7 @@ use crate::util::term::{Stderr, Stdout, WriteMode}; /// // Run the Hurl sample /// let result = runner::run( /// content, +/// Some(filename).as_ref(), /// &runner_opts, /// &variables, /// &logger_opts @@ -79,6 +86,7 @@ use crate::util::term::{Stderr, Stdout, WriteMode}; /// ``` pub fn run( content: &str, + filename: Option<&Input>, runner_options: &RunnerOptions, variables: &HashMap, logger_options: &LoggerOptions, @@ -97,7 +105,7 @@ pub fn run( let hurl_file = match hurl_file { Ok(h) => h, Err(e) => { - logger.error_parsing_rich(content, &e); + logger.error_parsing_rich(content, filename, &e); return Err(e.description()); } }; @@ -106,6 +114,7 @@ pub fn run( let result = run_entries( &hurl_file.entries, content, + filename, runner_options, variables, &mut stdout, @@ -114,13 +123,14 @@ pub fn run( ); if result.success && result.entries.last().is_none() { - let filename = &logger_options.filename; + let filename = filename.map_or(String::new(), |f| f.to_string()); logger.warning(&format!("No entry have been executed for file {filename}")); } Ok(result) } +#[allow(clippy::too_many_arguments)] /// Runs a list of `entries` and returns a [`HurlResult`] upon completion. /// /// `content` is the original source content, used to construct `entries`. It is used to construct @@ -130,6 +140,7 @@ pub fn run( pub fn run_entries( entries: &[Entry], content: &str, + filename: Option<&Input>, runner_options: &RunnerOptions, variables: &HashMap, stdout: &mut Stdout, @@ -177,7 +188,7 @@ pub fn run_entries( log_run_entry(entry_index, logger); - warn_deprecated(entry, logger); + warn_deprecated(entry, filename, logger); // We can report the progression of the run for --test mode. if let Some(listener) = listener { @@ -196,7 +207,7 @@ pub fn run_entries( errors: vec![error.clone()], ..Default::default() }; - log_errors(&entry_result, content, false, logger); + log_errors(&entry_result, content, filename, false, logger); entries_result.push(entry_result); if runner_options.continue_on_error { entry_index += 1; @@ -240,6 +251,7 @@ pub fn run_entries( entry, entry_index, content, + filename, &mut http_client, &options, &mut variables, @@ -303,6 +315,7 @@ fn run_request( entry: &Entry, entry_index: usize, content: &str, + filename: Option<&Input>, http_client: &mut Client, options: &RunnerOptions, variables: &mut HashMap, @@ -352,7 +365,7 @@ fn run_request( } if has_error { - log_errors(&result, content, retry, logger); + log_errors(&result, content, filename, retry, logger); } results.push(result); @@ -420,10 +433,10 @@ fn is_success(entries: &[EntryResult]) -> bool { } /// Logs deprecated syntax and provides alternatives. -fn warn_deprecated(entry: &Entry, logger: &mut Logger) { +fn warn_deprecated(entry: &Entry, filename: Option<&Input>, logger: &mut Logger) { + let filename = filename.map_or(String::new(), |f| f.to_string()); // HTTP/* is used instead of HTTP. if let Some(response) = &entry.response { - let filename = &logger.filename; let version = &response.version; let source_info = &version.source_info; let line = &source_info.start.line; @@ -443,7 +456,6 @@ fn warn_deprecated(entry: &Entry, logger: &mut Logger) { .. } = &entry.request { - let filename = &logger.filename; let source_info = &template.source_info; let line = &source_info.start.line; let column = &source_info.start.column; @@ -460,7 +472,6 @@ fn warn_deprecated(entry: &Entry, logger: &mut Logger) { .. }) = &entry.response { - let filename = &logger.filename; let source_info = &template.source_info; let line = &source_info.start.line; let column = &source_info.start.column; @@ -557,12 +568,17 @@ fn log_run_info( /// Logs runner `errors`. /// If we're going to `retry` the entry, we log errors only in verbose. Otherwise, we log error on stderr. -fn log_errors(entry_result: &EntryResult, content: &str, retry: bool, logger: &mut Logger) { +fn log_errors( + entry_result: &EntryResult, + content: &str, + filename: Option<&Input>, + retry: bool, + logger: &mut Logger, +) { if retry { - entry_result - .errors - .iter() - .for_each(|e| logger.debug_error(content, e, entry_result.source_info)); + entry_result.errors.iter().for_each(|error| { + logger.debug_error(content, filename, error, entry_result.source_info); + }); return; } @@ -571,10 +587,9 @@ fn log_errors(entry_result: &EntryResult, content: &str, retry: bool, logger: &m response.log_info_all(logger); } } - entry_result - .errors - .iter() - .for_each(|error| logger.error_runtime_rich(content, error, entry_result.source_info)); + entry_result.errors.iter().for_each(|error| { + logger.error_runtime_rich(content, filename, error, entry_result.source_info); + }); } /// Logs the header indicating the begin of the entry run. diff --git a/packages/hurl/src/util/logger.rs b/packages/hurl/src/util/logger.rs index 8dc85157b..5341dd3ba 100644 --- a/packages/hurl/src/util/logger.rs +++ b/packages/hurl/src/util/logger.rs @@ -19,6 +19,7 @@ use hurl_core::ast::SourceInfo; use hurl_core::error::{DisplaySourceError, OutputFormat}; +use hurl_core::input::Input; use hurl_core::text::{Format, Style, StyledString}; use crate::runner::Value; @@ -51,7 +52,6 @@ impl Verbosity { pub struct Logger { pub(crate) color: bool, pub(crate) error_format: ErrorFormat, - pub(crate) filename: String, pub(crate) verbosity: Option, pub(crate) stderr: Stderr, } @@ -60,14 +60,12 @@ pub struct Logger { pub struct LoggerOptions { pub(crate) color: bool, pub(crate) error_format: ErrorFormat, - pub(crate) filename: String, pub(crate) verbosity: Option, } pub struct LoggerOptionsBuilder { color: bool, error_format: ErrorFormat, - filename: String, verbosity: Option, } @@ -97,18 +95,11 @@ impl LoggerOptionsBuilder { self } - /// Sets filename. - pub fn filename(&mut self, filename: &str) -> &mut Self { - self.filename = filename.to_string(); - self - } - /// Creates a new logger. pub fn build(&self) -> LoggerOptions { LoggerOptions { color: self.color, error_format: self.error_format, - filename: self.filename.clone(), verbosity: self.verbosity, } } @@ -119,7 +110,6 @@ impl Default for LoggerOptionsBuilder { LoggerOptionsBuilder { color: false, error_format: ErrorFormat::Short, - filename: String::new(), verbosity: None, } } @@ -131,7 +121,6 @@ impl Logger { Logger { color: options.color, error_format: options.error_format, - filename: options.filename.to_string(), verbosity: options.verbosity, stderr: term, } @@ -194,14 +183,16 @@ impl Logger { pub fn debug_error( &mut self, content: &str, + filename: Option<&Input>, error: &E, entry_src_info: SourceInfo, ) { if self.verbosity.is_none() { return; } + let filename = filename.map_or(String::new(), |f| f.to_string()); let message = error.to_string( - &self.filename, + &filename, content, Some(entry_src_info), OutputFormat::Terminal(self.color), @@ -272,26 +263,29 @@ impl Logger { self.stderr.eprintln(&s.to_string(fmt)); } - pub fn error_parsing_rich(&mut self, content: &str, error: &E) { + pub fn error_parsing_rich( + &mut self, + content: &str, + filename: Option<&Input>, + error: &E, + ) { // FIXME: peut-être qu'on devrait faire rentrer le prefix `error:` qui est // fournit par `self.error_rich` dans la méthode `error.to_string` - let message = error.to_string( - &self.filename, - content, - None, - OutputFormat::Terminal(self.color), - ); + let filename = filename.map_or(String::new(), |f| f.to_string()); + let message = error.to_string(&filename, content, None, OutputFormat::Terminal(self.color)); self.error_rich(&message); } pub fn error_runtime_rich( &mut self, content: &str, + filename: Option<&Input>, error: &E, entry_src_info: SourceInfo, ) { + let filename = filename.map_or(String::new(), |f| f.to_string()); let message = error.to_string( - &self.filename, + &filename, content, Some(entry_src_info), OutputFormat::Terminal(self.color), diff --git a/packages/hurl/tests/sample.rs b/packages/hurl/tests/sample.rs index 0a35a0df9..eacd4f11d 100644 --- a/packages/hurl/tests/sample.rs +++ b/packages/hurl/tests/sample.rs @@ -24,6 +24,7 @@ use hurl::runner; use hurl::runner::{EntryResult, HurlResult, RunnerOptionsBuilder}; use hurl::util::logger::LoggerOptionsBuilder; use hurl::util::path::ContextDir; +use hurl_core::input::Input; use hurl_core::typing::Count; #[test] @@ -98,7 +99,7 @@ fn simple_sample() { `Hello World!` "#; - let filename = "-"; + let filename = Some(Input::Stdin); // Define runner and logger options let runner_opts = RunnerOptionsBuilder::new() @@ -128,7 +129,6 @@ fn simple_sample() { let logger_opts = LoggerOptionsBuilder::new() .color(false) - .filename(filename) .verbosity(None) .build(); @@ -136,7 +136,14 @@ fn simple_sample() { let variables = HashMap::default(); // Run the hurl file and check data: - let result = runner::run(content, &runner_opts, &variables, &logger_opts).unwrap(); + let result = runner::run( + content, + filename.as_ref(), + &runner_opts, + &variables, + &logger_opts, + ) + .unwrap(); check_result(&result); let entry = result.entries.first().unwrap();