bddkickstarter / Gherkin.TypeProvider

A Gherkin TypeProvider
The Unlicense
17 stars 2 forks source link

Issues with Validation #1

Closed deyanp closed 4 years ago

deyanp commented 4 years ago

I probably did not get this correctly, but it seems that I need to "touch" all steps with no arguments (no arguments to extract from the step text, no multiline string or data table), so that I can validate properly the scenario, right? This requires a lot of scenarioX.stepY |> ignore statements, which is not nice ..

Additionally, where should I call FeatureValidator.Validate in case I use Xunit and just have a bunch of [<Fact>]-decorated functions, 1 per scenario?

deyanp commented 4 years ago

Pls ignore the 2nd question - I can just create another [<Fact>]-decorated function per feature file, which calls FeatureValidator.Validate ...

bddkickstarter commented 4 years ago

Hi Deyan,

Thanks again for the excellent feedback!

You have found a fundamental difference between the 2 approaches (sign of a good early adopter!!). Because Specflow can be so tricky to maintain (what with all those step definitions) people tend to automate the feature with the minimum specflow code (Share steps using regex etc etc). This is great for keeping the feature file in line with your code, but horrible for maintaining a test suite. In specflow when an example fails its not immediately apparent exactly where that failure is (possibly in a helper function shared by multiple steps) - not great for maintenance.

My goal with this is to create a test suite that looks similar to the feature file - if something fails then I should have a single function to look at - that is why when were talking about individual scenarios I would rather have a test per scenario (even if that means a bit of cut n pasting

So with a given from which we take no data.....just because we don't extract something from it, doesn't mean it shouldn't be in our test....it literally describes what we're doing to setup our behaviour. Say for example I had 2 given steps:

Given user Paul was eligible for the discount And had previously registered

The first step obviously I need "Paul" and maybe "eligible" so will have to use the text from the step, will include it in my test But the second doesn't have any data we need...except it does....the whole given statement is the data. I don't extract any data from the text but I definitely want that given in my tests as its a free "comment". I have to add some ignores for sure but now I have a property in my code that says 1 Given had previously regsitered....and now I know the code that follows this is the code used to register my user.....amazing!!

IMHO tests are all about maintainability.....I hate comments in the code as they rarely get updated and often say one thing when your code does something else. The comment here is active....its guaranteed to be in sync with the feature. If your code doesn't match the step comment anymore....then your code is wrong and you have to change it in the one place it is wrong (i.e. the single test)

What if someone changes the second given to "had not previously registered" - if I don't have that given in my code, or I have ignored it my test suite is definitely not a true representation of the feature anymore and should fail.

It is different from Specflow but as Im using it day to day, Im finding the steps I don't extract data from just make the test suite much much easier to read.

Sorry for the long response but is a fundamental feature of the provider that it works like this - I definitely wouldn't ignore scenarios just because Im not visiting the steps

Does that make sense?....what do you think?

I haven't used xUnit for a while but you're going to want to use the feature validator once the test suite has been completed.. In Expecto that's just adding a line in the console app after the test run. Is there an after test suite run hook in xunit you could use?

I'll start playing with xUnit and seeing if I can get a solution for you

thanks again

Chris

On Wed, Dec 25, 2019 at 10:03 PM deyanp notifications@github.com wrote:

I probably did not get this correctly, but it seems that I need to "touch" all steps with no arguments (no arguments to extract from the step text, no multiline string or data table), so that I can validate properly the scenario, right? This requires a lot of scenarioX.stepY |> ignore statements, which is not nice ..

Additionally, where should I call FeatureValidator.Validate in case I use Xunit and just have a bunch of []-decorated functions, 1 per scenario?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bddkickstarter/Gherkin.TypeProvider/issues/1?email_source=notifications&email_token=AKTYB4EAGNESWREUYVXJFJ3Q2PKCRA5CNFSM4J7GU47KYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4ICUGMNQ, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKTYB4FUWBKCJWWDF7PJ2LTQ2PKCRANCNFSM4J7GU47A .

deyanp commented 4 years ago

Hi Chris,

Thank you for the detailed answer! The ignored "active comments" are not a deal-breaker for me, and I can follow your argumentation.

I am using XUnit with TickSpec currently, in particular with the TickSpec approach of having only functions without any type/class with mutable fields (https://github.com/fsprojects/TickSpec#step-definitions-f-without-mutable-field). I prefer to not use any classes with state in F#, that is why I do not want to use the XUnit's IClassFixture or similar (see https://xunit.net/docs/shared-context).

For the time being I am adding one more XUnit test as last one to validate the feature ... seems to work for the time being, and the parallel execution of different features seems not to interfere with the sequencial execution of tests within a feature ...

I need to take a 2nd look at your CE for scenario outlines, frankly said at a first glance I did not understand it ... and then compare if the implementation of the 3 features I selected out of our existing TickSpec-based test suite is easier with this approach. TickSpec has the advantage that steps can be automated individually and reused across scenarios (glueing takes place in the feature file!), however your amazing Gherkin Type Provider has the benefit of local context inside the function glueing together the steps ... this will be a tough comparison ;)

Br, Deyan

deyanp commented 4 years ago

To clarify what I do not understand about your ScenarioOutline builder: I do not see how the different examples/scenarios in a scenario outline can be displayed in the GUI (e.g. Rider->Unit Tests, VS or VS Code with Test Explorer) as individual tests ... For that in XUnit MemberData must be used I guess ..

bddkickstarter commented 4 years ago

I totally get it - its been a while since I used xunit and I've been spoilt with Expecto ;) Expecto allows me to create a either a single test or a list and will be run either if returned from a function attributed with Tests attribute (thats why its plural!). The scenario builder would return a list of Expecto tests which would be then be run when the console app is run (working on a blog post now about it no)

Unfortunately xunit doesnt work that way so it would be impossible to return a list of functions attributed with Fact from the builder. I'm looking into solutions now but they tend to involve Reflection.Emit which can be.......interesting ;)

xUnit was my test framework of choice when I used C# so I'm definitely going to try and get a solution for the builder in F# (again...suggestions always welcome!)

deyanp commented 4 years ago

Trying currently , I guess it should work, what I am not happy with is the type of the exampleData (too long) ...

let examples =
    accountCreationFeature
        .``Creating an account with missing data results in validation errors``
        .Examples
    |> Array.map Array.singleton
[<Theory;MemberData("examples")>]
let test3 (exampleData:AccountCreationFeature.AccountCreationFeature_Feature.Creating_an_account_with_missing_data_results_in_validation_errorsClass.DataClass) =
    let scenario = accountCreationFeature.``Creating an account with missing data results in validation errors``
    scenario.``0 Given that an account does not exist in the system`` |> ignore
    // TODO: arrange ..

    scenario.``1 When an account creation request, without specifying <field>, is triggered`` |> ignore
    let x = exampleData.field.Value
    // TODO: act ..

    scenario.``2 Then a response with http status code 400 is returned`` |> ignore
    // TODO: assert ..

    let argThenAnd =
        scenario
            .``3 And the account has the following properties``
            .Argument
    // TODO: assert ..

    ()

For scenario outline:

  Scenario Outline: Creating an account with missing data results in validation errors
    Given that an account does not exist in the system
    When an account creation request, without specifying <field>, is triggered
    Then a response with http status code 400 is returned
    And the account has the following properties
      | Property | Value      |
      | code     | Validation |

    Examples:
      | field    |
      | type     |
      | balance  |
      | currency |
deyanp commented 4 years ago

Ok, maybe this is good enough for me:

let examples =
    accountCreationFeature
        .``Creating an account with missing data results in validation errors``
        .Examples
    |> Array.map (fun example -> [| example.field.Value |])
[<Theory;MemberData("examples")>]
let ``Creating an account with missing data results in validation errors`` (field:string) =
    let scenario = accountCreationFeature.``Creating an account with missing data results in validation errors``
...
bddkickstarter commented 4 years ago

Oh fantastic, that looks very promising! I think I've seen a way to extend the Theory and Fact attributes to take additional data. I'd love to take the name of the test from the scenario outline name, rather than the variable name like I can with Expecto

deyanp commented 4 years ago

Do you mean there is a solution to the problem I have currently, that the test function name is a copy-paste of the scenario name, but there is no automatic sync between them, so they might deviate from each other? I was thinking of tunneling everything through a single function using MemberData like this:

type AccountUpdateFeature = GherkinProvider<const(__SOURCE_DIRECTORY__ + "/AccountUpdate.feature")>
let accountUpdateFeature = AccountUpdateFeature.CreateFeature()
let scenarios =
    accountUpdateFeature.Scenarios
    |> Array.map Array.singleton

[<Theory; MemberData("scenarios")>]
let AccountUpdate (scenario:AccountUpdateFeature.AccountUpdateFeature_ScenarioBase) =
    printfn "-----%A-----" scenario
    Debug.WriteLine(scenario.ToString())

but then I started writing separate Fact-decorated functions for the individual scenarios, as I saw no point in tunneling ... Of course now I have to maintain the Fact-decorated function names by hand ...

bddkickstarter commented 4 years ago

Closing this for now as its not really a problem with the Gherkin.TypeProvider (that's working as expected). If you find a nice way of using xUnit or I find a way of making testing in xUnit easier I'll reopen. Thanks for all the help