Fix not panicking on fail_on_skipped() with retries (#250, #249)

- add `NotFound` variant to `event::StepError`

CHANGELOG.md

@@ -6,6 +6,25 @@ All user visible changes to `cucumber` crate will be documented in this file. Th
 
 
 
+## [0.18.0] · 2022-??-??
+[0.18.0]: /../../tree/v0.18.0
+
+[Diff](/../../compare/v0.17.0...v0.18.0) | [Milestone](/../../milestone/21)
+
+### BC Breaks
+
+- Added `NotFound` variant to `event::StepError`. ([#250])
+
+### Fixed
+
+- Not panicking on `fail_on_skipped()` with retries. ([#250], [#249])
+
+[#249]: /../../issues/249
+[#250]: /../../pull/250
+
+
+
+
 ## [0.17.0] · 2022-11-23
 [0.17.0]: /../../tree/v0.17.0
 

Cargo.toml

@@ -19,7 +19,7 @@ repository = "https://github.com/cucumber-rs/cucumber"
 readme = "README.md"
 categories = ["asynchronous", "development-tools::testing"]
 keywords = ["cucumber", "testing", "bdd", "atdd", "async"]
-include = ["/src/", "/tests/after_hook.rs", "/tests/fail_fast.rs", "/tests/json.rs", "/tests/junit.rs", "/tests/libtest.rs", "/tests/retry.rs", "/tests/wait.rs", "/LICENSE-*", "/README.md", "/CHANGELOG.md"]
+include = ["/src/", "/tests/after_hook.rs", "/tests/fail_fast.rs", "/tests/json.rs", "/tests/junit.rs", "/tests/libtest.rs", "/tests/retry.rs", "/tests/retry_fail_on_skipped.rs", "/tests/wait.rs", "/LICENSE-*", "/README.md", "/CHANGELOG.md"]
 
 [package.metadata.docs.rs]
 all-features = true
@@ -105,6 +105,10 @@ harness = false
 name = "retry"
 harness = false
 
+[[test]]
+name = "retry_fail_on_skipped"
+harness = false
+
 [[test]]
 name = "wait"
 required-features = ["libtest"]

src/event.rs

@@ -8,6 +8,10 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// TODO: Only because of `derive_more` macros, try to remove on next
+//       `derive_more` upgrade.
+#![allow(clippy::use_self)]
+
 //! Key occurrences in a lifecycle of [Cucumber] execution.
 //!
 //! The top-level enum here is [`Cucumber`].
@@ -396,6 +400,16 @@ impl<World> Clone for Step<World> {
 /// [`Step`]: gherkin::Step
 #[derive(Clone, Debug, Display, Error, From)]
 pub enum StepError {
+    /// [`Step`] doesn't match any [`Regex`].
+    ///
+    /// It's emitted whenever a [`Step::Skipped`] event cannot be tolerated
+    /// (such as when [`fail_on_skipped()`] is used).
+    ///
+    /// [`Regex`]: regex::Regex
+    /// [`fail_on_skipped()`]: crate::WriterExt::fail_on_skipped()
+    #[display(fmt = "Step doesn't match any function")]
+    NotFound,
+
     /// [`Step`] matches multiple [`Regex`]es.
     ///
     /// [`Regex`]: regex::Regex

src/runner/basic.rs

@@ -1001,7 +1001,6 @@ struct Executor<W, Before, After> {
     /// [`Step`]s [`Collection`].
     ///
     /// [`Collection`]: step::Collection
-    /// [`Step`]: step::Step
     collection: step::Collection<W>,
 
     /// Function, executed on each [`Scenario`] before running all [`Step`]s,

src/writer/basic.rs

@@ -650,7 +650,12 @@ impl<Out: io::Write> Basic<Out> {
         self.clear_last_lines_if_term_present()?;
 
         let style = |s| {
-            if retries.filter(|r| r.left > 0).is_some() {
+            if retries
+                .filter(|r| {
+                    r.left > 0 && !matches!(err, event::StepError::NotFound)
+                })
+                .is_some()
+            {
                 self.styles.bright().retry(s)
             } else {
                 self.styles.err(s)
@@ -924,7 +929,12 @@ impl<Out: io::Write> Basic<Out> {
         self.clear_last_lines_if_term_present()?;
 
         let style = |s| {
-            if retries.filter(|r| r.left > 0).is_some() {
+            if retries
+                .filter(|r| {
+                    r.left > 0 && !matches!(err, event::StepError::NotFound)
+                })
+                .is_some()
+            {
                 self.styles.bright().retry(s)
             } else {
                 self.styles.err(s)

src/writer/fail_on_skipped.rs

@@ -68,17 +68,12 @@ where
     ) {
         use event::{
             Cucumber, Feature, RetryableScenario, Rule, Scenario, Step,
-            StepError::Panic,
+            StepError::NotFound,
         };
 
         let map_failed = |f: &Arc<_>, r: &Option<_>, sc: &Arc<_>| {
             if (self.should_fail)(f, r.as_deref(), sc) {
-                Step::Failed(
-                    None,
-                    None,
-                    None,
-                    Panic(Arc::new("not allowed to skip")),
-                )
+                Step::Failed(None, None, None, NotFound)
             } else {
                 Step::Skipped
             }

src/writer/json.rs

@@ -291,6 +291,7 @@ impl<Out: io::Write> Json<Out> {
             },
             event::Step::Failed(_, loc, _, err) => {
                 let status = match &err {
+                    event::StepError::NotFound => Status::Undefined,
                     event::StepError::AmbiguousMatch(..) => Status::Ambiguous,
                     event::StepError::Panic(..) => Status::Failed,
                 };
@@ -405,10 +406,7 @@ mod status {
         /// [`event::StepError::AmbiguousMatch`].
         Ambiguous,
 
-        /// Never constructed and is here only to fully describe
-        /// [JSON schema][1].
-        ///
-        /// [1]: https://github.com/cucumber/cucumber-json-schema
+        /// [`event::Step::Failed`] with an [`event::StepError::NotFound`].
         Undefined,
 
         /// Never constructed and is here only to fully describe

src/writer/libtest.rs

@@ -590,7 +590,12 @@ impl<W: Debug + World, Out: io::Write> Libtest<W, Out> {
                 }
             }
             Step::Failed(_, loc, world, err) => {
-                if retries.map(|r| r.left > 0).unwrap_or_default() {
+                if retries
+                    .map(|r| {
+                        r.left > 0 && !matches!(err, event::StepError::NotFound)
+                    })
+                    .unwrap_or_default()
+                {
                     self.retried += 1;
                 } else {
                     self.failed += 1;

src/writer/summarize.rs

@@ -369,8 +369,13 @@ impl<Writer> Summarize<Writer> {
                     .handled_scenarios
                     .insert((feature, rule, scenario), Skipped);
             }
-            Step::Failed(..) => {
-                if retries.filter(|r| r.left > 0).is_some() {
+            Step::Failed(_, _, _, err) => {
+                if retries
+                    .filter(|r| {
+                        r.left > 0 && !matches!(err, event::StepError::NotFound)
+                    })
+                    .is_some()
+                {
                     self.steps.retried += 1;
 
                     let inserted_before = self

tests/after_hook.rs

@@ -51,6 +51,7 @@ async fn main() {
 
             future::ready(()).boxed()
         })
+        .fail_on_skipped()
         .run_and_exit("tests/features/wait");
 
     let err = AssertUnwindSafe(res)
@@ -59,12 +60,12 @@ async fn main() {
         .expect_err("should err");
     let err = err.downcast_ref::<String>().unwrap();
 
-    assert_eq!(err, "2 steps failed, 1 parsing error, 4 hook errors");
+    assert_eq!(err, "4 steps failed, 1 parsing error, 8 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_HOOKS.load(Ordering::SeqCst), 4);
+    assert_eq!(NUMBER_OF_PASSED_STEPS.load(Ordering::SeqCst), 4);
+    assert_eq!(NUMBER_OF_SKIPPED_STEPS.load(Ordering::SeqCst), 4);
     assert_eq!(NUMBER_OF_FAILED_STEPS.load(Ordering::SeqCst), 2);
 }
 

tests/features/wait/nested/rule.feature

@@ -9,6 +9,13 @@ Feature: Basic
     Then unknown
     Then 1 sec
 
+  @allow.skipped
+  Scenario: 1 sec
+    Given 1 sec
+    When 1 sec
+    Then unknown
+    Then 1 sec
+
   Rule: rule
     @fail_before
     Scenario: 2 secs

tests/features/wait/rule.feature

@@ -9,6 +9,13 @@ Feature: Basic
     Then unknown
     Then 1 sec
 
+  @allow.skipped
+  Scenario: 1 sec
+    Given 1 sec
+    When 1 sec
+    Then unknown
+    Then 1 sec
+
   Rule: rule
     @fail_before
     Scenario: 2 secs

tests/json.rs

@@ -35,6 +35,7 @@ async fn main() {
                 .boxed_local()
             })
             .with_writer(writer::Json::new(file.reopen().unwrap()))
+            .fail_on_skipped()
             .run("tests/features/wait")
             .await,
     );