Skip to content

Commit

Permalink
Pass step instance to step definition invocation (#54)
Browse files Browse the repository at this point in the history
This simplifies the step definition API; instead of passing it a couple of step attributes (name and attachement), the AST step object is passed.
  • Loading branch information
jbpros committed Apr 12, 2012
1 parent 7b88395 commit 6b58ca2
Show file tree
Hide file tree
Showing 4 changed files with 163 additions and 49 deletions.
15 changes: 13 additions & 2 deletions lib/cucumber/ast/step.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,22 @@ var Step = function(keyword, name, line) {
return attachment;
},

getAttachmentContents: function getAttachmentContents() {
var attachment = self.getAttachment();
var attachmentContents;
if (attachment)
attachmentContents = attachment.getContents();
return attachmentContents;
},

getDocString: function getDocString() { return docString; },

getDataTable: function getDataTable() { return dataTable; },

hasAttachment: function hasAttachment() {
return self.hasDocString() || self.hasDataTable();
},

hasDocString: function hasDocString() {
return !!docString;
},
Expand Down Expand Up @@ -138,8 +150,7 @@ var Step = function(keyword, name, line) {
execute: function execute(visitor, callback) {
var stepDefinition = visitor.lookupStepDefinitionByName(name);
var world = visitor.getWorld();
var attachment = self.getAttachment();
stepDefinition.invoke(name, world, attachment, callback);
stepDefinition.invoke(self, world, callback);
}
};
return self;
Expand Down
13 changes: 7 additions & 6 deletions lib/cucumber/support_code/step_definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ var StepDefinition = function(regexp, code) {
return regexp.test(stepName);
},

invoke: function invoke(stepName, world, stepAttachment, callback) {
invoke: function invoke(step, world, callback) {
var codeCallback = function() {
var successfulStepResult = Cucumber.Runtime.SuccessfulStepResult();
callback(successfulStepResult);
Expand All @@ -24,7 +24,7 @@ var StepDefinition = function(regexp, code) {
callback(failedStepResult);
};

var parameters = self.buildInvocationParameters(stepName, stepAttachment, codeCallback);
var parameters = self.buildInvocationParameters(step, codeCallback);
try {
code.apply(world, parameters);
} catch (exception) {
Expand All @@ -34,12 +34,13 @@ var StepDefinition = function(regexp, code) {
}
},

buildInvocationParameters: function buildInvocationParameters(stepName, stepAttachment, callback) {
buildInvocationParameters: function buildInvocationParameters(step, callback) {
var stepName = step.getName();
var parameters = regexp.exec(stepName);
parameters.shift();
if (stepAttachment) {
var contents = stepAttachment.getContents();
parameters.push(contents);
if (step.hasAttachment()) {
var attachmentContents = step.getAttachmentContents();
parameters.push(attachmentContents);
}
parameters.push(callback);
return parameters;
Expand Down
102 changes: 92 additions & 10 deletions spec/cucumber/ast/step_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,95 @@ describe("Cucumber.Ast.Step", function() {
});
});

describe("getAttachmentContents()", function() {
beforeEach(function() {
spyOn(step, 'getAttachment');
});

it("gets the attachment", function() {
step.getAttachmentContents();
expect(step.getAttachment).toHaveBeenCalled();
});

describe("when there is an attachment", function() {
var attachment, attachmentContents;

beforeEach(function() {
attachmentContents = createSpy("step attachment contents");
attachment = createSpyWithStubs("step attachment", {getContents: attachmentContents});
step.getAttachment.andReturn(attachment);
});

it("gets the attachment contents", function() {
step.getAttachmentContents();
expect(attachment.getContents).toHaveBeenCalled();
});

it("returns the attachment contents", function() {
expect(step.getAttachmentContents()).toBe(attachmentContents);
});
});

describe("when there is no attachement", function() {
it("returns nothing", function() {
expect(step.getAttachmentContents()).toBeUndefined();
});
});
});

describe("hasAttachment()", function() {
beforeEach(function() {
spyOn(step, 'hasDocString');
spyOn(step, 'hasDataTable');
});

it("checks wether the step has a doc string attached or not", function() {
step.hasAttachment();
expect(step.hasDocString).toHaveBeenCalled();
});

describe("when there is a doc string attached", function() {
beforeEach(function() {
step.hasDocString.andReturn(true);
});

it("is truthy", function() {
expect(step.hasAttachment()).toBeTruthy();
});
});

describe("when there is no doc string attached", function() {
beforeEach(function() {
step.hasDocString.andReturn(false);
});

it("checks wether the step has a data table attached or not", function() {
step.hasAttachment();
expect(step.hasDataTable).toHaveBeenCalled();
});

describe("when there is a data table attached", function() {
beforeEach(function() {
step.hasDataTable.andReturn(true);
});

it("is truthy", function() {
expect(step.hasAttachment()).toBeTruthy();
});
});

describe("when there is no data table attached", function() {
beforeEach(function() {
step.hasDataTable.andReturn(false);
});

it("is truthy", function() {
expect(step.hasAttachment()).toBeFalsy();
});
});
});
});

describe("attachDataTableRow()", function() {
var row, dataTable;

Expand Down Expand Up @@ -632,19 +721,17 @@ describe("Cucumber.Ast.Step", function() {
});

describe("execute()", function() {
var stepDefinition, world, attachment;
var stepDefinition, world;
var visitor, callback;

beforeEach(function() {
stepDefinition = createSpy("step definition");
world = createSpy("world");
attachment = createSpy("attachment");
visitor = createSpy("visitor");
callback = createSpy("callback received by execute()");
spyOnStub(stepDefinition, 'invoke');
spyOnStub(visitor, 'lookupStepDefinitionByName').andReturn(stepDefinition);
spyOnStub(visitor, 'getWorld').andReturn(world);
spyOnStub(step, 'getAttachment').andReturn(attachment);
});

it("looks up the step definition based on the step string", function() {
Expand All @@ -657,14 +744,9 @@ describe("Cucumber.Ast.Step", function() {
expect(visitor.getWorld).toHaveBeenCalled();
});

it("gets the step attachement", function() {
step.execute(visitor, callback);
expect(step.getAttachment).toHaveBeenCalled();
});

it("invokes the step definition with the step name, world, attachment and the callback", function() {
it("invokes the step definition", function() {
step.execute(visitor, callback);
expect(stepDefinition.invoke).toHaveBeenCalledWith(name, world, attachment, callback);
expect(stepDefinition.invoke).toHaveBeenCalledWith(step, world, callback);
});
});
});
82 changes: 51 additions & 31 deletions spec/cucumber/support_code/step_definition_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,29 +34,27 @@ describe("Cucumber.SupportCode.StepDefinition", function() {
});

describe("invoke()", function() {
var stepName, world, stepAttachment, callback;
var step, world, callback;
var parameters;

beforeEach(function() {
stepName = createSpy("step name to match");
step = createSpy("step");
world = createSpy("world");
stepAttachment = createSpy("step attachment");
callback = createSpy("callback");
parameters = createSpy("code execution parameters");
spyOn(stepDefinition, 'buildInvocationParameters').andReturn(parameters);
spyOn(stepDefinitionCode, 'apply');
});

it("builds the step invocation parameters", function() {
stepDefinition.invoke(stepName, world, stepAttachment, callback);
stepDefinition.invoke(step, world, callback);
expect(stepDefinition.buildInvocationParameters).toHaveBeenCalled();
expect(stepDefinition.buildInvocationParameters).toHaveBeenCalledWithValueAsNthParameter(stepName, 1);
expect(stepDefinition.buildInvocationParameters).toHaveBeenCalledWithValueAsNthParameter(stepAttachment, 2);
expect(stepDefinition.buildInvocationParameters).toHaveBeenCalledWithAFunctionAsNthParameter(3);
expect(stepDefinition.buildInvocationParameters).toHaveBeenCalledWithValueAsNthParameter(step, 1);
expect(stepDefinition.buildInvocationParameters).toHaveBeenCalledWithAFunctionAsNthParameter(2);
});

it("calls the step definition code with the parameters and World as 'this'", function() {
stepDefinition.invoke(stepName, world, stepAttachment, callback);
stepDefinition.invoke(step, world, callback);
expect(stepDefinitionCode.apply).toHaveBeenCalledWith(world, parameters);
});

Expand All @@ -65,8 +63,8 @@ describe("Cucumber.SupportCode.StepDefinition", function() {
var successfulStepResult;

beforeEach(function() {
stepDefinition.invoke(stepName, world, stepAttachment, callback);
codeExecutionCallback = stepDefinition.buildInvocationParameters.mostRecentCall.args[2];
stepDefinition.invoke(step, world, callback);
codeExecutionCallback = stepDefinition.buildInvocationParameters.mostRecentCall.args[1];
successfulStepResult = createSpy("successful step result");
spyOn(Cucumber.Runtime, 'SuccessfulStepResult').andReturn(successfulStepResult);
});
Expand Down Expand Up @@ -148,67 +146,89 @@ describe("Cucumber.SupportCode.StepDefinition", function() {
});

it("creates a new failed step result", function() {
stepDefinition.invoke(stepName, world, stepAttachment, callback);
stepDefinition.invoke(step, world, callback);
expect(Cucumber.Runtime.FailedStepResult).toHaveBeenCalledWith(failureException);
});

it("calls back with the step result", function() {
stepDefinition.invoke(stepName, world, stepAttachment, callback);
stepDefinition.invoke(step, world, callback);
expect(callback).toHaveBeenCalledWith(failedStepResult);
});
});
});

describe("buildInvocationParameters()", function() {
var stepName, stepAttachment, stepAttachmentContents;
var step, stepName, stepAttachment, stepAttachmentContents;
var matches, callback;

beforeEach(function() {
stepName = createSpy("step name to match");
stepName = createSpy("step name to match");
stepAttachmentContents = createSpy("step attachment contents");
stepAttachment = createSpyWithStubs("step attachment", {getContents: stepAttachmentContents});
matches = createSpyWithStubs("matches", {shift: null, push: null});
callback = createSpy("callback");
step = createSpyWithStubs("step", {hasAttachment: null, getName: stepName, getAttachmentContents: stepAttachmentContents});
matches = createSpyWithStubs("matches", {shift: null, push: null});
callback = createSpy("callback");
spyOnStub(stepRegexp, 'exec').andReturn(matches);
});

it("gets the step name", function() {
stepDefinition.buildInvocationParameters(step, callback);
expect(step.getName).toHaveBeenCalled();
});

it("executes the step regexp against the step name", function() {
stepDefinition.buildInvocationParameters(stepName, stepAttachment, callback);
stepDefinition.buildInvocationParameters(step, callback);
expect(stepRegexp.exec).toHaveBeenCalledWith(stepName);
});

it("removes the whole matched string of the regexp result array (to only keep matching groups)", function() {
stepDefinition.buildInvocationParameters(stepName, stepAttachment, callback);
stepDefinition.buildInvocationParameters(step, callback);
expect(matches.shift).toHaveBeenCalled();
});

describe("when a step attachment is present", function() {
it("gets the attachment's value", function() {
stepDefinition.buildInvocationParameters(stepName, stepAttachment, callback);
expect(stepAttachment.getContents).toHaveBeenCalled();
it("checks wether the step has an attachment or not", function() {
stepDefinition.buildInvocationParameters(step, callback);
expect(step.hasAttachment).toHaveBeenCalled();
});

describe("when the step has an attachment", function() {
beforeEach(function() {
step.hasAttachment.andReturn(true);
});

it("gets the attachment contents", function() {
stepDefinition.buildInvocationParameters(step, callback);
expect(step.getAttachmentContents).toHaveBeenCalled();
});

it("adds the attachment contents to the parameter array", function() {
stepDefinition.buildInvocationParameters(stepName, stepAttachment, callback);
stepDefinition.buildInvocationParameters(step, callback);
expect(matches.push).toHaveBeenCalledWith(stepAttachmentContents);
});
});

describe("when no step attachment is present", function() {
it("does not add the attachment contents to the parameter array", function() {
stepAttachment = undefined;
stepDefinition.buildInvocationParameters(stepName, stepAttachment, callback);
expect(matches.push).not.toHaveBeenCalledWith(undefined);
describe("when the step has no attachment", function() {
beforeEach(function() {
step.hasAttachment.andReturn(false);
});

it("does not get the attachment contents", function() {
stepDefinition.buildInvocationParameters(step, callback);
expect(step.getAttachmentContents).not.toHaveBeenCalled();
});

it("does not add the attachement contents to the parameter array", function() {
stepDefinition.buildInvocationParameters(step, callback);
expect(matches.push.callCount).toBe(1);
});
});

it("adds the callback to the parameter array", function() {
stepDefinition.buildInvocationParameters(stepName, stepAttachment, callback);
stepDefinition.buildInvocationParameters(step, callback);
expect(matches.push).toHaveBeenCalledWith(callback);
});

it("returns the parameters", function() {
expect(stepDefinition.buildInvocationParameters(stepName, stepAttachment, callback)).toBe(matches);
expect(stepDefinition.buildInvocationParameters(step, callback)).toBe(matches);
});
});
});

1 comment on commit 6b58ca2

@jbpros
Copy link
Member Author

@jbpros jbpros commented on 6b58ca2 Apr 12, 2012

Choose a reason for hiding this comment

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

This relates to #57, not 54.

Please sign in to comment.