gorghoa / ScenarioStateBehatExtension

Provide a way to share scenario state through steps
MIT License
33 stars 7 forks source link

[Discussion] Using a more explicit syntax #36

Open Sydney-o9 opened 6 years ago

Sydney-o9 commented 6 years ago

I really like the idea behind this extension.

But, when you are testing a stateless system, chiefly an API, then the resulting state of our steps is handled by no one. This is the case for this extension.

(Even though most of the time, if you are at Level 3 of the Richardson Maturity Model, you can use HATEOS & Hypermedia to follow _links but this is not always possible - hence the necessity of a solution which your extension solves really well.)


This discussion was very interesting to me and I think you guys are solving something important.

@jakzal said I shouldn't debug to see where these things come from. This should be self-explanatory imo. Explicit.

In regards to readability, I wonder if one might consider the following as on top of what was discussed, I think:

Behat scenarios are all about state. First you put the system under test to a special state through the Given steps. Then you continue to manipulate your system through When steps and finally testing the resulting state via the Then steps.

It should be the same for the store.

How about using the gherkin language instead?

    Feature: Gorilla Lunch Basket
        In order for a gorilla to eat at lunch time
        He needs to be able to gather bananas from his fellow bonobos
        And keep them in his basket

        Scenario: A bonobo can take a banana and give it to a gorilla for him to eat
            Given there is a bonobo "@bonobo"
            And there is a tree with 1 banana "@banana"
            And there is a gorilla "@gorilla"

            When bonobo "@bonobo" takes banana "@banana"
            And bonobo "@bonobo" gives banana "@banana" to gorilla "@gorilla"
            Then gorilla "@gorilla" should have 1 banana left in his basket

            When I send an api request to get gorilla "@gorilla" basket
            Then the response status code should be 200
            And the "count" property should equal 1

             When gorilla "@gorilla" eats 1 banana
             Then gorilla "@gorilla" should have 0 banana in his basket

            When I send an api request to get gorilla "@gorilla" basket
            Then the response status code should be 200
            And the "count" property should equal 0

This would be more explicit, and would allow to identify the store keys easily. Now, we know all objects being stored start with @ (it could be any notation) but when reading the test, it is more readable and really takes on the power of your extension which is the ability to store stuff between contexts easily.

You can then imagine the following when things get tricky:

        Scenario: Many bonobos can take many bananas and give them to a gorilla for him to eat
            Given there are the following 2 bonobos
                | storeKey         | name                     |
                | @bonobo1      | Monkey Chan        |
                | @bonobo2     | Bonono Chan         |
            And there is a tree "@tree" with the following 3 bananas 
                | storeKey        | name                      |
                | @banana1      | Yellow Banana       |
                | @banana2     | Yellowish Banana   |
                | @banana3     | Greenish  Banana   |

            When I send an api request to get tree "@tree" details
            Then the response status code should be 200
            And the "count" property should equal 3

            When I send an api request to get gorilla "@gorilla" basket
            Then the response status code should be 200
            And the "count" property should equal 0

            When bonobo "@bonobo1" takes bananas "@banana1, @banana2" from the tree "@tree"
            And bonobo "@bonobo1" gives banana "@banana1" to gorilla "@gorilla"
            And I ask how many bananas are left in "@gorilla" basket
            Then gorilla "@gorilla" should have 1 banana in his basket

            When I send an api request to get gorilla "@gorilla" basket
            Then the response status code should be 200
            And the "count" property should equal 1

            When I send an api request to get tree "@tree" details
            Then the response status code should be 200
            And the "count" property should equal 1

            When bonobo "@bonobo2" takes banana "@banana3" from the tree "@tree"
            Then the tree "@tree" should have 0 banana left

            When I send an api request to get tree "@tree" details
            Then the response status code should be 200
            And the "count" property should equal 0

            When bonobo "@bonobo2" gives bananas "@banana3" to gorilla "@gorilla"
            Then gorilla "@gorilla" should have 2 bananas in his basket

            When I send an api request to get gorilla "@gorilla" basket
            Then the response status code should be 200
            And the "count" property should equal 2

Basically I think being explicit is key and this should never happen:

When I add token "123" to this bucket

It should be

Given there is a token "@token" which has for value "123" And there is a bucket "@bucket" When I add token "@token" to bucket "@bucket"


Also, in regards to this:

/**
 * @When bonobo gives this banana to :monkey
 *
 * @ScenarioStateArgument("scenarioBanana")
 * @ScenarioStateArgument(name="scenarioBonobo", argument="bonobo")
 *
 * @param string $monkey
 * @param string $scenarioBanana
 * @param Bonobo $bonobo
 */
public function giveBananaToGorilla($monkey, $scenarioBanana, Bonobo $bonobo)
{
}

It could be:

/**
 * @When a there is a bonobo :bonoboStoreKey named :name
 *
 * @ScenarioStateAutosave()
 *
 * @param string $bonobo
 */
public function thereIsABonoboNamed($bonoboStoreKey, $name)
{
    $bonobo = new Bonobo($name);

    return $bonobo;
}

/**
 * @When bonobo :bonoboStoreKey gives banana :bananaStoreKey to gorilla :gorillaStoreKey
 *
 * @ScenarioStateAutoload()
 *
 * @param Banana $banana
 * @param Bonobo $bonobo
 * @param Gorilla $gorilla
 */
public function bonoboGivesBananaToGorilla(Bonobo $bonobo, Banana $banana, Gorilla $gorilla)
{
}
Sydney-o9 commented 6 years ago

I think this is worth a shot - I've finished to work on a Proof Of Concept here. Feel free to let me know what you think.


Future Enhancements

Ultimately, I think to be able to improve further on all the points @jakzal, @stof & others made, it would be great to apply similar concepts from redux:

Dispatching actions via $this->store->dispatch() for example, using reducers, etc and NOT updating the store directly as currently set-up.

This would avoid this:

@jakzal I don't see how it's simpler. What I see is lots of WTF moments trying to trace which step this variable came from.

This is correct.

There is an existing php-redux library so it would be possible to build on top of it easily - I haven't tried it.

These are simple concepts yet very powerful. This could actually leverage Behat test suite, not necessarily limited to Scenarios.

As mentioned, I've done a Proof of Concept for what was mentioned in my previous comment for you guys to have a think about.

jakzal commented 6 years ago

I was summoned here by your comments referencing my username ;)

I didn't have time to look through all your work unfortunately. Let me share with you my recent thoughts on this topic. Perhaps it'll give you some ideas.

The approach I recently used involved a builder. Builder is stateful and gathers all the needed information as steps are invoked (mostly givens). Then, at some point one of the steps will use the information gathered by the builder to construct the object(s) we need to perform an action (probably a "when"). The builder is stateful, if it ever returns an object it's always a new instance (or an immutable object). This makes it easier to avoid the WTF moments, when object references are passed around and modified.

This worked well for my use case. It might not work for everyone though.

gorghoa commented 6 years ago

Hi @Sydney-o9,

(sorry didn’t saw your comment earlier…)

First of all, thanks for your well written well thought issue <3.

I’m a little too brainfucked right now to have a real well thought feedback, so this is my hot reaction to your issue ;)

Regarding your feature file proposal:

I see your point, and I feel it’s definitely more explicit from a developer point of view.

But, I am a bit more concerned from a non technical .feature reader/writer point of view though. I have the feeling that your proposal might encourage to sneakily add some technicalities in the feature description (which I am strongly against).

The more near natural language the feature is, the better.

From this perspectives, I think I am more inclined to let the gherkin untouched for now.

That being said, the explicitness of what this extension brings seems to trigger lot of reactions and it’s definitely something we should improve, in this regard your proposition and @jakzal advices are very valuable :)

@vincentchalamon, any thoughts on this?


Regarding more flexibility when storing data to the store is definitely interesting (I’ll open another issue to discuss it).

vincentchalamon commented 6 years ago

Hi all,

I tried to understand all your points, so I hope I won't be out ot scope in my answer :-)

  • Currently, the scenario is being shifted to in the Context classes when it should stay in the .feature files
  • storeKeys are only made available in the context

The approach we wanted to share through this extension is to keep scenarios (.feature files) non-technical, like @gorghoa said in the last comment. Introducing technical steps in the scenario will break this rule.

I invite you to have a look at Samuel Roze's conference, which describes this point: Behat is more than that.

It is not possible to store more than 1 instance in the store of the same type without having to keep track of the store keys used in the annotation provided

If I understand this point, you want to store several values on the same key in the store? Do you have any example for this case please? If so, this could be a great enhancement imo, if it doesn't break BC and doesn't make annotation usage too complex. Feel free to open a PR to propose it @Sydney-o9. What do you think of it @gorghoa?

reading the feature is not enough to know the state of the store (we should now the state of the store by reading the feature)

Same as first point: scenarios should be non-technical. If you want to know the context for your scenario, just explain it literally. For instance:

Scenario: A gorilla gives a banana to another gorilla
    Given there are 2 gorillas
    And there is a banana
    When the gorilla gives the banana to another gorilla
    Then the last gorilla has the banana

In this example, it's very easy to undertand the context, whatever is saved in the store. Technical points (like the state of the store) are out of scenarios and should be kept in the context.

Dispatching actions via $this->store->dispatch() for example, using reducers, etc and NOT updating the store directly as currently set-up.

This is, imo, a really good idea & improvement! I'm just wondering if it wouldn't be overkill, as the main goal of this extension is to store data between steps. I can't see any case where dispatch the store would be useful, could you print an example @Sydney-o9 please? What do you think of that feature @gorghoa?

gorghoa commented 6 years ago

@Sydney-o9 :

Dispatching actions via $this->store->dispatch() for example, using reducers, etc and NOT updating the store directly as currently set-up.

@vincentchalamon :

I'm just wondering if it wouldn't be overkill

My exact thoughts @vincentchalamon ;)

I fear the complexity this would add to the extension, maybe even counterproductive with the explicitness we aim for.

My first thoughts is maybe this should live userland, in your own Contexts. I guess it wouldn’t be that hard to abstract the $this->scenarioState->provideStateFragment('scenarioBanana', $banana); within some custom logic (for instance with some kind of php-redux, I guess you could call provideStateFragment in an action).

@Sydney-o9

It is not possible to store more than 1 instance in the store of the same type without having to keep track of the store keys used in the annotation provided

Could you elaborate this point? I’m not sure to understand exactly what you mean