feat(cucumber): timed out steps and scenarios are correctly reported

affects: @serenity-js/core, @serenity-js/cucumber, @integration/cucumber,
@serenity-js-examples/cucumber

examples/cucumber/features/reporting_results/narrative.txt

@@ -1,4 +1,4 @@
-Reporting Results
+Reporting results
 
 Narrative:
 In order to quickly learn how to use Serenity/JS with Cucumber

examples/cucumber/features/reporting_results/reports_timed_out_scenarios.feature

@@ -0,0 +1,14 @@
+Feature: Reports timed out scenarios
+
+  In order to see how Serenity/JS reports a timed out Cucumber scenario
+  As a curious developer
+  I'd like to see an example implementation
+
+  Scenario: A timed out scenario
+
+    Here's an example of a scenario that times out.
+    Should at least one of the steps time out, the subsequent steps are skipped and the entire scenario is marked as timed out.
+
+    Given a step that passes
+      And a step that times out
+      And a step that passes

examples/cucumber/features/step_definitions/sample.steps.ts

@@ -21,7 +21,7 @@ export = function() {
         return Promise.resolve();
     });
 
-    this.Given(/^.*step times out$/,  { timeout: 100 }, function() {
+    this.Given(/^.*step.*times out$/,  { timeout: 100 }, function() {
         return new Promise((resolve, reject) => {
             setTimeout(resolve, 1000);
         });

integration/cucumber/features/step_definitions/callback.steps.ts

@@ -23,7 +23,7 @@ export = function() {
         done();
     });
 
-    this.Given(/^.*step times out$/,  { timeout: 100 }, function(done: Callback) {
+    this.Given(/^.*step that times out$/,  { timeout: 100 }, function(done: Callback) {
         setTimeout(done, 1000);
     });
 };

integration/cucumber/features/step_definitions/promise.steps.ts

@@ -21,7 +21,7 @@ export = function() {
         return Promise.resolve();
     });
 
-    this.Given(/^.*step times out$/,  { timeout: 100 }, function() {
+    this.Given(/^.*step that times out$/,  { timeout: 100 }, function() {
         return new Promise((resolve, reject) => {
             setTimeout(resolve, 1000);
         });

integration/cucumber/features/step_definitions/synchronous.steps.ts

@@ -20,8 +20,4 @@ export = function() {
     this.Given(/^.*step (?:.*) receives a doc string:$/, function(docstring: string) {
         return void 0;
     });
-
-    this.Given(/^.*step times out$/,  { timeout: 100 }, function(done: (error: Error, pending: string) => void) {
-        setTimeout(done, 1000);
-    });
 };

integration/cucumber/features/timed_out_scenario.feature

@@ -0,0 +1,6 @@
+Feature: Serenity/JS recognises a timed out scenario
+
+  Scenario: A timed out scenario
+
+    Given a step that times out
+

integration/cucumber/spec/timed_out_scenario.spec.ts

@@ -0,0 +1,53 @@
+import { expect, ifExitCodeIsOtherThan, logOutput } from '@integration/testing-tools';
+import {
+    ActivityFinished,
+    ActivityStarts,
+    SceneFinished,
+    SceneStarts,
+    SceneTagged,
+    TestRunnerDetected,
+} from '@serenity-js/core/lib/events';
+import { ExecutionFailedWithError, FeatureTag, Name } from '@serenity-js/core/lib/model';
+
+import 'mocha';
+import { given } from 'mocha-testdata';
+
+import { cucumber, Pick } from '../src';
+
+describe('@serenity-js/cucumber', function() {
+
+    this.timeout(5000);
+
+    given([
+        'promise',
+        'callback',
+    ]).
+    it('recognises a timed out scenario', (stepInterface: string) =>
+        cucumber(
+            '--require', 'features/support/configure_serenity.ts',
+            '--require', `features/step_definitions/${ stepInterface }.steps.ts`,
+            '--require', 'node_modules/@serenity-js/cucumber/register.js',
+            'features/timed_out_scenario.feature',
+        ).
+        then(ifExitCodeIsOtherThan(1, logOutput)).
+        then(res => {
+            expect(res.exitCode).to.equal(1);
+
+            expect(res.events).to.have.lengthOf(6);
+
+            Pick.from(res.events)
+                .next(SceneStarts,         event => expect(event.value.name).to.equal(new Name('A timed out scenario')))
+                .next(TestRunnerDetected,  event => expect(event.value).to.equal(new Name('Cucumber')))
+                .next(SceneTagged,         event => expect(event.tag).to.equal(new FeatureTag('Serenity/JS recognises a timed out scenario')))
+                .next(ActivityStarts,      event => expect(event.value.name).to.equal(new Name('Given a step that times out')))
+                .next(ActivityFinished,    event => {
+                    expect(event.outcome).to.be.instanceOf(ExecutionFailedWithError);
+                    expect((event.outcome as ExecutionFailedWithError).error.message).to.equal('function timed out after 100 milliseconds');
+                })
+                .next(SceneFinished,       event => {
+                    expect(event.outcome).to.be.instanceOf(ExecutionFailedWithError);
+                    expect((event.outcome as ExecutionFailedWithError).error.message).to.equal('function timed out after 100 milliseconds');
+                })
+            ;
+        }));
+});

packages/core/src/stage/crew/serenity-bdd-reporter/ErrorParser.ts

@@ -2,7 +2,6 @@ import * as ErrorStackParser from 'error-stack-parser';
 
 export class ErrorParser {
     parse(error: Error) {
-        // todo: add an instanceof check perhaps?
         return {
             errorType:    error.name,
             message:      error.message,

packages/cucumber/src/listener/Notifier.ts

@@ -96,27 +96,42 @@ export class Notifier {
     private scenarioOutcomeFrom(result: cucumber.events.ScenarioResultPayload): Outcome {
         const
             status: string = result.getStatus(),
-            error: Error   = result.getFailureException();
+            error: Error   = this.errorFrom(result.getFailureException());
 
         return this.outcomeFrom(status, error);
     }
 
     private stepOutcomeFrom(result: cucumber.events.StepResultPayload): Outcome {
+        const
+            status: string                          = result.getStatus(),
+            ambiguousStepsError: Error | undefined  = this.ambiguousStepsDetectedIn(result),
+            error: Error | undefined                = this.errorFrom(result.getFailureException());
+
+        return this.outcomeFrom(status, error || ambiguousStepsError);
+    }
+
+    private ambiguousStepsDetectedIn(result: cucumber.events.StepResultPayload): Error | undefined {
         const ambiguousStepDefinitions = result.getAmbiguousStepDefinitions() || [];
-        const ambiguousStepsDetected = ambiguousStepDefinitions.length > 0
-            ? ambiguousStepDefinitions
+
+        if (ambiguousStepDefinitions.length === 0) {
+            return void 0;
+        }
+
+        return ambiguousStepDefinitions
                 .map(step => `${step.getPattern().toString()} - ${step.getUri()}:${step.getLine()}`)
                 .reduce((err: Error, issue) => {
                     err.message += `\n${issue}`;
                     return err;
-                }, new Error('Each step should have one matching step definition, yet there are several:'))
-            : void 0;
-
-        const
-            status: string = result.getStatus(),
-            error: Error   = result.getFailureException() || ambiguousStepsDetected;
+                }, new Error('Each step should have one matching step definition, yet there are several:'));
+    }
 
-        return this.outcomeFrom(status, error);
+    private errorFrom(error: Error | string | undefined): Error | undefined {
+        switch (typeof error) {
+            case 'string':   return new Error(error as string);
+            case 'object':   return error as Error;
+            case 'function': return error as Error;
+            default:         return void 0;
+        }
     }
 
     private outcomeFrom(status: string, error?: Error) {