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

Create native feature runner that can explicitly run a scenario from a test #50

Merged
merged 6 commits into from
Mar 24, 2017

Conversation

jacdevos
Copy link
Contributor

@jacdevos jacdevos commented Nov 15, 2016

I created a direct, explicit native test runner for scenarios, based on my observation in Issue
#49

It allows you to create normal XCTests, and run native feature files explicitly, as in:
func testNativeExampleWithScenarioRunner() {
NativeRunner.runScenario(featureFile: "NativeFeatures/native_example.feature", scenario: "This is a basic happy path example", testCase: self)
}

This is just V0.001 so please give suggestions if the idea has wings.

Copy link
Member

@kerrmarin kerrmarin left a comment

Choose a reason for hiding this comment

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

Looks good 👍 I've added a few comments, mostly small things related to style. One thing I would love to see is some tests for the native runner


public class func runScenario(featureFile: String, scenario: String?, testCase: XCTestCase) {
testCase.state.loadAllStepsIfNeeded()
let path = (Bundle(for: type(of: testCase)).resourceURL?.appendingPathComponent(featureFile))!
Copy link
Member

Choose a reason for hiding this comment

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

Hey, we usually discourage the use of force unwrapping. Do you think we could split this out into a couple of guard lets and fail the test here?

let features = loadFeatures(path: path)

for feature in features {
let scenarios = feature.scenarios.filter({ scenario == nil || $0.scenarioDescription.hasPrefix(scenario!)});
Copy link
Member

Choose a reason for hiding this comment

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

What do you think of making this not inline and removing the semicolon?


for feature in features {
let scenarios = feature.scenarios.filter({ scenario == nil || $0.scenarioDescription.hasPrefix(scenario!)});
if scenarios.count < 1{
Copy link
Member

Choose a reason for hiding this comment

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

Nitpicky, but you're missing a space between 1 and {

}


private class func loadFeatures(path : URL) -> [NativeFeature] {
Copy link
Member

Choose a reason for hiding this comment

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

nitpicky, but can we remove the space between path and :

}

for scenario in scenarios {
let allScenarioStepsDefined = scenario.stepDescriptions.map(testCase.state.matchingGherkinStepExpressionFound).reduce(true) { $0 && $1 }
Copy link
Member

Choose a reason for hiding this comment

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

Nice

@smaljaar
Copy link
Contributor

@jacdevos could you plz update the PR so this can be merged? it's been open for a while

@nap-sam-dean
Copy link
Member

@smaljaar If you're interested in this getting into master I don't mind merging it and fixing it myself?

@jacdevos
Copy link
Contributor Author

@smaljaar my bad, I'll make some time for it this week, it has been quite useful on our projects.

@jacdevos
Copy link
Contributor Author

jacdevos commented Mar 21, 2017

@kerrmarin @smaljaar I've pushed the style changes you (rightly) suggested. I also have a few tests in ExampleNativeRunnerTest.

I think we are ready to go. I have a few additions to this that came up in our project, but will submit those as a new PR.

@kerrmarin
Copy link
Member

Thanks @jacdevos I'll investigate the test failures later today

@kerrmarin
Copy link
Member

Looks like it was fixed by running the tests again :)

XCTFail("No scenario found with name: \(scenario)")
}

for scenario in scenarios {
Copy link
Contributor

Choose a reason for hiding this comment

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

This for loop is duplicate code copied from the NativeTestCase method featureScenarioTest(). I extracted the code from featureScenarioTest() into this auxiliary method in NativeTestCase so we avoid having the same code in two places:

  class func perform(scenario: NativeScenario, from feature: NativeFeature, in testCase: XCTestCase) {
    let allScenarioStepsDefined = scenario.stepDescriptions.map(testCase.state.matchingGherkinStepExpressionFound).reduce(true) { $0 && $1 }
    var allFeatureBackgroundStepsDefined = true
    
    if let defined = feature.background?.stepDescriptions.map(testCase.state.matchingGherkinStepExpressionFound).reduce(true, { $0 && $1 }) {
        allFeatureBackgroundStepsDefined = defined
    }
    
    guard allScenarioStepsDefined && allFeatureBackgroundStepsDefined else {
        XCTFail("Some step definitions not found for the scenario: \(scenario.scenarioDescription)")
        return
    }
    
    if let background = feature.background {
        background.stepDescriptions.forEach(testCase.performStep)
    }
    scenario.stepDescriptions.forEach(testCase.performStep)
}

You can now replace the body of this for loop with the line:

    NativeRunner.runScenario(featureFile: featureFile, scenario: nil, testCase: testCase)

In NativeTestCase you can call this method like:

    NativeTestCase.perform(scenario: scenario, from: feature, in: self)

If you like the suggestion, please use it 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion @smaljaar - maybe create another PR for this, I see change has been merged.

@smaljaar
Copy link
Contributor

@jacdevos I studied the NativeRunner class and I really like how you solved the problem of disappearing test cases in the Xcode Test Navigator 👍

@nap-sam-dean nap-sam-dean merged commit 88ad8f1 into net-a-porter-mobile:master Mar 24, 2017
@jacdevos
Copy link
Contributor Author

@smaljaar glad you like it - it's been working quite well for us. There are a few enhancements that would help, which we can work on if we see the value:
• seems like we can remove some boilerplate
• a generator would be really nice: feed it the feature file and spit out a XCTest class with one test per scenario and a steps file with blank steps

@nap-sam-dean
Copy link
Member

nap-sam-dean commented Mar 27, 2017

@jacdevos Nice ideas - I've made the generator one into an issue (#55) - I think I've heard someone else talk about this too, let's see if there's interest. And if you can see ways to reduce boilerplate - that would be fantastic!

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.

None yet

4 participants