Configurable Logging Levels & More Ignore Options (#237)

* Allow for changing the logging levels of node and rpcs

* Allow for ignoring cases with failing steps

* Add logging level config for geth

Author: 0xOmarA

crates/config/src/lib.rs

@@ -218,12 +218,14 @@ impl AsRef<ReportConfiguration> for Context {
     }
 }
 
-impl AsRef<IgnoreSuccessConfiguration> for Context {
-    fn as_ref(&self) -> &IgnoreSuccessConfiguration {
+impl AsRef<IgnoreCasesConfiguration> for Context {
+    fn as_ref(&self) -> &IgnoreCasesConfiguration {
+        static DEFAULT: LazyLock<IgnoreCasesConfiguration> = LazyLock::new(Default::default);
+
         match self {
             Self::Test(context) => context.as_ref().as_ref(),
-            Self::Benchmark(..) => unreachable!(),
-            Self::ExportJsonSchema | Self::ExportGenesis(..) => unreachable!(),
+            Self::Benchmark(..) => &DEFAULT,
+            Self::ExportJsonSchema | Self::ExportGenesis(..) => &DEFAULT,
         }
     }
 }
@@ -317,9 +319,9 @@ pub struct TestExecutionContext {
     #[clap(flatten, next_help_heading = "Report Configuration")]
     pub report_configuration: ReportConfiguration,
 
-    /// Configuration parameters for ignoring certain test cases based on the report
+    /// Configuration parameters for ignoring certain test cases.
     #[clap(flatten, next_help_heading = "Ignore Success Configuration")]
-    pub ignore_success_configuration: IgnoreSuccessConfiguration,
+    pub ignore_configuration: IgnoreCasesConfiguration,
 }
 
 impl TestExecutionContext {
@@ -623,9 +625,9 @@ impl AsRef<ReportConfiguration> for TestExecutionContext {
     }
 }
 
-impl AsRef<IgnoreSuccessConfiguration> for TestExecutionContext {
-    fn as_ref(&self) -> &IgnoreSuccessConfiguration {
-        &self.ignore_success_configuration
+impl AsRef<IgnoreCasesConfiguration> for TestExecutionContext {
+    fn as_ref(&self) -> &IgnoreCasesConfiguration {
+        &self.ignore_configuration
     }
 }
 
@@ -856,6 +858,14 @@ pub struct GethConfiguration {
         value_parser = parse_duration
     )]
     pub start_timeout_ms: Duration,
+
+    /// The logging configuration to pass to the binary when it's being started.
+    #[clap(
+        id = "geth.logging-level",
+        long = "geth.logging-level",
+        default_value = "3"
+    )]
+    pub logging_level: String,
 }
 
 /// A set of configuration parameters for kurtosis.
@@ -904,6 +914,14 @@ pub struct ReviveDevNodeConfiguration {
     )]
     pub consensus: String,
 
+    /// The logging configuration to pass to the binary when it's being started.
+    #[clap(
+        id = "revive-dev-node.logging-level",
+        long = "revive-dev-node.logging-level",
+        default_value = "error,evm=debug,sc_rpc_server=info,runtime::revive=debug"
+    )]
+    pub logging_level: String,
+
     /// Specifies the connection string of an existing node that's not managed by the framework.
     ///
     /// If this argument is specified then the framework will not spawn certain nodes itself but
@@ -969,6 +987,14 @@ pub struct PolkadotOmnichainNodeConfiguration {
         long = "polkadot-omni-node.parachain-id"
     )]
     pub parachain_id: Option<usize>,
+
+    /// The logging configuration to pass to the binary when it's being started.
+    #[clap(
+        id = "polkadot-omni-node.logging-level",
+        long = "polkadot-omni-node.logging-level",
+        default_value = "error,evm=debug,sc_rpc_server=info,runtime::revive=debug"
+    )]
+    pub logging_level: String,
 }
 
 /// A set of configuration parameters for the ETH RPC.
@@ -989,6 +1015,14 @@ pub struct EthRpcConfiguration {
         value_parser = parse_duration
     )]
     pub start_timeout_ms: Duration,
+
+    /// The logging configuration to pass to the binary when it's being started.
+    #[clap(
+        id = "eth-rpc.logging-level",
+        long = "eth-rpc.logging-level",
+        default_value = "info,eth-rpc=debug"
+    )]
+    pub logging_level: String,
 }
 
 /// A set of configuration parameters for the genesis.
@@ -1132,10 +1166,28 @@ pub struct ReportConfiguration {
 }
 
 #[derive(Clone, Debug, Parser, Serialize, Deserialize)]
-pub struct IgnoreSuccessConfiguration {
-    /// The path of the report generated by the tool to use to ignore the cases that succeeded.
-    #[clap(long = "ignore-success.report-path")]
-    pub path: Option<PathBuf>,
+pub struct IgnoreCasesConfiguration {
+    /// The path to a report where all of the succeeding cases found in should
+    /// be ignored.
+    #[clap(
+        id = "ignore.succeeding-cases-from-report",
+        long = "ignore.succeeding-cases-from-report"
+    )]
+    pub succeeding_cases_from_report: Option<PathBuf>,
+
+    /// Allows the tool to ignore any test case where there's a step that's
+    /// expected to fail
+    #[clap(
+        id = "ignore.cases-with-failing-steps",
+        long = "ignore.cases-with-failing-steps"
+    )]
+    pub ignore_cases_with_failing_steps: bool,
+}
+
+impl Default for IgnoreCasesConfiguration {
+    fn default() -> Self {
+        IgnoreCasesConfiguration::parse_from(["ignore-cases-configuration"])
+    }
 }
 
 /// Represents the working directory that the program uses.

crates/core/src/differential_benchmarks/entry_point.rs

@@ -97,7 +97,7 @@ pub async fn handle_differential_benchmarks(
         &full_context,
         &corpus,
         &platforms_and_nodes,
-        None,
+        &Default::default(),
         reporter.clone(),
     )
     .await

crates/core/src/differential_tests/entry_point.rs

@@ -10,7 +10,7 @@ use std::{
 use ansi_term::{ANSIStrings, Color};
 use anyhow::Context as _;
 use futures::{FutureExt, StreamExt};
-use revive_dt_common::{cached_fs::read_to_string, types::PrivateKeyAllocator};
+use revive_dt_common::types::PrivateKeyAllocator;
 use revive_dt_core::Platform;
 use revive_dt_format::corpus::Corpus;
 use tokio::sync::{Mutex, RwLock, Semaphore};
@@ -21,7 +21,10 @@ use revive_dt_report::{Reporter, ReporterEvent, TestCaseStatus};
 
 use crate::{
     differential_tests::Driver,
-    helpers::{CachedCompiler, NodePool, create_test_definitions_stream},
+    helpers::{
+        CachedCompiler, NodePool, TestCaseIgnoreResolvedConfiguration,
+        create_test_definitions_stream,
+    },
 };
 
 /// Handles the differential testing executing it according to the information defined in the
@@ -81,20 +84,14 @@ pub async fn handle_differential_tests(
     info!("Spawned the platform nodes");
 
     // Preparing test definitions.
-    let only_execute_failed_tests = match context.ignore_success_configuration.path.as_ref() {
-        Some(path) => {
-            let report = read_to_string(path)
-                .context("Failed to read the report file to ignore the succeeding test cases")?;
-            Some(serde_json::from_str(&report).context("Failed to deserialize report")?)
-        }
-        None => None,
-    };
+    let test_case_ignore_configuration =
+        TestCaseIgnoreResolvedConfiguration::try_from(context.ignore_configuration.clone())?;
     let full_context = Context::Test(Box::new(context.clone()));
     let test_definitions = create_test_definitions_stream(
         &full_context,
         &corpus,
         &platforms_and_nodes,
-        only_execute_failed_tests.as_ref(),
+        &test_case_ignore_configuration,
         reporter.clone(),
     )
     .await

crates/core/src/helpers/test.rs

@@ -2,10 +2,12 @@ use std::collections::BTreeMap;
 use std::sync::Arc;
 use std::{borrow::Cow, path::Path};
 
+use anyhow::Context as _;
 use futures::{Stream, StreamExt, stream};
 use indexmap::{IndexMap, indexmap};
+use revive_dt_common::cached_fs::read_to_string;
 use revive_dt_common::types::PlatformIdentifier;
-use revive_dt_config::Context;
+use revive_dt_config::{Context, IgnoreCasesConfiguration};
 use revive_dt_format::corpus::Corpus;
 use serde_json::{Value, json};
 
@@ -29,7 +31,7 @@ pub async fn create_test_definitions_stream<'a>(
     context: &Context,
     corpus: &'a Corpus,
     platforms_and_nodes: &'a BTreeMap<PlatformIdentifier, (&dyn Platform, NodePool)>,
-    only_execute_failed_tests: Option<&Report>,
+    test_case_ignore_configuration: &TestCaseIgnoreResolvedConfiguration,
     reporter: Reporter,
 ) -> impl Stream<Item = TestDefinition<'a>> {
     let cloned_reporter = reporter.clone();
@@ -130,7 +132,7 @@ pub async fn create_test_definitions_stream<'a>(
     )
     // Filter out the test cases which are incompatible or that can't run in the current setup.
     .filter_map(move |test| async move {
-        match test.check_compatibility(only_execute_failed_tests) {
+        match test.check_compatibility(test_case_ignore_configuration) {
             Ok(()) => Some(test),
             Err((reason, additional_information)) => {
                 debug!(
@@ -165,6 +167,69 @@ pub async fn create_test_definitions_stream<'a>(
     })
 }
 
+#[derive(Clone, Debug, Default)]
+pub struct TestCaseIgnoreResolvedConfiguration {
+    /// Some existing report which will be used for a number of purposes.
+    pub report: Option<Report>,
+
+    /// Controls if test cases which succeeded from the existing report should
+    /// be ignored or not.
+    pub ignore_succeeding_test_cases_from_report: bool,
+
+    /// Controls if test cases which contain steps expected to fail should be
+    /// ignored or not.
+    pub ignore_cases_with_failing_steps: bool,
+}
+
+impl TestCaseIgnoreResolvedConfiguration {
+    pub fn with_report(self, report: impl Into<Option<Report>>) -> Self {
+        self.mutate(|this| this.report = report.into())
+    }
+
+    pub fn with_ignore_succeeding_test_cases_from_report(
+        self,
+        ignore_succeeding_test_cases_from_report: bool,
+    ) -> Self {
+        self.mutate(|this| {
+            this.ignore_succeeding_test_cases_from_report = ignore_succeeding_test_cases_from_report
+        })
+    }
+
+    pub fn with_ignore_cases_with_failing_steps(
+        self,
+        ignore_cases_with_failing_steps: bool,
+    ) -> Self {
+        self.mutate(|this| this.ignore_cases_with_failing_steps = ignore_cases_with_failing_steps)
+    }
+
+    fn mutate(mut self, mutator: impl FnOnce(&mut Self)) -> Self {
+        mutator(&mut self);
+        self
+    }
+}
+
+impl TryFrom<IgnoreCasesConfiguration> for TestCaseIgnoreResolvedConfiguration {
+    type Error = anyhow::Error;
+
+    fn try_from(value: IgnoreCasesConfiguration) -> Result<Self, Self::Error> {
+        let mut this = Self::default()
+            .with_ignore_cases_with_failing_steps(value.ignore_cases_with_failing_steps);
+
+        this = if let Some(succeeding_cases_from_report) = value.succeeding_cases_from_report {
+            let content = read_to_string(succeeding_cases_from_report)
+                .context("Failed to read report at the specified path")?;
+            let report =
+                serde_json::from_str::<Report>(&content).context("Failed to deserialize report")?;
+            this.with_report(report)
+                .with_ignore_succeeding_test_cases_from_report(true)
+        } else {
+            this
+        };
+
+        Ok(this)
+    }
+}
+
 /// This is a full description of a differential test to run alongside the full metadata file, the
 /// specific case to be tested, the platforms that the tests should run on, the specific nodes of
 /// these platforms that they should run on, the compilers to use, and everything else needed making
@@ -192,14 +257,14 @@ impl<'a> TestDefinition<'a> {
     /// Checks if this test can be ran with the current configuration.
     pub fn check_compatibility(
         &self,
-        only_execute_failed_tests: Option<&Report>,
+        test_case_ignore_configuration: &TestCaseIgnoreResolvedConfiguration,
     ) -> TestCheckFunctionResult {
         self.check_metadata_file_ignored()?;
         self.check_case_file_ignored()?;
         self.check_target_compatibility()?;
         self.check_evm_version_compatibility()?;
         self.check_compiler_compatibility()?;
-        self.check_ignore_succeeded(only_execute_failed_tests)?;
+        self.check_ignore_configuration(test_case_ignore_configuration)?;
         Ok(())
     }
 
@@ -317,32 +382,49 @@ impl<'a> TestDefinition<'a> {
 
     /// Checks if the test case should be executed or not based on the passed report and whether the
     /// user has instructed the tool to ignore the already succeeding test cases.
-    fn check_ignore_succeeded(
+    fn check_ignore_configuration(
         &self,
-        only_execute_failed_tests: Option<&Report>,
+        test_case_ignore_configuration: &TestCaseIgnoreResolvedConfiguration,
     ) -> TestCheckFunctionResult {
-        let Some(report) = only_execute_failed_tests else {
-            return Ok(());
-        };
-
-        let test_case_status = report
-            .execution_information
-            .get(&(self.metadata_file_path.to_path_buf().into()))
-            .and_then(|obj| obj.case_reports.get(&self.case_idx))
-            .and_then(|obj| obj.mode_execution_reports.get(&self.mode))
-            .and_then(|obj| obj.status.as_ref());
-
-        match test_case_status {
-            Some(TestCaseStatus::Failed { .. }) => Ok(()),
-            Some(TestCaseStatus::Ignored { .. }) => Err((
-                "Ignored since it was ignored in a previous run",
+        // Check if the test case should be ignored based on it containing
+        // failing steps.
+        if test_case_ignore_configuration.ignore_cases_with_failing_steps
+            && self.case.any_step_expected_to_fail()
+        {
+            return Err((
+                "Ignored since it contains steps which are expected to fail",
                 indexmap! {},
-            )),
-            Some(TestCaseStatus::Succeeded { .. }) => {
-                Err(("Ignored since it succeeded in a prior run", indexmap! {}))
+            ));
+        }
+
+        // Check if the succeeding cases from the provided report should be
+        // ignored or not.
+        if let (Some(report), true) = (
+            test_case_ignore_configuration.report.as_ref(),
+            test_case_ignore_configuration.ignore_succeeding_test_cases_from_report,
+        ) {
+            let test_case_status = report
+                .execution_information
+                .get(&(self.metadata_file_path.to_path_buf().into()))
+                .and_then(|obj| obj.case_reports.get(&self.case_idx))
+                .and_then(|obj| obj.mode_execution_reports.get(&self.mode))
+                .and_then(|obj| obj.status.as_ref());
+
+            match test_case_status {
+                Some(TestCaseStatus::Failed { .. }) | None => {}
+                Some(TestCaseStatus::Ignored { .. }) => {
+                    return Err((
+                        "Ignored since it was ignored in a previous run",
+                        indexmap! {},
+                    ));
+                }
+                Some(TestCaseStatus::Succeeded { .. }) => {
+                    return Err(("Ignored since it succeeded in a prior run", indexmap! {}));
+                }
             }
-            None => Ok(()),
         }
+
+        Ok(())
     }
 }