Skip to content

Fix empty docstring delivery to step functions (#809)#810

Merged
Pierre-Sassoulas merged 4 commits into
pytest-dev:masterfrom
golikovichev:fix/809-empty-docstring
Jul 3, 2026
Merged

Fix empty docstring delivery to step functions (#809)#810
Pierre-Sassoulas merged 4 commits into
pytest-dev:masterfrom
golikovichev:fix/809-empty-docstring

Conversation

@golikovichev

@golikovichev golikovichev commented May 9, 2026

Copy link
Copy Markdown
Contributor

Closes #809.

Problem

When a step has an empty docstring, pytest-bdd raises fixture 'docstring' not found:

When extra labels are injected
  """
  """

The reproducer from the issue triggers this on pytest-bdd 8.1.0.

Cause

In ScenarioTemplate.steps_from_template_steps (src/pytest_bdd/parser.py), the docstring of each rendered step was assigned with a truthy check:

docstring=render_string(step.docstring, context) if step.docstring else None,

step.docstring here is a str (set earlier in parse_steps from step.docstring.content). For an empty docstring, that string is "", which is falsy, so the whole expression collapses to None. The downstream check in scenario.py only forwards the docstring to the step function when step.docstring is not None, so the argument is never injected and pytest treats docstring as a missing fixture.

The earlier line in parse_steps is fine because it tests the DocString object itself (always truthy when present) before reading .content.

Fix

Switch the truthy check to is not None so empty docstrings are forwarded as "":

docstring=render_string(step.docstring, context) if step.docstring is not None else None,

Tests

  • New regression test test_steps_with_empty_docstring in tests/steps/test_docstring.py, following the style of the existing tests in that file.
  • Verified the new test fails on master and passes with the fix.
  • Existing docstring tests still pass (pytest tests/steps/test_docstring.py: 5 passed).
  • Targeted run of tests/steps, tests/parser/test_parser.py, tests/feature/test_scenario.py, tests/feature/test_outline.py: 42 passed.

CHANGES.rst entry added under the Unreleased Fixed section.

ScenarioTemplate.steps_from_template_steps used a truthy check
(`if step.docstring`) when building the step list. An empty docstring
is a valid value, but `""` is falsy, so the docstring was replaced
with None. The step then looked like it had no docstring at all,
and pytest fell back to fixture lookup, raising
"fixture 'docstring' not found".

Switched the check to `is not None` so empty docstrings are passed
through as `""`. Added a regression test in tests/steps/test_docstring.py.
@golikovichev golikovichev force-pushed the fix/809-empty-docstring branch from 9084928 to 28ac968 Compare May 9, 2026 14:19
@golikovichev

Copy link
Copy Markdown
Contributor Author

Hi, just checking if there's anything I can clarify on this.

@golikovichev

Copy link
Copy Markdown
Contributor Author

Friendly nudge - this is still ready whenever someone has a moment. It fixes #809 (empty docstrings were not delivered to step functions), the change is small with a regression test, and CI is green. Happy to rebase onto main if that helps.

@youtux youtux enabled auto-merge (squash) June 15, 2026 07:04
@golikovichev

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Quick note on the red CI: those 4 failing tests already fail on master, so they are not from this change. The same set is red on the latest master run (the 2026-06-09 push, run 27186482739):

  • tests/feature/test_outline.py::test_outline_with_escaped_pipes
  • tests/feature/test_scenario.py::test_scenario_comments
  • tests/parser/test_errors.py::test_step_outside_scenario_or_background_error
  • tests/parser/test_parser.py::test_parser

This branch does not add any new failures. I can open a separate PR to fix the master breakage if that would help.

@golikovichev

Copy link
Copy Markdown
Contributor Author

I dug into the one that reproduces everywhere, test_step_outside_scenario_or_background_error. It is a behavior change in gherkin-official, not a regression in pytest-bdd.

On gherkin-official 39.1.0, parsing a feature with a step outside a scenario no longer raises CompositeParserException - the orphan step is silently dropped from the AST. I confirmed it directly:

Parser().parse(feature_with_orphan_step)  # no exception, feature has only the scenario child

Because no exception is raised, the ERROR_PATTERNS match in gherkin_parser.py never fires, so the expected FeatureError is never raised and the test fails.

Two ways to fix, depending on what you prefer:

  1. Keep the strict behavior: detect the orphan step in pytest-bdd and raise FeatureError ourselves (more work, since gherkin discards it before we see it).
  2. Accept the upstream leniency: update the test (and possibly the now-unused ERROR_PATTERNS entry).

Happy to send a PR for whichever direction you want. The other CI failures look pin/env-specific - they pass locally on gherkin 39, so they are likely a separate locked-deps issue rather than the same cause.

@golikovichev

Copy link
Copy Markdown
Contributor Author

Thanks again for the approval. The master CI failures I flagged in June are resolved upstream now: they came from a gherkin-official behaviour change, and gherkin-official 41 fixed it, so current master is green. The checks here are just waiting for a maintainer to approve the workflow run. Whenever you get a moment to approve and run them, they should come back green, and this is ready to merge. No separate fix PR is needed.

@Pierre-Sassoulas Pierre-Sassoulas disabled auto-merge July 3, 2026 05:02
@Pierre-Sassoulas Pierre-Sassoulas enabled auto-merge (squash) July 3, 2026 05:02

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving because it was first approved by youtux and the change all come from main;

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.12%. Comparing base (9454b38) to head (41ee446).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #810      +/-   ##
==========================================
+ Coverage   96.10%   96.12%   +0.01%     
==========================================
  Files          55       55              
  Lines        2390     2398       +8     
  Branches      136      136              
==========================================
+ Hits         2297     2305       +8     
  Misses         56       56              
  Partials       37       37              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Pierre-Sassoulas Pierre-Sassoulas merged commit 4835112 into pytest-dev:master Jul 3, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Empty docstring results in "fixture not found"

3 participants