Skip to content
This repository has been archived by the owner on Jun 12, 2021. It is now read-only.

Analyzer warning for code outside step delegates #406

Closed
adamralph opened this issue Nov 8, 2017 · 5 comments
Closed

Analyzer warning for code outside step delegates #406

adamralph opened this issue Nov 8, 2017 · 5 comments
Labels
wontfix This will not be worked on

Comments

@adamralph
Copy link
Owner

The basic premise of xbehave is that everything should be inside step delegates so that success or failure of a given scenario always related to a specific logical step, identified by a natural language sentence. If code is outside a step delegate, then this benefit is lost.

Slightly contrived example:

[Scenario]
public void Addition(int x, int y, Calculator calculator, int answer)
{
    "Given the number 1"
        .x(() => x = 1);

    "And the number 2"
        .x(() => y = 2);

    calculator = new Calculator());       // this could potentially throw

    "When I add the numbers together"
        .x(() => calculator.Add(x, y));

    answer = calculator.GetLastAnswer();  // this could potentially throw

    "Then the answer is 3"
        .x(() => Xunit.Assert.Equal(3, answer));
}

If one of the two commented operations throw, it's not possible to tell any more from the natural language output which one of them threw. It is necessary to resort to inspecting the call stack, which is the case when not using xbehave. Thus, the main benefit of xbehave is lost.

@ursenzler
Copy link
Contributor

We typically initialize some values at the beginning of a scenario (outside of a step). And I think that is okay because this code really should not fail. If it does, the whole scenario fails, which is okay with my understanding.
So the analyzer should probably only check for code after the first step.

Btw answer = calculator.GetLastAnswer(); is something that we would encapsulate with a dummy step - as I mentioned in #362: "_".x(() => answer = calculator.GetLastAnswer());

@adamralph
Copy link
Owner Author

adamralph commented Nov 8, 2017

We typically initialize some values at the beginning of a scenario (outside of a step).

Why not put that in a step? It can even go into a background step if you want to reuse across scenarios.

Btw answer = calculator.GetLastAnswer(); is something that we would encapsulate with a dummy step - as I mentioned in #362: "_".x(() => answer = calculator.GetLastAnswer());

Why does it have to be a "dummy" step? If you are writing a step anyway, I'd give it a proper sentence:

"And I get the last answer"
    .x(() => answer = calculator.GetLastAnswer());

@ma499
Copy link

ma499 commented Apr 20, 2018

Why not put that in a step?

I agree with initiatlisation in a step - that's how I interpret 'Given' steps.

However, I find it a bit ugly for test variables to be declared as method parameters - particularly when the test also has parameters that I'm driving using [Example(..)]. I would prefer to declare test variables inside the method. The documentation seems to suggest that this is acceptable.

@adamralph
Copy link
Owner Author

@ma499 that's a fair point. I guess it's a matter of taste. I guess such an analyzer could attempt ignore variable declarations, but it's always the edge cases that trip up those kind of things.

@adamralph adamralph changed the title Analyzer warning for code oustide step delegates Analyzer warning for code outside step delegates Jun 1, 2018
@adamralph adamralph removed the ready label Jun 1, 2018
@adamralph
Copy link
Owner Author

adamralph commented Sep 22, 2018

Will be made redundant by #362.

@adamralph adamralph added wontfix This will not be worked on and removed P2 enhancement New feature or request labels Sep 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants