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

Issue: 997 Start of BeforeStep functionality #1058

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
37 changes: 37 additions & 0 deletions features/before_after_step_hooks.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
Feature: Before and After Step Hooks

Background:
Given a file named "features/a.feature" with:
"""
Feature: some feature
Scenario: some scenario
Given a step
"""
And a file named "features/step_definitions/cucumber_steps.js" with:
"""
import {Given} from 'cucumber'

Given(/^a step$/, function() {})
"""

Scenario: Failing before step fails the scenario
Given a file named "features/support/hooks.js" with:
"""
import {BeforeStep} from 'cucumber'

BeforeStep(function() { throw 'Fail' })
"""
When I run cucumber-js
Then it fails

Scenario: Only run BeforeStep hooks with appropriate tags
Given a file named "features/support/hooks.js" with:
"""
import {After, Before, BeforeStep } from 'cucumber'

BeforeStep('@any-tag', function() {
throw Error("Would fail if ran")
})
"""
When I run cucumber-js
Then it passes
32 changes: 0 additions & 32 deletions features/hooks.feature
Original file line number Diff line number Diff line change
Expand Up @@ -62,35 +62,3 @@ Feature: Environment Hooks
When I run cucumber-js
Then it fails
And the "After" hook has status "passed"

Scenario: World is this in hooks
Given a file named "features/support/world.js" with:
"""
import {setWorldConstructor} from 'cucumber'

function WorldConstructor() {
return {
isWorld: function() { return true }
}
}

setWorldConstructor(WorldConstructor)
"""
Given a file named "features/support/hooks.js" with:
"""
import {After, Before} from 'cucumber'

Before(function() {
if (!this.isWorld()) {
throw Error("Expected this to be world")
}
})

After(function() {
if (!this.isWorld()) {
throw Error("Expected this to be world")
}
})
"""
When I run cucumber-js
Then it passes
73 changes: 73 additions & 0 deletions features/world_in_hooks.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
Feature: World in Hooks

Background:
Given a file named "features/a.feature" with:
"""
Feature: some feature
Scenario: some scenario
Given a step
"""
And a file named "features/step_definitions/cucumber_steps.js" with:
"""
import {Given} from 'cucumber'

Given(/^a step$/, function() {})
"""

Scenario: World is this in hooks
Given a file named "features/support/world.js" with:
"""
import {setWorldConstructor} from 'cucumber'

function WorldConstructor() {
return {
isWorld: function() { return true }
}
}

setWorldConstructor(WorldConstructor)
"""
Given a file named "features/support/hooks.js" with:
"""
import {After, Before, BeforeStep } from 'cucumber'

Before(function() {
if (!this.isWorld()) {
throw Error("Expected this to be world")
}
})

After(function() {
if (!this.isWorld()) {
throw Error("Expected this to be world")
}
})
"""
When I run cucumber-js
Then it passes

Scenario: World is this in BeforeStep hooks
Given a file named "features/support/world.js" with:
"""
import {setWorldConstructor} from 'cucumber'

function WorldConstructor() {
return {
isWorld: function() { return true }
}
}

setWorldConstructor(WorldConstructor)
"""
Copy link
Author

Choose a reason for hiding this comment

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

can move this to background

Given a file named "features/support/hooks.js" with:
"""
import {After, Before, BeforeStep } from 'cucumber'

BeforeStep(function() {
if (!this.isWorld()) {
throw Error("Expected this to be world")
}
})
"""
When I run cucumber-js
Then it passes
Copy link
Author

Choose a reason for hiding this comment

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

I can add a new line

5 changes: 3 additions & 2 deletions src/formatter/rerun_formatter_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,9 @@ describe('RerunFormatter', () => {

it('outputs the references needed to run the scenarios again', function() {
expect(this.output).to.eql(
`${this.feature1Path}:1${separator.expected}${this
.feature2Path}:2`
`${this.feature1Path}:1${separator.expected}${
this.feature2Path
}:2`
Copy link
Author

Choose a reason for hiding this comment

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

I was getting a linting error here.

)
})
})
Expand Down
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const { methods } = supportCodeLibraryBuilder
export const After = methods.After
export const AfterAll = methods.AfterAll
export const Before = methods.Before
export const BeforeStep = methods.BeforeStep
export const BeforeAll = methods.BeforeAll
export const defineParameterType = methods.defineParameterType
export const defineStep = methods.defineStep
Expand Down
27 changes: 27 additions & 0 deletions src/models/test_step_hook_definition.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import PickleFilter from '../pickle_filter'
import StepDefinition from './step_definition'

export default class TestStepHookDefinition extends StepDefinition {
constructor(data) {
super(data)
this.pickleFilter = new PickleFilter({
tagExpression: this.options.tags,
})
}

appliesToTestCase({ pickle, uri }) {
return this.pickleFilter.matches({ pickle, uri })
}

getInvalidCodeLengthMessage() {
return this.buildInvalidCodeLengthMessage('0 or 1', '2')
}

getInvocationParameters({ hookParameter }) {
return [hookParameter]
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think these hooks need invocation parameters (like what is passed to before/after hooks)


getValidCodeLengths() {
return [0, 1, 2]
}
}
36 changes: 35 additions & 1 deletion src/runtime/test_case_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export default class TestCaseRunner {
parameters: worldParameters,
})
this.beforeHookDefinitions = this.getBeforeHookDefinitions()
this.beforeStepHookDefinitions = this.getBeforeStepHookDefinitions()
this.afterHookDefinitions = this.getAfterHookDefinitions()
this.testStepIndex = 0
this.result = {
Expand Down Expand Up @@ -91,6 +92,12 @@ export default class TestCaseRunner {
)
}

getBeforeStepHookDefinitions() {
return this.supportCodeLibrary.beforeTestStepHookDefinitions.filter(
stepHookDefinition => stepHookDefinition.appliesToTestCase(this.testCase)
)
}

getStepDefinitions(step) {
return this.supportCodeLibrary.stepDefinitions.filter(stepDefinition =>
stepDefinition.matchesStepName({
Expand Down Expand Up @@ -174,6 +181,13 @@ export default class TestCaseRunner {
return this.invokeStep(null, hookDefinition, hookParameter)
}

async runStepHook(step, hookDefinition, hookParameter) {
if (this.skip) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we already skip the steps inside runStep with isSkippingSteps check

Copy link
Author

Choose a reason for hiding this comment

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

removed.

return { status: Status.SKIPPED }
}
return this.invokeStep(step, hookDefinition, hookParameter)
}

async runHooks(hookDefinitions, hookParameter) {
await Promise.each(hookDefinitions, async hookDefinition => {
await this.aroundTestStep(() =>
Expand All @@ -182,6 +196,14 @@ export default class TestCaseRunner {
})
}

async runStepHooks(step, hookDefinitions) {
return Promise.all(
hookDefinitions.map(async hookDefinition =>
this.runStepHook(step, hookDefinition)
)
)
}
Copy link
Member

Choose a reason for hiding this comment

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

Thinking on this function, we are currently not adding the duration of the step hooks to the overall duration of the step. That should be testable with unit tests


async runStep(step) {
Copy link
Author

Choose a reason for hiding this comment

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

@charlierudolph BeforeStep/AfterStep are working. I do have questions about stacktrace reporting and duration calculation.
For duration my current thinking is to calculate the duration for the beforeSteps, the step, and the afterSteps and update that value on the stepResult prior to returning.
I'm interested in hearing what you feel would be the best way to handle multiple errors in the hooks .

const stepDefinitions = this.getStepDefinitions(step)
if (stepDefinitions.length === 0) {
Expand All @@ -194,7 +216,19 @@ export default class TestCaseRunner {
} else if (this.isSkippingSteps()) {
return { status: Status.SKIPPED }
}
return this.invokeStep(step, stepDefinitions[0])
return this.runStepHooks(step, this.beforeStepHookDefinitions).then(
responses => {
const errors = responses.filter(response => response.exception)
if (errors && errors.length) {
return {
exception: 'BeforeStep hook failed', // TODO return stacktrace(s)?
Copy link
Author

Choose a reason for hiding this comment

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

I'm curious how we should handle multiple exceptions? And should I instead check if status is failed here instead of checking for exception in any of the responses? Or both?

status: Status.FAILED,
}
} else {
return this.invokeStep(step, stepDefinitions[0])
}
}
)
}

async runSteps() {
Expand Down
83 changes: 83 additions & 0 deletions src/runtime/test_case_runner_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, it } from 'mocha'
import { expect } from 'chai'
import sinon from 'sinon'
import TestCaseHookDefinition from '../models/test_case_hook_definition'
import TestStepHookDefinition from '../models/test_step_hook_definition'
import TestCaseRunner from './test_case_runner'
import Status from '../status'
import StepRunner from './step_runner'
Expand Down Expand Up @@ -29,6 +30,7 @@ describe('TestCaseRunner', () => {
}
this.supportCodeLibrary = {
afterTestCaseHookDefinitions: [],
beforeTestStepHookDefinitions: [],
beforeTestCaseHookDefinitions: [],
defaultTimeout: 5000,
stepDefinitions: [],
Expand Down Expand Up @@ -148,6 +150,87 @@ describe('TestCaseRunner', () => {
})
})

describe('with a passing step and a before step hook', () => {
beforeEach(async function() {
const testStepHookDefinition = new TestStepHookDefinition({
code() {
throw new Error('error')
},
line: 4,
options: {},
uri: 'path/to/hooks',
})
this.supportCodeLibrary.beforeTestStepHookDefinitions = [
testStepHookDefinition,
]
this.step = { uri: 'path/to/feature', locations: [{ line: 2 }] }
this.stepResult = {
duration: 1,
status: Status.PASSED,
}
const stepDefinition = {
uri: 'path/to/steps',
line: 3,
matchesStepName: sinon.stub().returns(true),
}
StepRunner.run.resolves(this.stepResult)
this.supportCodeLibrary.stepDefinitions = [stepDefinition]
this.testCase.pickle.steps = [this.step]
const scenarioRunner = new TestCaseRunner({
eventBroadcaster: this.eventBroadcaster,
skip: false,
testCase: this.testCase,
supportCodeLibrary: this.supportCodeLibrary,
})
await scenarioRunner.run()
})

it('emits test-case-prepared', function() {
expect(this.onTestCasePrepared).to.have.callCount(1)
expect(this.onTestCasePrepared).to.have.been.calledWith({
steps: [
{
actionLocation: { line: 3, uri: 'path/to/steps' },
sourceLocation: { line: 2, uri: 'path/to/feature' },
},
],
sourceLocation: { line: 1, uri: 'path/to/feature' },
})
})

it('emits test-case-started', function() {
expect(this.onTestCaseStarted).to.have.callCount(1)
expect(this.onTestCaseStarted).to.have.been.calledWith({
sourceLocation: { line: 1, uri: 'path/to/feature' },
})
})

it('emits test-step-started', function() {
expect(this.onTestStepStarted).to.have.callCount(1)
expect(this.onTestStepStarted).to.have.been.calledWith({
index: 0,
testCase: { sourceLocation: { line: 1, uri: 'path/to/feature' } },
})
})

it('emits test-step-finished', function() {
expect(this.onTestStepFinished).to.have.callCount(1)
expect(this.onTestStepFinished).to.have.been.calledWith({
index: 0,
testCase: { sourceLocation: { line: 1, uri: 'path/to/feature' } },
result: { duration: 1, status: Status.PASSED },
})
})

it('emits test-case-finished', function() {
expect(this.onTestCaseFinished).to.have.callCount(1)
expect(this.onTestCaseFinished).to.have.been.calledWith({
result: { duration: 1, status: Status.PASSED },
sourceLocation: { line: 1, uri: 'path/to/feature' },
})
})
})

describe('with a failing step', () => {
beforeEach(async function() {
this.step = { uri: 'path/to/feature', locations: [{ line: 2 }] }
Expand Down
Loading