From d37913f5ec25b7a433ec317ab02880e119843654 Mon Sep 17 00:00:00 2001 From: Daniel Beland Date: Thu, 6 Jul 2023 13:34:35 -0400 Subject: [PATCH 1/3] improve performance --- src/pytest_bdd/parser.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/pytest_bdd/parser.py b/src/pytest_bdd/parser.py index aa7c4ec..6bbf8b1 100644 --- a/src/pytest_bdd/parser.py +++ b/src/pytest_bdd/parser.py @@ -292,6 +292,7 @@ class Scenario: class Step: type: str _name: str + _full_name: str | None line_number: int indent: int keyword: str @@ -311,6 +312,7 @@ class Step: self.scenario = None self.background = None self.lines = [] + self._full_name = None def add_line(self, line: str) -> None: """Add line to the multiple step. @@ -318,25 +320,29 @@ class Step: :param str line: Line of text - the continuation of the step name. """ self.lines.append(line) + self._full_name = None @property def name(self) -> str: - multilines_content = textwrap.dedent("\n".join(self.lines)) if self.lines else "" + if self._full_name is None: + multilines_content = textwrap.dedent("\n".join(self.lines)) if self.lines else "" - # Remove the multiline quotes, if present. - multilines_content = re.sub( - pattern=r'^"""\n(?P.*)\n"""$', - repl=r"\g", - string=multilines_content, - flags=re.DOTALL, # Needed to make the "." match also new lines - ) + # Remove the multiline quotes, if present. + multilines_content = re.sub( + pattern=r'^"""\n(?P.*)\n"""$', + repl=r"\g", + string=multilines_content, + flags=re.DOTALL, # Needed to make the "." match also new lines + ) - lines = [self._name] + [multilines_content] - return "\n".join(lines).strip() + lines = [self._name] + [multilines_content] + self._full_name = "\n".join(lines).strip() + return self._full_name @name.setter def name(self, value: str) -> None: self._name = value + self._full_name = None def __str__(self) -> str: """Full step name including the type.""" From c4a041d4baf4e2e450fb726d5f7d6bbd3982bae9 Mon Sep 17 00:00:00 2001 From: Daniel Beland Date: Thu, 13 Jul 2023 11:26:38 -0400 Subject: [PATCH 2/3] use cached_property --- src/pytest_bdd/parser.py | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/src/pytest_bdd/parser.py b/src/pytest_bdd/parser.py index 6bbf8b1..699cb8a 100644 --- a/src/pytest_bdd/parser.py +++ b/src/pytest_bdd/parser.py @@ -6,6 +6,7 @@ import textwrap import typing from collections import OrderedDict from dataclasses import dataclass, field +from functools import cached_property from typing import cast from . import exceptions, types @@ -292,7 +293,6 @@ class Scenario: class Step: type: str _name: str - _full_name: str | None line_number: int indent: int keyword: str @@ -312,7 +312,6 @@ class Step: self.scenario = None self.background = None self.lines = [] - self._full_name = None def add_line(self, line: str) -> None: """Add line to the multiple step. @@ -320,29 +319,33 @@ class Step: :param str line: Line of text - the continuation of the step name. """ self.lines.append(line) - self._full_name = None + if "full_name" in self.__dict__: + del self.__dict__["full_name"] + + @cached_property + def full_name(self) -> str: + multilines_content = textwrap.dedent("\n".join(self.lines)) if self.lines else "" + + # Remove the multiline quotes, if present. + multilines_content = re.sub( + pattern=r'^"""\n(?P.*)\n"""$', + repl=r"\g", + string=multilines_content, + flags=re.DOTALL, # Needed to make the "." match also new lines + ) + + lines = [self._name] + [multilines_content] + return "\n".join(lines).strip() @property def name(self) -> str: - if self._full_name is None: - multilines_content = textwrap.dedent("\n".join(self.lines)) if self.lines else "" - - # Remove the multiline quotes, if present. - multilines_content = re.sub( - pattern=r'^"""\n(?P.*)\n"""$', - repl=r"\g", - string=multilines_content, - flags=re.DOTALL, # Needed to make the "." match also new lines - ) - - lines = [self._name] + [multilines_content] - self._full_name = "\n".join(lines).strip() - return self._full_name + return self.full_name @name.setter def name(self, value: str) -> None: self._name = value - self._full_name = None + if "full_name" in self.__dict__: + del self.__dict__["full_name"] def __str__(self) -> str: """Full step name including the type.""" From 86b6c942d9baced0a7fc36cab8b0a3c3ee544d7d Mon Sep 17 00:00:00 2001 From: Daniel Beland Date: Fri, 14 Jul 2023 04:53:38 -0400 Subject: [PATCH 3/3] test step name cache behavior --- src/pytest_bdd/parser.py | 11 +++++++---- tests/steps/test_common.py | 24 +++++++++++++++++++++++- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/pytest_bdd/parser.py b/src/pytest_bdd/parser.py index 699cb8a..e49f847 100644 --- a/src/pytest_bdd/parser.py +++ b/src/pytest_bdd/parser.py @@ -319,8 +319,7 @@ class Step: :param str line: Line of text - the continuation of the step name. """ self.lines.append(line) - if "full_name" in self.__dict__: - del self.__dict__["full_name"] + self._invalidate_full_name_cache() @cached_property def full_name(self) -> str: @@ -337,6 +336,11 @@ class Step: lines = [self._name] + [multilines_content] return "\n".join(lines).strip() + def _invalidate_full_name_cache(self) -> None: + """Invalidate the full_name cache.""" + if "full_name" in self.__dict__: + del self.full_name + @property def name(self) -> str: return self.full_name @@ -344,8 +348,7 @@ class Step: @name.setter def name(self, value: str) -> None: self._name = value - if "full_name" in self.__dict__: - del self.__dict__["full_name"] + self._invalidate_full_name_cache() def __str__(self) -> str: """Full step name including the type.""" diff --git a/tests/steps/test_common.py b/tests/steps/test_common.py index 535f785..7108aaa 100644 --- a/tests/steps/test_common.py +++ b/tests/steps/test_common.py @@ -4,7 +4,7 @@ from unittest import mock import pytest -from pytest_bdd import given, parsers, then, when +from pytest_bdd import given, parser, parsers, then, when from pytest_bdd.utils import collect_dumped_objects @@ -316,3 +316,25 @@ def test_step_catches_all(pytester): objects = collect_dumped_objects(result) assert objects == ["foo", ("foo parametrized", 1), "foo", ("foo parametrized", 2), "foo", ("foo parametrized", 3)] + + +def test_step_name_is_cached(): + """Test that the step name is cached and not re-computed eache time.""" + step = parser.Step(name="step name", type="given", indent=8, line_number=3, keyword="Given") + assert step.name == "step name" + + # manipulate the step name directly and validate the cache value is still returned + step._name = "incorrect step name" + assert step.name == "step name" + + # change the step name using the property and validate the cache has been invalidated + step.name = "new step name" + assert step.name == "new step name" + + # manipulate the step lines and validate the cache value is still returned + step.lines.append("step line 1") + assert step.name == "new step name" + + # add a step line and validate the cache has been invalidated + step.add_line("step line 2") + assert step.name == "new step name\nstep line 1\nstep line 2"