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

Added SpecFlow support #727

Merged
merged 1 commit into from
Mar 8, 2016
Merged

Added SpecFlow support #727

merged 1 commit into from
Mar 8, 2016

Conversation

bjorkstromm
Copy link
Member

Added support for SpecFlow. Solves issue #695

At first I thought it would be nice if SpecFlow tool could intercept the Unit Test runner (NUnit/MSTest) just like OpenCover/DotCover, but SpecFlow sets some requirements on the arguments for the Unit Test runner. So it might be simpler to leave it as is.

Unit Tests still to be written.

@patriksvensson
Copy link
Member

@mholo65 I see.

Couldn't you use an interceptor but have different settings classes? You could then let the settings classes like NUnitSpecFlowSettings/XUnitSpecFlowSettings (which all derive from SpecFlowSettings) validate and render the arguments for the SpecFlow tool runner.

@bjorkstromm
Copy link
Member Author

@patriksvensson That could actually work.

By intercepting the tool, there shouldn't be any need for separate MSBuild/NUnit/XUnit aliases nor separate Settings classes, one alias for Test Execution reports should be enough.

Example:

public static void SpecFlowExecutionReport(
       Action<ICakeContext> action,
       FilePath projectFile,
       SpecFlowSettings settings
)

The SpecFlow runner should intercept the action (accepting only NUnit/MSTest/XUnit), examine the arguments (and throw exception/try to fix if invalid) and set SpecFlow parameters accordingly.

We'll still need an separate alias for the Step Definition Report though.

@patriksvensson
Copy link
Member

@mholo65 Sounds good to me. Implementing the step definition report should probably be done in a separate PR though?

@bjorkstromm
Copy link
Member Author

@patriksvensson could be a separate PR. altough I've implemented it already in this PR :)

@patriksvensson
Copy link
Member

@mholo65 Ah, but then proceed as normal 😄

@bjorkstromm
Copy link
Member Author

@patriksvensson that should be it! Could You have a look? When ok, I'll rebase.

@bjorkstromm bjorkstromm changed the title [WIP] Added SpecFlow support. (Tests still missing) [WIP] Added SpecFlow support. Mar 4, 2016
@patriksvensson
Copy link
Member

@mholo65 Sure, I'll take a look later tonight.

}
else if (executable.IndexOf("xunit", StringComparison.OrdinalIgnoreCase) >= 0)
{
builder.Append("nunitexecutionreport");
Copy link
Member

Choose a reason for hiding this comment

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

Is this correct?

@patriksvensson
Copy link
Member

@mholo65 Left some minor comments, but other than that it looks good to me.

@devlead @gep13 Any other comments about this one?

@bjorkstromm
Copy link
Member Author

@patriksvensson SpecFlow is able to parse test results from either MSTest or NUnit (v2), thus making it compatible with xUnit if the runner is configured to write output in NUnit (v2) format. See #729

The three if-statements could be moved to a separate internal class to enhance readability/maintainability. I will do this + add code examples to documentation for aliases.

Any other comments from @devlead or @gep13?

@bjorkstromm
Copy link
Member Author

@patriksvensson Rebased and moved the argument generation based on Unit Test provider to a separate class.

@bjorkstromm bjorkstromm changed the title [WIP] Added SpecFlow support. Added SpecFlow support Mar 8, 2016
@devlead
Copy link
Member

devlead commented Mar 8, 2016

LGTM 👍 I'll do some local testing and then merge if those go ok.

@devlead devlead merged commit d0d84fa into cake-build:develop Mar 8, 2016
@devlead devlead mentioned this pull request Mar 8, 2016
@bjorkstromm bjorkstromm deleted the specflow branch March 10, 2016 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants