Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix undefined hook parameter #919

Merged
merged 7 commits into from
Aug 29, 2017
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/support_files/api_reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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.

Expand Down
20 changes: 10 additions & 10 deletions docs/support_files/attachments.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand All @@ -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');
}
Expand All @@ -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');
}
Expand All @@ -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');
Expand Down
6 changes: 3 additions & 3 deletions docs/support_files/hooks.md
Original file line number Diff line number Diff line change
@@ -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');
Expand All @@ -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) {
Expand Down Expand Up @@ -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.

Expand Down
2 changes: 1 addition & 1 deletion features/attachments.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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]))
Expand Down
102 changes: 102 additions & 0 deletions features/hook_parameter.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
Feature: Hook Parameters

Background:
Given a file named "features/my_feature.feature" with:
"""
Feature: a feature
Scenario: a scenario
Given a step
"""

@spawn
Scenario: before hook parameter
Given a file named "features/step_definitions/my_steps.js" with:
"""
import {defineSupportCode} from 'cucumber'

defineSupportCode(({When}) => {
When(/^a step$/, function() {})
})
"""
Given 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 (failing test case)
Given a file named "features/step_definitions/my_steps.js" with:
"""
import {defineSupportCode} from 'cucumber'

defineSupportCode(({When}) => {
When(/^a step$/, function() {})
})
"""
Given 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 the output contains the text:
"""
features/my_feature.feature:2 did not fail
"""

@spawn
Scenario: after hook parameter (failing test case)
Given a file named "features/step_definitions/my_steps.js" with:
"""
import {defineSupportCode} from 'cucumber'

defineSupportCode(({When}) => {
When(/^a step$/, function() {
throw new Error("my error")
})
})
"""
Given 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 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you need an else? seems like the test case only fails.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the same After hook as the previous scenario. I'll join the two scenarios into one

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 failed
"""
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
4 changes: 2 additions & 2 deletions src/models/test_case_hook_definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
10 changes: 5 additions & 5 deletions src/runtime/step_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ const { beginTiming, endTiming } = Time

async function run({
defaultTimeout,
scenarioResult,
hookParameter,
parameterTypeRegistry,
step,
stepDefinition,
parameterTypeRegistry,
world
}) {
beginTiming()
Expand All @@ -20,9 +20,9 @@ async function run({
try {
parameters = await Promise.all(
stepDefinition.getInvocationParameters({
scenarioResult,
step,
parameterTypeRegistry
hookParameter,
parameterTypeRegistry,
step
})
)
} catch (err) {
Expand Down
35 changes: 20 additions & 15 deletions src/runtime/test_case_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
})
}
Expand Down Expand Up @@ -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)
})
})
}
Expand Down
9 changes: 0 additions & 9 deletions src/status.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import _ from 'lodash'
import upperCaseFirst from 'upper-case-first'

const statuses = {
AMBIGUOUS: 'ambiguous',
Expand All @@ -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])
Expand Down
Loading