search

pytest-dev / pytest-bdd

BDD library for the pytest runner
https://pytest-bdd.readthedocs.io/en/latest/
MIT License
1.32k stars 221 forks source link

Pytest-bdd multiline steps are too dangerous #474

Closed theObserver1 closed 2 months ago

theObserver1 commented 2 years ago

From the documentation:

As Gherkin, pytest-bdd supports multiline steps (aka PyStrings). But in much cleaner and powerful way: Step is considered as multiline one, if the next line(s) after it’s first line, is indented relatively to the first line. The step name is then >simply extended by adding further lines with newlines.`

While i agree this implementation is much cleaner than Gherkin, it is also far too dangerous. i have personally seen test cases where a six step BDD test case was treated as a single step by pytest-bdd because a tester mixed tabs and spaces before a step. Even worse, the test case was then passed because the first line was successfully parsed, executed and passed. The remaining lines were treated as a continuation of the first and ignored. We've come close to missing some major issues due to this implementation of multiline support.

As a workaround we now remove all whitepace and tabs before each GIVEN/WHEN/THEN/AND line before passing the feature file to pytest-bdd. It's just too dangerous otherwise.

Is it possible to throw an exception if a line starting with GIVEN/WHEN/THEN/AND is treated as a multiple line continuation of the previous step due the indentation? Or at least throw a warning? A warning message would also be welcome if indentations consists of spaces and tabs as this will likely cause issues with current multiline implementation ie the line may appear correctly aligned in JIRA etc but Python can treat the tabs differently than spaces and, at least in our case, steps are going to be ignored.

youtux commented 2 years ago

Good point, maybe we should allow only multiline steps according to the Gherkin spec, and it could go in the next major release, since we are making backwards incompatible changes anyway. cc @olegpidsadnyi .

jsa34 commented 2 months ago

This is resolved by being fully Gherkin compliant in https://github.com/pytest-dev/pytest-bdd/pull/698

Shall we close this issue @youtux ?

youtux commented 2 months ago

Yes, but would you mind checking the Readme to make sure we document this correctly?

jsa34 commented 2 months ago

Absolutely!