Metadrop / behat-contexts

Behat contexts with additional steps
GNU General Public License v2.0
9 stars 19 forks source link

Remove last entity created support (or at least discourage it) #108

Open jorgetutor opened 3 years ago

jorgetutor commented 3 years ago

There are several steps with "last entity created", I am afraid we are overusing it when it should be only used in one scenario: when we don't know the name of the entity.

When we know the name, we should always use the steps "entity with the label" as it is more BDD-oriented and clearer to understand what are you testing.

Last entity created

src/Behat/Context/ParagraphsContext.php:   * Create a paragraph and reference it in the given field of the last node created.
src/Behat/Context/NodeAccessContext.php:   * Refresh node_access for the last node created.
src/Behat/Context/NodeAccessContext.php:   * @Given the access of last node created is refreshed
src/Behat/Context/NodeAccessContext.php:   * @Given the access of last node created with :bundle bundle is refreshed
src/Behat/Context/EntityContext.php:   * Go to last entity created.
src/Behat/Context/EntityContext.php:   * @Given I go to the last entity :entity created
src/Behat/Context/EntityContext.php:   * @Given I go to the last entity :entity with :bundle bundle created
src/Behat/Context/EntityContext.php:   * @Given I go to :subpath of the last entity :entity created
src/Behat/Context/EntityContext.php:   * @Given I go to :subpath of the last entity :entity with :bundle bundle created
src/Behat/Context/EntityContext.php:   * Delete the last entity created.
src/Behat/Context/EntityContext.php:   * @Given last entity :entity created is deleted
src/Behat/Context/EntityContext.php:   * @Given last entity :entity with :bundle bundle created is deleted

Entity with label

src/Behat/Context/EntityContext.php:   * @Given I go to the :entity_type entity with label :label
src/Behat/Context/EntityContext.php:   * @Given I go to :subpath of the :entity_type entity with label :label
omarlopesino commented 2 years ago

I was thinking that there are entities without an identifiable label like the paragraph entity which uses the entity ID as the label. But it should not be important as those entities won't be accessed by URL. Indeed. I think all the entities that are accessed by URL have a label. So I think now we could deprecate the 'Last entity created' steps, and remove those steps in a 2.x version.

I have just one question (maybe I am overthinking): are these steps helping to make proper BDD? I mean, BDD tests try to imitate the behaviour of a real user. But all these steps are shortcuts to go to pages. What it does is go to a page/subpage by writing the URL. But user do not usually write URLs in the browser. For example, a user does not go to the article 'Multiple databases on a single instance on Drupal 9' of metadrop.net, the user goes first to the front page, then click 'Articles', and then click on that article.

In that case, maybe we could remove all these steps, in favour of doing steps that involves doing the real user actions on the site. What do you think?

rsanzante commented 2 years ago

That's indeed a legitimate concern.

I don't like this kind of steps either because of what you explain. But I accepted them because they were needed in some cases, if I'm not wrong. However, I don't recall those cases and anyway I think we should revisit them and try to accomplish those cases without this kind of steps.

I would review our projects to see in which cases we use those steps and see if you can get rid of them. If yes, I would mark them as deprecated.

omarlopesino commented 1 year ago

For the moment I will focus on the main topic of the issue. After removing "last entity created" steps, we can think later if we can remove all the steps, or maintain them as utilities in case we want shortcut steps when we don't want to execute each task manually, which is tested otherwhere.

I've checked our projects and the usage I see we use for "last entity created" is:

Projects using this will need a short time to adapt, and I think that now with the new steps we have, having a step to go to the last entity created becomes unnecessary.

We can do a release that throws the warning, and another release that removes the step.