From 4fbd17feac28f282fce4b3ce5d51c80fb62290c2 Mon Sep 17 00:00:00 2001 From: ilslv Date: Fri, 7 Jan 2022 09:17:01 +0300 Subject: [PATCH 1/8] Make `runner::Basic` execute Scenarios as they are available, not in batches --- src/runner/basic.rs | 324 ++++++++++++++++++++++++++++---------------- 1 file changed, 210 insertions(+), 114 deletions(-) diff --git a/src/runner/basic.rs b/src/runner/basic.rs index a7f7c707..78ea5b93 100644 --- a/src/runner/basic.rs +++ b/src/runner/basic.rs @@ -18,7 +18,7 @@ use std::{ panic::{self, AssertUnwindSafe}, path::PathBuf, sync::{ - atomic::{AtomicBool, AtomicUsize, Ordering}, + atomic::{AtomicBool, Ordering}, Arc, }, }; @@ -464,12 +464,22 @@ async fn insert_features( /// Retrieves [`Feature`]s and executes them. /// +/// # Events +/// +/// - [`Scenario`] events are emitted by [`Executor`]. +/// - If [`Scenario`] was first or last for particular [`Rule`] or [`Feature`], +/// emits starting or finishing events for them. +/// /// [`Feature`]: gherkin::Feature +/// [`Rule`]: gherkin::Rule +/// [`Scenario`]: gherkin::Scenario async fn execute( features: Features, max_concurrent_scenarios: Option, collection: step::Collection, - sender: mpsc::UnboundedSender>>>, + event_sender: mpsc::UnboundedSender< + parser::Result>>, + >, before_hook: Option, after_hook: Option, ) where @@ -500,60 +510,78 @@ async fn execute( let hook = panic::take_hook(); panic::set_hook(Box::new(|_| {})); - let mut executor = - Executor::new(collection, before_hook, after_hook, sender); + let (finished_sender, finished_receiver) = mpsc::unbounded(); + let mut storage = FinishedRulesAndFeatures::new(finished_receiver); + let executor = Executor::new( + collection, + before_hook, + after_hook, + event_sender, + finished_sender, + ); - executor.send(event::Cucumber::Started); + executor.send_event(event::Cucumber::Started); + let mut started_scenarios = max_concurrent_scenarios; + let mut run_scenarios = stream::FuturesUnordered::new(); loop { - let runnable = features.get(max_concurrent_scenarios).await; - if runnable.is_empty() { + let runnable = features.get(started_scenarios).await; + if run_scenarios.is_empty() && runnable.is_empty() { if features.is_finished() { break; } continue; } - let started = executor.start_scenarios(&runnable); - executor.send_all(started); + let started = storage.start_scenarios(&runnable); + executor.send_all_events(started); - drop( - runnable - .into_iter() - .map(|(f, r, s)| executor.run_scenario(f, r, s)) - .collect::>() - .await, - ); + if let Some(sc) = started_scenarios.as_mut() { + *sc -= runnable.len(); + } + + for (f, r, s) in runnable { + run_scenarios.push(executor.run_scenario(f, r, s)); + } + + if run_scenarios.next().await.is_some() { + if let Some(sc) = started_scenarios.as_mut() { + *sc += 1; + } + } + + while let Ok(Some((feat, rule))) = storage.finished_receiver.try_next() + { + let mut cleanup = false; + if let Some(r) = rule { + if let Some(f) = + storage.rule_scenario_finished(Arc::clone(&feat), r) + { + executor.send_event(f); + cleanup = true; + } + } + + if let Some(f) = storage.feature_scenario_finished(feat) { + executor.send_event(f); + cleanup = true; + } - executor.cleanup_finished_rules_and_features(); + if cleanup { + storage.cleanup_finished_rules_and_features(); + } + } } - executor.send(event::Cucumber::Finished); + executor.send_event(event::Cucumber::Finished); panic::set_hook(hook); } -/// Stores currently ran [`Feature`]s and notifies about their state of -/// completion. +/// Runs [`Scenario`]s and notifies about their state of completion. /// -/// [`Feature`]: gherkin::Feature. +/// [`Scenario`]: gherkin::Scenario struct Executor { - /// Number of finished [`Scenario`]s of [`Feature`]. - /// - /// [`Feature`]: gherkin::Feature - /// [`Scenario`]: gherkin::Scenario - features_scenarios_count: HashMap, AtomicUsize>, - - /// Number of finished [`Scenario`]s of [`Rule`]. - /// - /// We also store path to `.feature` file so [`Rule`]s with same names and - /// spans in different files will have different hashes. - /// - /// [`Rule`]: gherkin::Rule - /// [`Scenario`]: gherkin::Scenario - rule_scenarios_count: - HashMap<(Option, Arc), AtomicUsize>, - /// [`Step`]s [`Collection`]. /// /// [`Collection`]: step::Collection @@ -574,10 +602,20 @@ struct Executor { /// [`Step`]: gherkin::Step after_hook: Option, - /// Sender for notifying state of [`Feature`]s completion. + /// Sender for [`Scenario`] [events][1]. /// - /// [`Feature`]: gherkin::Feature - sender: mpsc::UnboundedSender>>>, + /// [`Scenario`]: gherkin::Scenario + /// [1]: event::Scenario + event_sender: + mpsc::UnboundedSender>>>, + + /// Sender for notifying of [`Scenario`]s completion. + /// + /// [`Scenario`]: gherkin::Scenario + finished_sender: mpsc::UnboundedSender<( + Arc, + Option>, + )>, } impl Executor @@ -602,17 +640,20 @@ where collection: step::Collection, before_hook: Option, after_hook: Option, - sender: mpsc::UnboundedSender< + event_sender: mpsc::UnboundedSender< parser::Result>>, >, + finished_sender: mpsc::UnboundedSender<( + Arc, + Option>, + )>, ) -> Self { Self { - features_scenarios_count: HashMap::new(), - rule_scenarios_count: HashMap::new(), collection, before_hook, after_hook, - sender, + event_sender, + finished_sender, } } @@ -621,8 +662,6 @@ where /// # Events /// /// - Emits all [`Scenario`] events. - /// - If [`Scenario`] was last for particular [`Rule`] or [`Feature`], also - /// emits finishing events for them. /// /// [`Feature`]: gherkin::Feature /// [`Rule`]: gherkin::Rule @@ -672,7 +711,7 @@ where event::Scenario::step_failed, ); - self.send(event::Cucumber::scenario( + self.send_event(event::Cucumber::scenario( Arc::clone(&feature), rule.clone(), Arc::clone(&scenario), @@ -732,27 +771,21 @@ where .await .map_or((), drop); - self.send(event::Cucumber::scenario( + self.send_event(event::Cucumber::scenario( Arc::clone(&feature), rule.clone(), Arc::clone(&scenario), event::Scenario::Finished, )); - if let Some(r) = rule { - if let Some(f) = - self.rule_scenario_finished(Arc::clone(&feature), r) - { - self.send(f); - } - } - - if let Some(f) = self.feature_scenario_finished(feature) { - self.send(f); - } + self.scenario_finished(feature, rule); } /// Executes [`HookType::Before`], if present. + /// + /// # Events + /// + /// - Emits [`HookType::Before`] event. async fn run_before_hook( &self, feature: &Arc, @@ -776,7 +809,7 @@ where }; if let Some(hook) = self.before_hook.as_ref() { - self.send(event::Cucumber::scenario( + self.send_event(event::Cucumber::scenario( Arc::clone(feature), rule.map(Arc::clone), Arc::clone(scenario), @@ -798,7 +831,7 @@ where match fut.await { Ok(world) => { - self.send(event::Cucumber::scenario( + self.send_event(event::Cucumber::scenario( Arc::clone(feature), rule.map(Arc::clone), Arc::clone(scenario), @@ -807,7 +840,7 @@ where Ok(Some(world)) } Err((info, world)) => { - self.send(event::Cucumber::scenario( + self.send_event(event::Cucumber::scenario( Arc::clone(feature), rule.map(Arc::clone), Arc::clone(scenario), @@ -826,6 +859,10 @@ where } /// Executes [`HookType::After`], if present. + /// + /// # Event + /// + /// - Emits [`HookType::After`] event. async fn run_after_hook( &self, mut world: Option, @@ -834,7 +871,7 @@ where scenario: &Arc, ) -> Result, ()> { if let Some(hook) = self.after_hook.as_ref() { - self.send(event::Cucumber::scenario( + self.send_event(event::Cucumber::scenario( Arc::clone(feature), rule.map(Arc::clone), Arc::clone(scenario), @@ -857,7 +894,7 @@ where #[allow(clippy::shadow_unrelated)] match fut.await { Ok(world) => { - self.send(event::Cucumber::scenario( + self.send_event(event::Cucumber::scenario( Arc::clone(feature), rule.map(Arc::clone), Arc::clone(scenario), @@ -866,7 +903,7 @@ where Ok(world) } Err((info, world)) => { - self.send(event::Cucumber::scenario( + self.send_event(event::Cucumber::scenario( Arc::clone(feature), rule.map(Arc::clone), Arc::clone(scenario), @@ -908,7 +945,7 @@ where event::StepError, ) -> event::Cucumber, { - self.send(started(Arc::clone(&step))); + self.send_event(started(Arc::clone(&step))); let run = async { let (step_fn, captures, ctx) = match self.collection.find(&step) { @@ -953,38 +990,124 @@ where #[allow(clippy::shadow_unrelated)] match run.await { Ok((Some(captures), Some(world))) => { - self.send(passed(step, captures)); + self.send_event(passed(step, captures)); Ok(world) } Ok((_, world)) => { - self.send(skipped(step)); + self.send_event(skipped(step)); Err(world) } Err((err, captures, world)) => { - self.send(failed(step, captures, world.map(Arc::new), err)); + self.send_event(failed( + step, + captures, + world.map(Arc::new), + err, + )); Err(None) } } } + /// Notifies [`FinishedRulesAndFeatures`] about [`Scenario`] being finished. + /// + /// [`Scenario`]: gherkin::Scenario + fn scenario_finished( + &self, + feature: Arc, + rule: Option>, + ) { + drop(self.finished_sender.unbounded_send((feature, rule))); + } + + /// Notifies with the given [`Cucumber`] event. + /// + /// [`Cucumber`]: event::Cucumber + fn send_event(&self, event: event::Cucumber) { + // If the receiver end is dropped, then no one listens for events + // so we can just ignore it. + drop(self.event_sender.unbounded_send(Ok(Event::new(event)))); + } + + /// Notifies with the given [`Cucumber`] events. + /// + /// [`Cucumber`]: event::Cucumber + fn send_all_events( + &self, + events: impl Iterator>, + ) { + for v in events { + // If the receiver end is dropped, then no one listens for events + // so we can just stop from here. + if self.event_sender.unbounded_send(Ok(Event::new(v))).is_err() { + break; + } + } + } +} + +/// Stores currently ran [`Rule`]s and [`Feature`]s and notifies about their +/// state of completion. +/// +/// [`Feature`]: gherkin::Feature +/// [`Rule`]: gherkin::Rule +struct FinishedRulesAndFeatures { + /// Number of finished [`Scenario`]s of [`Feature`]. + /// + /// [`Feature`]: gherkin::Feature + /// [`Scenario`]: gherkin::Scenario + features_scenarios_count: HashMap, usize>, + + /// Number of finished [`Scenario`]s of [`Rule`]. + /// + /// We also store path to `.feature` file so [`Rule`]s with same names and + /// spans in different files will have different hashes. + /// + /// [`Rule`]: gherkin::Rule + /// [`Scenario`]: gherkin::Scenario + rule_scenarios_count: HashMap<(Option, Arc), usize>, + + /// Receiver for notifying state of [`Scenario`]s completion. + /// + /// [`Scenario`]: gherkin::Scenario + finished_receiver: mpsc::UnboundedReceiver<( + Arc, + Option>, + )>, +} + +impl FinishedRulesAndFeatures { + /// Creates a new [`Executor`]. + fn new( + finished_receiver: mpsc::UnboundedReceiver<( + Arc, + Option>, + )>, + ) -> Self { + Self { + features_scenarios_count: HashMap::new(), + rule_scenarios_count: HashMap::new(), + finished_receiver, + } + } + /// Marks [`Rule`]'s [`Scenario`] as finished and returns [`Rule::Finished`] /// event if no [`Scenario`]s left. /// /// [`Rule`]: gherkin::Rule /// [`Rule::Finished`]: event::Rule::Finished /// [`Scenario`]: gherkin::Scenario - fn rule_scenario_finished( - &self, + fn rule_scenario_finished( + &mut self, feature: Arc, rule: Arc, ) -> Option> { let finished_scenarios = self .rule_scenarios_count - .get(&(feature.path.clone(), Arc::clone(&rule))) - .unwrap_or_else(|| panic!("No Rule {}", rule.name)) - .fetch_add(1, Ordering::SeqCst) - + 1; - (rule.scenarios.len() == finished_scenarios) + .get_mut(&(feature.path.clone(), Arc::clone(&rule))) + .unwrap_or_else(|| panic!("No Rule {}", rule.name)); + *finished_scenarios += 1; + (rule.scenarios.len() == *finished_scenarios) .then(|| event::Cucumber::rule_finished(feature, rule)) } @@ -994,18 +1117,17 @@ where /// [`Feature`]: gherkin::Feature /// [`Feature::Finished`]: event::Feature::Finished /// [`Scenario`]: gherkin::Scenario - fn feature_scenario_finished( - &self, + fn feature_scenario_finished( + &mut self, feature: Arc, ) -> Option> { let finished_scenarios = self .features_scenarios_count - .get(&feature) - .unwrap_or_else(|| panic!("No Feature {}", feature.name)) - .fetch_add(1, Ordering::SeqCst) - + 1; + .get_mut(&feature) + .unwrap_or_else(|| panic!("No Feature {}", feature.name)); + *finished_scenarios += 1; let scenarios = feature.count_scenarios(); - (scenarios == finished_scenarios) + (scenarios == *finished_scenarios) .then(|| event::Cucumber::feature_finished(feature)) } @@ -1018,7 +1140,7 @@ where /// [`Rule`]: gherkin::Rule /// [`Rule::Started`]: event::Rule::Started /// [`Scenario`]: gherkin::Scenario - fn start_scenarios( + fn start_scenarios( &mut self, runnable: impl AsRef< [( @@ -1037,7 +1159,7 @@ where .entry(Arc::clone(&feature)) .or_insert_with(|| { started_features.push(feature); - 0.into() + 0 }); } @@ -1054,7 +1176,7 @@ where .entry((feat.path.clone(), Arc::clone(&rule))) .or_insert_with(|| { started_rules.push((feat, rule)); - 0.into() + 0 }); } @@ -1077,41 +1199,15 @@ where self.features_scenarios_count = self .features_scenarios_count .drain() - .filter(|(f, count)| { - f.count_scenarios() != count.load(Ordering::SeqCst) - }) + .filter(|(f, count)| f.count_scenarios() != *count) .collect(); self.rule_scenarios_count = self .rule_scenarios_count .drain() - .filter(|((_, r), count)| { - r.scenarios.len() != count.load(Ordering::SeqCst) - }) + .filter(|((_, r), count)| r.scenarios.len() != *count) .collect(); } - - /// Notifies with the given [`Cucumber`] event. - /// - /// [`Cucumber`]: event::Cucumber - fn send(&self, event: event::Cucumber) { - // If the receiver end is dropped, then no one listens for events - // so we can just ignore it. - drop(self.sender.unbounded_send(Ok(Event::new(event)))); - } - - /// Notifies with the given [`Cucumber`] events. - /// - /// [`Cucumber`]: event::Cucumber - fn send_all(&self, events: impl Iterator>) { - for v in events { - // If the receiver end is dropped, then no one listens for events - // so we can just stop from here. - if self.sender.unbounded_send(Ok(Event::new(v))).is_err() { - break; - } - } - } } /// [`Scenario`]s storage. From f2413a5474d768be3dac3432c5d0ff3fe8e8fb6b Mon Sep 17 00:00:00 2001 From: ilslv Date: Fri, 7 Jan 2022 09:21:31 +0300 Subject: [PATCH 2/8] CHANGELOG --- CHANGELOG.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49b63408..70a2579a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,20 @@ All user visible changes to `cucumber` crate will be documented in this file. Th +## [0.11.1] · ??? +[0.11.1]: /../../tree/v0.11.1 + +[Diff](/../../compare/v0.11.0...v0.11.1) | [Milestone](/../../milestone/6) + +### Added + +- Make `runner::Basic` execute Scenarios as they are available, not in batches. ([#195]) + +[#195]: /../../pull/195 + + + + ## [0.11.0] · 2022-01-03 [0.11.0]: /../../tree/v0.11.0 From 30071c0e00625e4884f1aa08903d175284c82fd1 Mon Sep 17 00:00:00 2001 From: ilslv Date: Fri, 7 Jan 2022 09:34:55 +0300 Subject: [PATCH 3/8] Corrections --- CHANGELOG.md | 2 +- src/runner/basic.rs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70a2579a..d6c5dbf7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ All user visible changes to `cucumber` crate will be documented in this file. Th ### Added -- Make `runner::Basic` execute Scenarios as they are available, not in batches. ([#195]) +- Make `runner::Basic` execute `Scenario`s as they are available, not in batches. ([#195]) [#195]: /../../pull/195 diff --git a/src/runner/basic.rs b/src/runner/basic.rs index 78ea5b93..40f9c685 100644 --- a/src/runner/basic.rs +++ b/src/runner/basic.rs @@ -1017,6 +1017,8 @@ where feature: Arc, rule: Option>, ) { + // If the receiver end is dropped, then no one listens for events + // so we can just ignore it. drop(self.finished_sender.unbounded_send((feature, rule))); } From 75c682c132e06f0960562697cc10456dd1d94255 Mon Sep 17 00:00:00 2001 From: ilslv Date: Fri, 7 Jan 2022 11:30:22 +0300 Subject: [PATCH 4/8] Corrections --- src/runner/basic.rs | 40 ++++++++++------------------------------ 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/src/runner/basic.rs b/src/runner/basic.rs index 40f9c685..838e5a38 100644 --- a/src/runner/basic.rs +++ b/src/runner/basic.rs @@ -552,23 +552,16 @@ async fn execute( while let Ok(Some((feat, rule))) = storage.finished_receiver.try_next() { - let mut cleanup = false; if let Some(r) = rule { if let Some(f) = storage.rule_scenario_finished(Arc::clone(&feat), r) { executor.send_event(f); - cleanup = true; } } if let Some(f) = storage.feature_scenario_finished(feat) { executor.send_event(f); - cleanup = true; - } - - if cleanup { - storage.cleanup_finished_rules_and_features(); } } } @@ -1109,8 +1102,12 @@ impl FinishedRulesAndFeatures { .get_mut(&(feature.path.clone(), Arc::clone(&rule))) .unwrap_or_else(|| panic!("No Rule {}", rule.name)); *finished_scenarios += 1; - (rule.scenarios.len() == *finished_scenarios) - .then(|| event::Cucumber::rule_finished(feature, rule)) + (rule.scenarios.len() == *finished_scenarios).then(|| { + let _ = self + .rule_scenarios_count + .remove(&(feature.path.clone(), Arc::clone(&rule))); + event::Cucumber::rule_finished(feature, rule) + }) } /// Marks [`Feature`]'s [`Scenario`] as finished and returns @@ -1129,8 +1126,10 @@ impl FinishedRulesAndFeatures { .unwrap_or_else(|| panic!("No Feature {}", feature.name)); *finished_scenarios += 1; let scenarios = feature.count_scenarios(); - (scenarios == *finished_scenarios) - .then(|| event::Cucumber::feature_finished(feature)) + (scenarios == *finished_scenarios).then(|| { + let _ = self.features_scenarios_count.remove(&feature); + event::Cucumber::feature_finished(feature) + }) } /// Marks [`Scenario`]s as started and returns [`Rule::Started`] and @@ -1191,25 +1190,6 @@ impl FinishedRulesAndFeatures { .map(|(f, r)| event::Cucumber::rule_started(f, r)), ) } - - /// Removes all finished [`Rule`]s and [`Feature`]s as all their events are - /// emitted already. - /// - /// [`Feature`]: gherkin::Feature - /// [`Rule`]: gherkin::Rule - fn cleanup_finished_rules_and_features(&mut self) { - self.features_scenarios_count = self - .features_scenarios_count - .drain() - .filter(|(f, count)| f.count_scenarios() != *count) - .collect(); - - self.rule_scenarios_count = self - .rule_scenarios_count - .drain() - .filter(|((_, r), count)| r.scenarios.len() != *count) - .collect(); - } } /// [`Scenario`]s storage. From 1081698f0cdd2029c7d621ec11d2aad5fe0ae259 Mon Sep 17 00:00:00 2001 From: ilslv Date: Fri, 7 Jan 2022 12:50:50 +0300 Subject: [PATCH 5/8] Add `--fail-fast` CLI flag --- src/runner/basic.rs | 117 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 101 insertions(+), 16 deletions(-) diff --git a/src/runner/basic.rs b/src/runner/basic.rs index 838e5a38..e44c1a85 100644 --- a/src/runner/basic.rs +++ b/src/runner/basic.rs @@ -15,8 +15,8 @@ use std::{ collections::HashMap, convert::identity, fmt, mem, + ops::ControlFlow, panic::{self, AssertUnwindSafe}, - path::PathBuf, sync::{ atomic::{AtomicBool, Ordering}, Arc, @@ -48,6 +48,10 @@ pub struct Cli { /// value configured in tests runner, or 64 by default. #[clap(long, short, name = "int")] pub concurrency: Option, + + /// Runs tests until first failure. + #[clap(long)] + pub fail_fast: bool, } /// Type determining whether [`Scenario`]s should run concurrently or @@ -101,6 +105,11 @@ pub type AfterHookFn = for<'a> fn( Option<&'a mut World>, ) -> LocalBoxFuture<'a, ()>; +/// Alias for a failed [`Scenario`]. +/// +/// [`Scenario`]: gherkin::Scenario +type Failed = bool; + /// Default [`Runner`] implementation which follows [_order guarantees_][1] from /// the [`Runner`] trait docs. /// @@ -148,6 +157,12 @@ pub struct Basic< /// [`Scenario`]: gherkin::Scenario /// [`Step`]: gherkin::Step after_hook: Option, + + /// Indicates, whether execution should be stopped after the first failure. + /// But all already started [`Scenario`]s will be finished. + /// + /// [`Scenario`]: gherkin::Scenario + fail_fast: bool, } // Implemented manually to omit redundant trait bounds on `World` and to omit @@ -171,6 +186,7 @@ impl Basic { which_scenario: (), before_hook: None, after_hook: None, + fail_fast: false, } } } @@ -192,6 +208,7 @@ impl Default for Basic { which_scenario, before_hook: None, after_hook: None, + fail_fast: false, } } } @@ -210,6 +227,13 @@ impl Basic { self } + /// Run tests until the first failure. + #[must_use] + pub const fn fail_fast(mut self) -> Self { + self.fail_fast = true; + self + } + /// Function determining whether a [`Scenario`] is [`Concurrent`] or /// a [`Serial`] one. /// @@ -231,6 +255,7 @@ impl Basic { steps, before_hook, after_hook, + fail_fast, .. } = self; Basic { @@ -239,6 +264,7 @@ impl Basic { which_scenario: func, before_hook, after_hook, + fail_fast, } } @@ -263,6 +289,7 @@ impl Basic { steps, which_scenario, after_hook, + fail_fast, .. } = self; Basic { @@ -271,6 +298,7 @@ impl Basic { which_scenario, before_hook: Some(func), after_hook, + fail_fast, } } @@ -304,6 +332,7 @@ impl Basic { steps, which_scenario, before_hook, + fail_fast, .. } = self; Basic { @@ -312,6 +341,7 @@ impl Basic { which_scenario, before_hook, after_hook: Some(func), + fail_fast, } } @@ -392,8 +422,10 @@ where which_scenario, before_hook, after_hook, + fail_fast, } = self; + let fail_fast = if cli.fail_fast { true } else { fail_fast }; let buffer = Features::default(); let (sender, receiver) = mpsc::unbounded(); @@ -402,6 +434,7 @@ where features, which_scenario, sender.clone(), + fail_fast, ); let execute = execute( buffer, @@ -410,6 +443,7 @@ where sender, before_hook, after_hook, + fail_fast, ); stream::select( @@ -436,6 +470,7 @@ async fn insert_features( features: S, which_scenario: F, sender: mpsc::UnboundedSender>>>, + fail_fast: bool, ) where S: Stream> + 'static, F: Fn( @@ -452,7 +487,7 @@ async fn insert_features( // If the receiver end is dropped, then no one listens for events // so we can just stop from here. Err(e) => { - if sender.unbounded_send(Err(e)).is_err() { + if sender.unbounded_send(Err(e)).is_err() || fail_fast { break; } } @@ -482,6 +517,7 @@ async fn execute( >, before_hook: Option, after_hook: Option, + fail_fast: bool, ) where W: World, Before: 'static @@ -522,10 +558,16 @@ async fn execute( executor.send_event(event::Cucumber::Started); - let mut started_scenarios = max_concurrent_scenarios; + // TODO: replace with ControlFlow::map_break once stabilized. + let map_break = |cf| match cf { + ControlFlow::Continue(cont) => cont, + ControlFlow::Break(()) => Some(0), + }; + + let mut started_scenarios = ControlFlow::Continue(max_concurrent_scenarios); let mut run_scenarios = stream::FuturesUnordered::new(); loop { - let runnable = features.get(started_scenarios).await; + let runnable = features.get(map_break(started_scenarios)).await; if run_scenarios.is_empty() && runnable.is_empty() { if features.is_finished() { break; @@ -536,7 +578,7 @@ async fn execute( let started = storage.start_scenarios(&runnable); executor.send_all_events(started); - if let Some(sc) = started_scenarios.as_mut() { + if let ControlFlow::Continue(Some(sc)) = &mut started_scenarios { *sc -= runnable.len(); } @@ -545,12 +587,13 @@ async fn execute( } if run_scenarios.next().await.is_some() { - if let Some(sc) = started_scenarios.as_mut() { + if let ControlFlow::Continue(Some(sc)) = &mut started_scenarios { *sc += 1; } } - while let Ok(Some((feat, rule))) = storage.finished_receiver.try_next() + while let Ok(Some((feat, rule, scenario_failed))) = + storage.finished_receiver.try_next() { if let Some(r) = rule { if let Some(f) = @@ -563,9 +606,17 @@ async fn execute( if let Some(f) = storage.feature_scenario_finished(feat) { executor.send_event(f); } + + if fail_fast && scenario_failed { + started_scenarios = ControlFlow::Break(()); + } } } + // This is done in case because of `fail_fast` not all Scenarios were + // executed. + executor.send_all_events(storage.finish_all_rules_and_features()); + executor.send_event(event::Cucumber::Finished); panic::set_hook(hook); @@ -608,6 +659,7 @@ struct Executor { finished_sender: mpsc::UnboundedSender<( Arc, Option>, + Failed, )>, } @@ -639,6 +691,7 @@ where finished_sender: mpsc::UnboundedSender<( Arc, Option>, + Failed, )>, ) -> Self { Self { @@ -711,6 +764,7 @@ where event::Scenario::Started, )); + let mut is_failed = false; let world = async { let before_hook = self .run_before_hook(&feature, rule.as_ref(), &scenario) @@ -757,12 +811,17 @@ where }) .await } + .inspect_err(|e| { + if e.is_none() { + is_failed = true; + } + }) .await .unwrap_or_else(identity); self.run_after_hook(world, &feature, rule.as_ref(), &scenario) .await - .map_or((), drop); + .map_or_else(|_| is_failed = true, drop); self.send_event(event::Cucumber::scenario( Arc::clone(&feature), @@ -771,7 +830,7 @@ where event::Scenario::Finished, )); - self.scenario_finished(feature, rule); + self.scenario_finished(feature, rule, is_failed); } /// Executes [`HookType::Before`], if present. @@ -1009,10 +1068,14 @@ where &self, feature: Arc, rule: Option>, + is_failed: Failed, ) { // If the receiver end is dropped, then no one listens for events // so we can just ignore it. - drop(self.finished_sender.unbounded_send((feature, rule))); + drop( + self.finished_sender + .unbounded_send((feature, rule, is_failed)), + ); } /// Notifies with the given [`Cucumber`] event. @@ -1055,12 +1118,14 @@ struct FinishedRulesAndFeatures { /// Number of finished [`Scenario`]s of [`Rule`]. /// - /// We also store path to `.feature` file so [`Rule`]s with same names and - /// spans in different files will have different hashes. + /// We also store path to [`Feature`] so [`Rule`]s with same names and + /// spans in different `.feature` files will have different hashes. /// + /// [`Feature`]: gherkin::Feature /// [`Rule`]: gherkin::Rule /// [`Scenario`]: gherkin::Scenario - rule_scenarios_count: HashMap<(Option, Arc), usize>, + rule_scenarios_count: + HashMap<(Arc, Arc), usize>, /// Receiver for notifying state of [`Scenario`]s completion. /// @@ -1068,6 +1133,7 @@ struct FinishedRulesAndFeatures { finished_receiver: mpsc::UnboundedReceiver<( Arc, Option>, + Failed, )>, } @@ -1077,6 +1143,7 @@ impl FinishedRulesAndFeatures { finished_receiver: mpsc::UnboundedReceiver<( Arc, Option>, + Failed, )>, ) -> Self { Self { @@ -1099,13 +1166,13 @@ impl FinishedRulesAndFeatures { ) -> Option> { let finished_scenarios = self .rule_scenarios_count - .get_mut(&(feature.path.clone(), Arc::clone(&rule))) + .get_mut(&(Arc::clone(&feature), Arc::clone(&rule))) .unwrap_or_else(|| panic!("No Rule {}", rule.name)); *finished_scenarios += 1; (rule.scenarios.len() == *finished_scenarios).then(|| { let _ = self .rule_scenarios_count - .remove(&(feature.path.clone(), Arc::clone(&rule))); + .remove(&(Arc::clone(&feature), Arc::clone(&rule))); event::Cucumber::rule_finished(feature, rule) }) } @@ -1132,6 +1199,24 @@ impl FinishedRulesAndFeatures { }) } + /// Marks all [`Rule`]s and [`Feature`]s as finished and returns + /// finished events. + /// + /// [`Feature`]: gherkin::Feature + /// [`Rule`]: gherkin::Rule + fn finish_all_rules_and_features( + &mut self, + ) -> impl Iterator> + '_ { + self.rule_scenarios_count + .drain() + .map(|((feat, rule), _)| event::Cucumber::rule_finished(feat, rule)) + .chain( + self.features_scenarios_count + .drain() + .map(|(feat, _)| event::Cucumber::feature_finished(feat)), + ) + } + /// Marks [`Scenario`]s as started and returns [`Rule::Started`] and /// [`Feature::Started`] if given [`Scenario`] was first for particular /// [`Rule`] or [`Feature`]. @@ -1174,7 +1259,7 @@ impl FinishedRulesAndFeatures { { let _ = self .rule_scenarios_count - .entry((feat.path.clone(), Arc::clone(&rule))) + .entry((Arc::clone(&feat), Arc::clone(&rule))) .or_insert_with(|| { started_rules.push((feat, rule)); 0 From c975c225effc9b2b4243902f853850d878564c31 Mon Sep 17 00:00:00 2001 From: ilslv Date: Fri, 7 Jan 2022 12:56:15 +0300 Subject: [PATCH 6/8] Docs --- book/src/cli.md | 3 +++ src/runner/basic.rs | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/book/src/cli.md b/book/src/cli.md index 2a9d3e1b..c51c8853 100644 --- a/book/src/cli.md +++ b/book/src/cli.md @@ -25,6 +25,9 @@ OPTIONS: Coloring policy for a console output [default: auto] + + --fail-fast + Runs tests until a first failure -h, --help Print help information diff --git a/src/runner/basic.rs b/src/runner/basic.rs index e44c1a85..b84efb44 100644 --- a/src/runner/basic.rs +++ b/src/runner/basic.rs @@ -49,7 +49,7 @@ pub struct Cli { #[clap(long, short, name = "int")] pub concurrency: Option, - /// Runs tests until first failure. + /// Runs tests until a first failure. #[clap(long)] pub fail_fast: bool, } From 4b47cc4b24ab8908546beff95ba367da6c302c40 Mon Sep 17 00:00:00 2001 From: ilslv Date: Fri, 7 Jan 2022 13:10:04 +0300 Subject: [PATCH 7/8] CHANGELOG --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a55d918..09a5c917 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,11 +11,16 @@ All user visible changes to `cucumber` crate will be documented in this file. Th [Diff](/../../compare/v0.11.0...v0.11.1) | [Milestone](/../../milestone/6) +### Added + +- Add `--fail-fast` CLI option to `runner::Basic`. ([#196]) + ### Changed - Optimized `runner::Basic` to not wait the whole batch to complete before executing next `Scenario`s. ([#195]) [#195]: /../../pull/195 +[#196]: /../../pull/196 From 9be78d2b98d1f50fe9b94b288e5c4c83775f467f Mon Sep 17 00:00:00 2001 From: tyranron Date: Fri, 7 Jan 2022 15:41:31 +0200 Subject: [PATCH 8/8] Corrections --- CHANGELOG.md | 2 +- book/src/cli.md | 2 +- book/src/quickstart.md | 2 ++ book/src/writing/asserting.md | 2 ++ src/runner/basic.rs | 28 +++++++++++++++------------- 5 files changed, 21 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09a5c917..c6d5e5e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ All user visible changes to `cucumber` crate will be documented in this file. Th ### Added -- Add `--fail-fast` CLI option to `runner::Basic`. ([#196]) +- `--fail-fast` CLI option to `runner::Basic`. ([#196]) ### Changed diff --git a/book/src/cli.md b/book/src/cli.md index c51c8853..b4d4b010 100644 --- a/book/src/cli.md +++ b/book/src/cli.md @@ -27,7 +27,7 @@ OPTIONS: [default: auto] --fail-fast - Runs tests until a first failure + Run tests until the first failure -h, --help Print help information diff --git a/book/src/quickstart.md b/book/src/quickstart.md index 6b6c47f5..9fd6d38e 100644 --- a/book/src/quickstart.md +++ b/book/src/quickstart.md @@ -274,6 +274,8 @@ fn cat_is_fed(world: &mut AnimalWorld) { And see the test failing: ![record](rec/quickstart_simple_fail.gif) +> __TIP__: By default, unlike [unit tests](https://doc.rust-lang.org/cargo/commands/cargo-test.html#test-options), failed [step]s don't terminate the execution instantly, and the whole test suite is executed regardless of them. Use `--fail-fast` [CLI] option to stop execution on first failure. + What if we also want to validate that even if the cat was never hungry to begin with, it won't end up hungry after it was fed? So, we may add an another [scenario] that looks quite similar: ```gherkin Feature: Animal feature diff --git a/book/src/writing/asserting.md b/book/src/writing/asserting.md index ea2bd8bb..2c0fdc4f 100644 --- a/book/src/writing/asserting.md +++ b/book/src/writing/asserting.md @@ -76,6 +76,8 @@ fn cat_is_fed(_: &mut AnimalWorld) { > __TIP__: To additionally print the state of the `World` at the moment of failure, increase output verbosity via `-vv` [CLI] option. +> __TIP__: By default, unlike [unit tests](https://doc.rust-lang.org/cargo/commands/cargo-test.html#test-options), failed [step]s don't terminate the execution instantly, and the whole test suite is executed regardless of them. Use `--fail-fast` [CLI] option to stop execution on first failure. + diff --git a/src/runner/basic.rs b/src/runner/basic.rs index 31a4c457..5fcbcdc7 100644 --- a/src/runner/basic.rs +++ b/src/runner/basic.rs @@ -49,7 +49,7 @@ pub struct Cli { #[clap(long, short, name = "int")] pub concurrency: Option, - /// Runs tests until a first failure. + /// Run tests until the first failure. #[clap(long)] pub fail_fast: bool, } @@ -158,10 +158,7 @@ pub struct Basic< /// [`Step`]: gherkin::Step after_hook: Option, - /// Indicates, whether execution should be stopped after the first failure. - /// But all already started [`Scenario`]s will be finished. - /// - /// [`Scenario`]: gherkin::Scenario + /// Indicates whether execution should be stopped after the first failure. fail_fast: bool, } @@ -228,6 +225,11 @@ impl Basic { } /// Run tests until the first failure. + /// + /// __NOTE__: All the already started [`Scenario`]s at the moment of failure + /// will be finished. + /// + /// [`Scenario`]: gherkin::Scenario #[must_use] pub const fn fail_fast(mut self) -> Self { self.fail_fast = true; @@ -558,7 +560,8 @@ async fn execute( executor.send_event(event::Cucumber::Started); - // TODO: replace with ControlFlow::map_break once stabilized. + // TODO: Replace with `ControlFlow::map_break()` once stabilized: + // https://github.com/rust-lang/rust/issues/75744 let map_break = |cf| match cf { ControlFlow::Continue(cont) => cont, ControlFlow::Break(()) => Some(0), @@ -602,7 +605,6 @@ async fn execute( executor.send_event(f); } } - if let Some(f) = storage.feature_scenario_finished(feat) { executor.send_event(f); } @@ -613,8 +615,8 @@ async fn execute( } } - // This is done in case because of `fail_fast` not all Scenarios were - // executed. + // This is done in case of `fail_fast: true`, when not all `Scenario`s might + // be executed. executor.send_all_events(storage.finish_all_rules_and_features()); executor.send_event(event::Cucumber::Finished); @@ -1118,7 +1120,7 @@ struct FinishedRulesAndFeatures { /// Number of finished [`Scenario`]s of [`Rule`]. /// - /// We also store path to [`Feature`] so [`Rule`]s with same names and + /// We also store path to a [`Feature`], so [`Rule`]s with same names and /// spans in different `.feature` files will have different hashes. /// /// [`Feature`]: gherkin::Feature @@ -1138,7 +1140,7 @@ struct FinishedRulesAndFeatures { } impl FinishedRulesAndFeatures { - /// Creates a new [`Executor`]. + /// Creates a new [`FinishedRulesAndFeatures`] store. fn new( finished_receiver: mpsc::UnboundedReceiver<( Arc, @@ -1199,8 +1201,8 @@ impl FinishedRulesAndFeatures { }) } - /// Marks all [`Rule`]s and [`Feature`]s as finished and returns - /// finished events. + /// Marks all the unfinished [`Rule`]s and [`Feature`]s as finished, and + /// returns all the appropriate finished events. /// /// [`Feature`]: gherkin::Feature /// [`Rule`]: gherkin::Rule