fix undefined hook parameter (#919)

docs/support_files/api_reference.md

@@ -34,7 +34,7 @@ Defines a hook which is run after each scenario.
   * `tags`: string tag expression used to apply this hook to only specific scenarios. See [cucumber-tag-expressions](https://docs.cucumber.io/tag-expressions/) for more information
   * `timeout`: A hook-specific timeout, to override the default timeout.
 * `fn`: A function, defined as follows:
-  * The first argument will be a [ScenarioResult](/src/models/scenario_result.js)
+  * The first argument will be an object of the form `{sourceLocation: {line, uri}, result: {duration, status}}` matching the event data for `test-case-finished`
   * When using the asynchronous callback interface, have one final argument for the callback function.
 
 `options` can also be a string as a shorthand for specifying `tags`.
@@ -58,7 +58,7 @@ Multiple `AfterAll` hooks are executed in the **reverse** order that they are de
 
 #### `Before([options,] fn)`
 
-Defines a hook which is run before each scenario. Same interface as `After`.
+Defines a hook which is run before each scenario. Same interface as `After` except the first argument passed to `fn` will be an object of the form `{sourceLocation: {line, uri}}` matching the event data for `test-case-started`.
 
 Multiple `Before` hooks are executed in the order that they are defined.
 

docs/support_files/attachments.md

@@ -2,7 +2,7 @@
 
 Text, images and files can be added to the output of the JSON formatter with attachments.
 The world constructor is passed an `attach` function,
-which the default world constructor assigns to `this.attach`. If using a custom world constructor, 
+which the default world constructor assigns to `this.attach`. If using a custom world constructor,
 you need to do this as well if you want to add attachments.
 
 ```javascript
@@ -33,12 +33,12 @@ The data will be `base64` encoded in the output.
 You should wait for the stream to be read before continuing by providing a callback or waiting for the returned promise to resolve.
 
 ```javascript
-var {defineSupportCode} = require('cucumber');
+var {defineSupportCode, Status} = require('cucumber');
 
 defineSupportCode(function({After}) {
   // Passing a callback
-  After(function (scenarioResult, callback) {
-    if (scenarioResult.isFailed()) {
+  After(function (testCase, callback) {
+    if (testCase.result.status === Status.FAILED) {
       var stream = getScreenshotOfError();
       this.attach(stream, 'image/png', callback);
     }
@@ -48,8 +48,8 @@ defineSupportCode(function({After}) {
   });
 
   // Returning the promise
-  After(function (scenarioResult) {
-    if (scenarioResult.isFailed()) {
+  After(function (testCase) {
+    if (testCase.result.status === Status.FAILED) {
       var stream = getScreenshotOfError();
       return this.attach(stream, 'image/png');
     }
@@ -64,8 +64,8 @@ The data will be `base64` encoded in the output.
 var {defineSupportCode} = require('cucumber');
 
 defineSupportCode(function({After}) {
-  After(function (scenarioResult) {
-    if (scenarioResult.isFailed()) {
+  After(function (testCase) {
+    if (testCase.result.status === Status.FAILED) {
       var buffer = getScreenshotOfError();
       this.attach(buffer, 'image/png');
     }
@@ -80,9 +80,9 @@ when a scenario fails:
 var {defineSupportCode} = require('cucumber');
 
 defineSupportCode(function({After}) {
-    After(function (scenarioResult) {
+    After(function (testCase) {
     var world = this;
-    if (scenarioResult.isFailed()) {
+    if (testCase.result.status === Status.FAILED) {
       return webDriver.takeScreenshot().then(function(screenShot) {
         // screenShot is a base-64 encoded PNG
         world.attach(screenShot, 'image/png');

docs/support_files/hooks.md

@@ -1,6 +1,6 @@
 # Hooks
 
-Hooks are used for setup and teardown the environment before and after each scenario. The first argument will be a [ScenarioResult](/src/models/scenario_result.js) for the current running scenario. Multiple *Before* hooks are executed in the order that they were defined. Multiple *After* hooks are executed in the **reverse** order that they were defined.
+Hooks are used for setup and teardown the environment before and after each scenario. See the [API reference](./api_reference.md) for the specification of the first argument passed to hooks. Multiple *Before* hooks are executed in the order that they were defined. Multiple *After* hooks are executed in the **reverse** order that they were defined.
 
 ```javascript
 var {defineSupportCode} = require('cucumber');
@@ -12,7 +12,7 @@ defineSupportCode(function({After, Before}) {
   });
 
   // Asynchronous Callback
-  Before(function (scenarioResult, callback) {
+  Before(function (testCase, callback) {
     var world = this;
     tmp.dir({unsafeCleanup: true}, function(error, dir) {
       if (error) {
@@ -67,7 +67,7 @@ See more documentation on [tag expressions](https://docs.cucumber.io/tag-express
 
 ## BeforeAll / AfterAll
 
-If you have some setup / teardown that needs to be done before or after all scenarios, use `BeforeAll` / `AfterAll`. Like hooks and steps, these can be synchronous, accept a callback, or return a promise. 
+If you have some setup / teardown that needs to be done before or after all scenarios, use `BeforeAll` / `AfterAll`. Like hooks and steps, these can be synchronous, accept a callback, or return a promise.
 
 Unlike `Before` / `After` these methods will not have a world instance as `this`. This is becauce each scenario gets its own world instance and these hooks run before / after **all** scenarios.
 

features/attachments.feature

@@ -39,7 +39,7 @@ Feature: Attachments
       import stream from 'stream'
 
       defineSupportCode(({Before}) => {
-        Before(function(scenarioResult, callback) {
+        Before(function(testCase, callback) {
           var passThroughStream = new stream.PassThrough()
           this.attach(passThroughStream, 'image/png', callback)
           passThroughStream.write(new Buffer([137, 80]))

features/hook_parameter.feature

@@ -0,0 +1,80 @@
+Feature: Hook Parameters
+
+  @spawn
+  Scenario: before hook parameter
+    Given a file named "features/my_feature.feature" with:
+      """
+      Feature: a feature
+        Scenario: a scenario
+          Given a step
+      """
+    And a file named "features/step_definitions/my_steps.js" with:
+      """
+      import {defineSupportCode} from 'cucumber'
+
+      defineSupportCode(({When}) => {
+        When(/^a step$/, function() {})
+      })
+      """
+    And a file named "features/support/hooks.js" with:
+      """
+      import {defineSupportCode} from 'cucumber'
+
+      defineSupportCode(({Before}) => {
+        Before(function(testCase) {
+          console.log(testCase.sourceLocation.uri + ":" + testCase.sourceLocation.line)
+        })
+      })
+      """
+    When I run cucumber.js
+    Then the output contains the text:
+      """
+      features/my_feature.feature:2
+      """
+
+  @spawn
+  Scenario: after hook parameter
+    Given a file named "features/my_feature.feature" with:
+      """
+      Feature: a feature
+        Scenario: a scenario
+          Given a passing step
+
+        Scenario: another scenario
+          Given a failing step
+      """
+    And a file named "features/step_definitions/my_steps.js" with:
+      """
+      import {defineSupportCode} from 'cucumber'
+
+      defineSupportCode(({When}) => {
+        When(/^a passing step$/, function() {})
+        When(/^a failing step$/, function() { throw new Error("my error") })
+      })
+      """
+    And a file named "features/support/hooks.js" with:
+      """
+      import {defineSupportCode, Status} from 'cucumber'
+
+      defineSupportCode(({After}) => {
+        After(function(testCase) {
+          let message = testCase.sourceLocation.uri + ":" + testCase.sourceLocation.line + " "
+          if (testCase.result.status === Status.FAILED) {
+            message += "failed"
+          } else {
+            message += "did not fail"
+          }
+          console.log(message)
+        })
+      })
+      """
+    When I run cucumber.js
+    Then it fails
+    And the output contains the text:
+      """
+      features/my_feature.feature:2 did not fail
+      """
+    And the output contains the text:
+      """
+      features/my_feature.feature:5 failed
+      """

package.json

@@ -145,7 +145,6 @@
     "stacktrace-js": "^2.0.0",
     "string-argv": "0.0.2",
     "title-case": "^2.1.1",
-    "upper-case-first": "^1.1.2",
     "util-arity": "^1.0.2",
     "verror": "^1.9.0"
   },

src/models/test_case_hook_definition.js

@@ -17,8 +17,8 @@ export default class TestCaseHookDefinition extends StepDefinition {
     return this.buildInvalidCodeLengthMessage('0 or 1', '2')
   }
 
-  getInvocationParameters({ scenarioResult }) {
-    return [scenarioResult]
+  getInvocationParameters({ hookParameter }) {
+    return [hookParameter]
   }
 
   getValidCodeLengths() {

src/runtime/step_runner.js

@@ -8,10 +8,10 @@ const { beginTiming, endTiming } = Time
 
 async function run({
   defaultTimeout,
-  scenarioResult,
+  hookParameter,
+  parameterTypeRegistry,
   step,
   stepDefinition,
-  parameterTypeRegistry,
   world
 }) {
   beginTiming()
@@ -20,9 +20,9 @@ async function run({
   try {
     parameters = await Promise.all(
       stepDefinition.getInvocationParameters({
-        scenarioResult,
-        step,
-        parameterTypeRegistry
+        hookParameter,
+        parameterTypeRegistry,
+        step
       })
     )
   } catch (err) {

src/runtime/test_case_runner.js

@@ -35,18 +35,18 @@ export default class TestCaseRunner {
       duration: 0,
       status: this.skip ? Status.SKIPPED : Status.PASSED
     }
-  }
-
-  emit(name, data) {
-    let sourceLocation = {
+    this.testCaseSourceLocation = {
       uri: this.testCase.uri,
       line: this.testCase.pickle.locations[0].line
     }
+  }
+
+  emit(name, data) {
     let eventData = { ...data }
     if (_.startsWith(name, 'test-case')) {
-      eventData.sourceLocation = sourceLocation
+      eventData.sourceLocation = this.testCaseSourceLocation
     } else {
-      eventData.testCase = { sourceLocation }
+      eventData.testCase = { sourceLocation: this.testCaseSourceLocation }
     }
     this.eventBroadcaster.emit(name, eventData)
   }
@@ -103,13 +103,13 @@ export default class TestCaseRunner {
     })
   }
 
-  invokeStep(step, stepDefinition) {
+  invokeStep(step, stepDefinition, hookParameter) {
     return StepRunner.run({
       defaultTimeout: this.supportCodeLibrary.defaultTimeout,
-      scenarioResult: this.scenarioResult,
+      hookParameter,
+      parameterTypeRegistry: this.supportCodeLibrary.parameterTypeRegistry,
       step,
       stepDefinition,
-      parameterTypeRegistry: this.supportCodeLibrary.parameterTypeRegistry,
       world: this.world
     })
   }
@@ -153,25 +153,30 @@ export default class TestCaseRunner {
   async run() {
     this.emitPrepared()
     this.emit('test-case-started', {})
-    await this.runHooks(this.beforeHookDefinitions)
+    await this.runHooks(this.beforeHookDefinitions, {
+      sourceLocation: this.testCaseSourceLocation
+    })
     await this.runSteps()
-    await this.runHooks(this.afterHookDefinitions)
+    await this.runHooks(this.afterHookDefinitions, {
+      sourceLocation: this.testCaseSourceLocation,
+      result: this.result
+    })
     this.emit('test-case-finished', { result: this.result })
     return this.result
   }
 
-  async runHook(hookDefinition) {
+  async runHook(hookDefinition, hookParameter) {
     if (this.skip) {
       return { status: Status.SKIPPED }
     } else {
-      return await this.invokeStep(null, hookDefinition)
+      return await this.invokeStep(null, hookDefinition, hookParameter)
     }
   }
 
-  async runHooks(hookDefinitions) {
+  async runHooks(hookDefinitions, hookParameter) {
     await Promise.each(hookDefinitions, async hookDefinition => {
       await this.aroundTestStep(() => {
-        return this.runHook(hookDefinition)
+        return this.runHook(hookDefinition, hookParameter)
       })
     })
   }

src/status.js

@@ -1,5 +1,4 @@
 import _ from 'lodash'
-import upperCaseFirst from 'upper-case-first'
 
 const statuses = {
   AMBIGUOUS: 'ambiguous',
@@ -12,14 +11,6 @@ const statuses = {
 
 export default statuses
 
-export function addStatusPredicates(protoype) {
-  _.each(statuses, status => {
-    protoype['is' + upperCaseFirst(status)] = function() {
-      return this.status === status
-    }
-  })
-}
-
 export function getStatusMapping(initialValue) {
   return _.chain(statuses)
     .map(status => [status, initialValue])