Pass event::ScenarioFinished to after hook (#246, #245)

Additionally:
- catch panics in user code, when they happen before returned futures are polled

Co-authored-by: Kai Ren <[email protected]>

Author: ilslv

CHANGELOG.md

@@ -6,6 +6,26 @@ All user visible changes to `cucumber` crate will be documented in this file. Th
 
 
 
+## [0.17.0] · 2022-??-??
+[0.17.0]: /../../tree/v0.17.0
+
+[Diff](/../../compare/v0.16.0...v0.17.0) | [Milestone](/../../milestone/20)
+
+### BC Breaks
+
+- Added `event::ScenarioFinished` as [`Cucumber::after`][0170-1] hook's argument, explaining why the `Scenario` has finished. ([#246], [#245])
+
+### Fixed
+
+- Uncaught panics of user code, when they happen before first poll of the returned `Future`s. ([#246])
+
+[#245]: /../../discussions/245
+[#246]: /../../pull/246
+[0170-1]: https://docs.rs/cucumber/0.17.0/cucumber/struct.Cucumber.html#method.after
+
+
+
+
 ## [0.16.0] · 2022-11-09
 [0.16.0]: /../../tree/v0.16.0
 

book/src/writing/hooks.md

@@ -61,7 +61,7 @@ World::cucumber()
 # 
 # fn main() {
 World::cucumber()
-    .after(|_feature, _rule, _scenario, _world| {
+    .after(|_feature, _rule, _scenario, _ev, _world| {
         time::sleep(Duration::from_millis(300)).boxed_local()
     })
     .run_and_exit("tests/features/book");
@@ -70,12 +70,15 @@ World::cucumber()
 
 > __NOTE__: [`After` hook] is enabled globally for all the executed [scenario]s. No exception is possible.
 
+> __TIP__: [`After` hook] receives an [`event::ScenarioFinished`] as one of its arguments, which indicates why the [scenario] has finished (passed, failed or skipped). This information, for example, may be used to decide whether some external resources (like files) should be cleaned up if the [scenario] passes, or leaved "as is" if it fails, so helping to "freeze" the failure conditions for better investigation. 
+
 
 
 
 [`After` hook]: https://cucumber.io/docs/cucumber/api#after
 [`Background`]: background.md
-[`Before` hook]: https://cucumber.io/docs/cucumber/api#before 
+[`Before` hook]: https://cucumber.io/docs/cucumber/api#before
+[`event::ScenarioFinished`]: https://docs.rs/cucumber/*/cucumber/event/struct.ScenarioFinished.html
 [hook]: https://cucumber.io/docs/cucumber/api#scenario-hooks
 [scenario]: https://cucumber.io/docs/gherkin/reference#example
 [step]: https://cucumber.io/docs/gherkin/reference#steps

src/cucumber.rs

@@ -922,6 +922,7 @@ where
             &'a gherkin::Feature,
             Option<&'a gherkin::Rule>,
             &'a gherkin::Scenario,
+            &'a event::ScenarioFinished,
             Option<&'a mut W>,
         ) -> LocalBoxFuture<'a, ()>
         + 'static,
@@ -1083,6 +1084,7 @@ where
                 &'a gherkin::Feature,
                 Option<&'a gherkin::Rule>,
                 &'a gherkin::Scenario,
+                &'a event::ScenarioFinished,
                 Option<&'a mut W>,
             ) -> LocalBoxFuture<'a, ()>
             + 'static,

src/event.rs

@@ -657,3 +657,28 @@ impl<World> Clone for RetryableScenario<World> {
         }
     }
 }
+
+/// Event explaining why a [Scenario] has finished.
+///
+/// [Scenario]: https://cucumber.io/docs/gherkin/reference/#example
+#[allow(variant_size_differences)]
+#[derive(Clone, Debug)]
+pub enum ScenarioFinished {
+    /// [`Before`] [`Hook::Failed`].
+    ///
+    /// [`Before`]: HookType::Before
+    BeforeHookFailed(Info),
+
+    /// [`Step::Passed`].
+    StepPassed,
+
+    /// [`Step::Skipped`].
+    StepSkipped,
+
+    /// [`Step::Failed`].
+    StepFailed(
+        Option<regex::CaptureLocations>,
+        Option<step::Location>,
+        StepError,
+    ),
+}

src/runner/basic.rs

@@ -294,6 +294,7 @@ pub type AfterHookFn<World> = for<'a> fn(
     &'a gherkin::Feature,
     Option<&'a gherkin::Rule>,
     &'a gherkin::Scenario,
+    &'a event::ScenarioFinished,
     Option<&'a mut World>,
 ) -> LocalBoxFuture<'a, ()>;
 
@@ -619,6 +620,7 @@ impl<World, Which, Before, After> Basic<World, Which, Before, After> {
             &'a gherkin::Feature,
             Option<&'a gherkin::Rule>,
             &'a gherkin::Scenario,
+            &'a event::ScenarioFinished,
             Option<&'a mut World>,
         ) -> LocalBoxFuture<'a, ()>,
     {
@@ -706,6 +708,7 @@ where
             &'a gherkin::Feature,
             Option<&'a gherkin::Rule>,
             &'a gherkin::Scenario,
+            &'a event::ScenarioFinished,
             Option<&'a mut W>,
         ) -> LocalBoxFuture<'a, ()>
         + 'static,
@@ -873,6 +876,7 @@ async fn execute<W, Before, After>(
             &'a gherkin::Feature,
             Option<&'a gherkin::Rule>,
             &'a gherkin::Scenario,
+            &'a event::ScenarioFinished,
             Option<&'a mut W>,
         ) -> LocalBoxFuture<'a, ()>,
 {
@@ -1046,6 +1050,7 @@ where
             &'a gherkin::Feature,
             Option<&'a gherkin::Rule>,
             &'a gherkin::Scenario,
+            &'a event::ScenarioFinished,
             Option<&'a mut W>,
         ) -> LocalBoxFuture<'a, ()>,
 {
@@ -1176,13 +1181,22 @@ where
         }
         .await;
 
-        let world = match &mut result {
-            Ok(world) => world.take(),
-            Err(exec_err) => exec_err.take_world(),
+        let (world, scenario_finished_ev) = match &mut result {
+            Ok(world) => (world.take(), event::ScenarioFinished::StepPassed),
+            Err(exec_err) => (
+                exec_err.take_world(),
+                exec_err.get_scenario_finished_event(),
+            ),
         };
 
         let (world, after_hook_meta, after_hook_error) = self
-            .run_after_hook(world, &feature, rule.as_ref(), &scenario)
+            .run_after_hook(
+                world,
+                &feature,
+                rule.as_ref(),
+                &scenario,
+                scenario_finished_ev,
+            )
             .await
             .map_or_else(
                 |(w, meta, info)| (w.map(Arc::new), Some(meta), Some(info)),
@@ -1260,7 +1274,7 @@ where
         retries: Option<Retries>,
     ) -> Result<Option<W>, ExecutionFailure<W>> {
         let init_world = async {
-            AssertUnwindSafe(W::new())
+            AssertUnwindSafe(async { W::new().await })
                 .catch_unwind()
                 .await
                 .map_err(Info::from)
@@ -1284,12 +1298,15 @@ where
             ));
 
             let fut = init_world.and_then(|mut world| async {
-                let fut = (hook)(
-                    feature.as_ref(),
-                    rule.as_ref().map(AsRef::as_ref),
-                    scenario.as_ref(),
-                    &mut world,
-                );
+                let fut = async {
+                    (hook)(
+                        feature.as_ref(),
+                        rule.as_ref().map(AsRef::as_ref),
+                        scenario.as_ref(),
+                        &mut world,
+                    )
+                    .await;
+                };
                 match AssertUnwindSafe(fut).catch_unwind().await {
                     Ok(()) => Ok(world),
                     Err(i) => Err((Info::from(i), Some(world))),
@@ -1361,7 +1378,10 @@ where
             let mut world = if let Some(w) = world {
                 w
             } else {
-                match AssertUnwindSafe(W::new()).catch_unwind().await {
+                match AssertUnwindSafe(async { W::new().await })
+                    .catch_unwind()
+                    .await
+                {
                     Ok(Ok(w)) => w,
                     Ok(Err(e)) => {
                         let e = event::StepError::Panic(coerce_into_info(
@@ -1376,7 +1396,7 @@ where
                 }
             };
 
-            match AssertUnwindSafe(step_fn(&mut world, ctx))
+            match AssertUnwindSafe(async { step_fn(&mut world, ctx).await })
                 .catch_unwind()
                 .await
             {
@@ -1512,17 +1532,22 @@ where
         feature: &Arc<gherkin::Feature>,
         rule: Option<&Arc<gherkin::Rule>>,
         scenario: &Arc<gherkin::Scenario>,
+        ev: event::ScenarioFinished,
     ) -> Result<
         (Option<W>, Option<AfterHookEventsMeta>),
         (Option<W>, AfterHookEventsMeta, Info),
     > {
         if let Some(hook) = self.after_hook.as_ref() {
-            let fut = (hook)(
-                feature.as_ref(),
-                rule.as_ref().map(AsRef::as_ref),
-                scenario.as_ref(),
-                world.as_mut(),
-            );
+            let fut = async {
+                (hook)(
+                    feature.as_ref(),
+                    rule.as_ref().map(AsRef::as_ref),
+                    scenario.as_ref(),
+                    &ev,
+                    world.as_mut(),
+                )
+                .await;
+            };
 
             let started = event::Metadata::new(());
             let res = AssertUnwindSafe(fut).catch_unwind().await;
@@ -2201,6 +2226,23 @@ impl<W> ExecutionFailure<W> {
             | Self::StepPanicked { world, .. } => world.take(),
         }
     }
+
+    /// Creates an [`event::ScenarioFinished`] from this [`ExecutionFailure`].
+    fn get_scenario_finished_event(&self) -> event::ScenarioFinished {
+        use event::ScenarioFinished::{
+            BeforeHookFailed, StepFailed, StepSkipped,
+        };
+
+        match self {
+            Self::BeforeHookPanicked { panic_info, .. } => {
+                BeforeHookFailed(Arc::clone(panic_info))
+            }
+            Self::StepSkipped(_) => StepSkipped,
+            Self::StepPanicked {
+                captures, loc, err, ..
+            } => StepFailed(captures.clone(), *loc, err.clone()),
+        }
+    }
 }
 
 #[cfg(test)]

tests/after_hook.rs

@@ -1,4 +1,5 @@
 use std::{
+    future,
     panic::AssertUnwindSafe,
     sync::atomic::{AtomicUsize, Ordering},
     time::Duration,
@@ -11,6 +12,10 @@ use tokio::time;
 
 static NUMBER_OF_BEFORE_WORLDS: AtomicUsize = AtomicUsize::new(0);
 static NUMBER_OF_AFTER_WORLDS: AtomicUsize = AtomicUsize::new(0);
+static NUMBER_OF_FAILED_HOOKS: AtomicUsize = AtomicUsize::new(0);
+static NUMBER_OF_PASSED_STEPS: AtomicUsize = AtomicUsize::new(0);
+static NUMBER_OF_SKIPPED_STEPS: AtomicUsize = AtomicUsize::new(0);
+static NUMBER_OF_FAILED_STEPS: AtomicUsize = AtomicUsize::new(0);
 
 #[tokio::main]
 async fn main() {
@@ -23,17 +28,28 @@ async fn main() {
             }
             .boxed()
         })
-        .after(move |_, _, _, w| {
-            async move {
-                if w.is_some() {
-                    let after =
-                        NUMBER_OF_AFTER_WORLDS.fetch_add(1, Ordering::SeqCst);
-                    assert_ne!(after, 8, "Too much after `World`s!");
-                } else {
-                    panic!("No World received");
-                }
+        .after(move |_, _, _, ev, w| {
+            use cucumber::event::ScenarioFinished::{
+                BeforeHookFailed, StepFailed, StepPassed, StepSkipped,
+            };
+
+            match ev {
+                BeforeHookFailed(_) => &NUMBER_OF_FAILED_HOOKS,
+                StepPassed => &NUMBER_OF_PASSED_STEPS,
+                StepSkipped => &NUMBER_OF_SKIPPED_STEPS,
+                StepFailed(_, _, _) => &NUMBER_OF_FAILED_STEPS,
             }
-            .boxed()
+            .fetch_add(1, Ordering::SeqCst);
+
+            if w.is_some() {
+                let after =
+                    NUMBER_OF_AFTER_WORLDS.fetch_add(1, Ordering::SeqCst);
+                assert_ne!(after, 8, "Too much after `World`s!");
+            } else {
+                panic!("No `World` received");
+            }
+
+            future::ready(()).boxed()
         })
         .run_and_exit("tests/features/wait");
 
@@ -46,6 +62,10 @@ async fn main() {
     assert_eq!(err, "2 steps failed, 1 parsing error, 4 hook errors");
     assert_eq!(NUMBER_OF_BEFORE_WORLDS.load(Ordering::SeqCst), 11);
     assert_eq!(NUMBER_OF_AFTER_WORLDS.load(Ordering::SeqCst), 11);
+    assert_eq!(NUMBER_OF_FAILED_HOOKS.load(Ordering::SeqCst), 2);
+    assert_eq!(NUMBER_OF_PASSED_STEPS.load(Ordering::SeqCst), 6);
+    assert_eq!(NUMBER_OF_SKIPPED_STEPS.load(Ordering::SeqCst), 2);
+    assert_eq!(NUMBER_OF_FAILED_STEPS.load(Ordering::SeqCst), 2);
 }
 
 #[given(regex = r"(\d+) secs?")]

tests/json.rs

@@ -28,7 +28,7 @@ async fn main() {
                 }
                 .boxed_local()
             })
-            .after(|_, _, sc, _| {
+            .after(|_, _, sc, _, _| {
                 async {
                     assert!(!sc.tags.iter().any(|t| t == "fail_after"), "Tag!");
                 }

tests/wait.rs

@@ -28,7 +28,7 @@ async fn main() {
             }
             .boxed_local()
         })
-        .after(move |_, _, _, _| time::sleep(cli.custom.pause).boxed_local())
+        .after(move |_, _, _, _, _| time::sleep(cli.custom.pause).boxed_local())
         .with_writer(writer::Libtest::or_basic())
         .with_cli(cli)
         .run_and_exit("tests/features/wait");