Fix historyId calculation by allure-pytest (fix #743, #744) (#745)

Author: delatrie

allure-pytest/src/listener.py

@@ -22,6 +22,7 @@
 from allure_pytest.utils import get_outcome_status, get_outcome_status_details
 from allure_pytest.utils import get_pytest_report_status
 from allure_pytest.utils import format_allure_link
+from allure_pytest.utils import get_history_id
 from allure_commons.utils import md5
 
 
@@ -101,18 +102,19 @@ def pytest_runtest_setup(self, item):
         self._update_fixtures_children(item)
         uuid = self._cache.get(item.nodeid)
         test_result = self.allure_logger.get_test(uuid)
-        params = item.callspec.params if hasattr(item, 'callspec') else {}
+        params = self.__get_pytest_params(item)
         test_result.name = allure_name(item, params)
         full_name = allure_full_name(item)
         test_result.fullName = full_name
-        test_result.historyId = md5(item.nodeid)
         test_result.testCaseId = md5(full_name)
         test_result.description = allure_description(item)
         test_result.descriptionHtml = allure_description_html(item)
         current_param_names = [param.name for param in test_result.parameters]
-        test_result.parameters.extend(
-            [Parameter(name=name, value=represent(value)) for name, value in params.items()
-             if name not in current_param_names])
+        test_result.parameters.extend([
+            Parameter(name=name, value=represent(value))
+            for name, value in params.items()
+            if name not in current_param_names
+        ])
 
     @pytest.hookimpl(hookwrapper=True)
     def pytest_runtest_call(self, item):
@@ -132,6 +134,11 @@ def pytest_runtest_teardown(self, item):
         yield
         uuid = self._cache.get(item.nodeid)
         test_result = self.allure_logger.get_test(uuid)
+        test_result.historyId = get_history_id(
+            test_result.fullName,
+            test_result.parameters,
+            original_values=self.__get_pytest_params(item)
+        )
         test_result.labels.extend([Label(name=name, value=value) for name, value in allure_labels(item)])
         test_result.labels.extend([Label(name=LabelType.TAG, value=value) for value in pytest_markers(item)])
         self.__apply_default_suites(item, test_result)
@@ -287,8 +294,18 @@ def add_parameter(self, name, value, excluded, mode: ParameterMode):
         if existing_param:
             existing_param.value = represent(value)
         else:
-            test_result.parameters.append(Parameter(name=name, value=represent(value),
-                                                    excluded=excluded or None, mode=mode.value if mode else None))
+            test_result.parameters.append(
+                Parameter(
+                    name=name,
+                    value=represent(value),
+                    excluded=excluded or None,
+                    mode=mode.value if mode else None
+                )
+            )
+
+    @staticmethod
+    def __get_pytest_params(item):
+        return item.callspec.params if hasattr(item, 'callspec') else {}
 
     def __apply_default_suites(self, item, test_result):
         default_suites = allure_suite_labels(item)

allure-pytest/src/utils.py

@@ -1,6 +1,6 @@
 import pytest
 from itertools import chain, islice
-from allure_commons.utils import represent, SafeFormatter
+from allure_commons.utils import represent, SafeFormatter, md5
 from allure_commons.utils import format_exception, format_traceback
 from allure_commons.model2 import Status
 from allure_commons.model2 import StatusDetails
@@ -176,3 +176,16 @@ def get_pytest_report_status(pytest_report):
     for pytest_status, status in zip(pytest_statuses, statuses):
         if getattr(pytest_report, pytest_status):
             return status
+
+
+def get_history_id(full_name, parameters, original_values):
+    return md5(
+        full_name,
+        *(original_values.get(p.name, p.value) for p in sorted(
+            filter(
+                lambda p: not p.excluded,
+                parameters
+            ),
+            key=lambda p: p.name
+        ))
+    )

allure-python-commons/src/utils.py

@@ -18,8 +18,11 @@
 def md5(*args):
     m = hashlib.md5()
     for arg in args:
-        part = arg.encode('utf-8')
-        m.update(part)
+        if not isinstance(arg, bytes):
+            if not isinstance(arg, str):
+                arg = repr(arg)
+            arg = arg.encode('utf-8')
+        m.update(arg)
     return m.hexdigest()
 
 

tests/allure_pytest/acceptance/history_id/history_id_test.py

@@ -1,6 +1,7 @@
 from hamcrest import assert_that
 from tests.allure_pytest.pytest_runner import AllurePytestRunner
 
+import allure
 from allure_commons_test.report import has_test_case
 from allure_commons_test.result import has_history_id
 
@@ -40,3 +41,92 @@ def test_history_id_for_skipped(allure_pytest_runner: AllurePytestRunner):
             has_history_id()
         )
     )
+
+
+@allure.issue("743", name="Issue 743")
+def test_history_id_affected_by_allure_parameter(
+    allure_pytest_runner: AllurePytestRunner
+):
+    """
+    >>> import allure
+    >>> from time import perf_counter
+
+    >>> def test_allure_parameter_with_changing_value():
+    ...     allure.dynamic.parameter("time", perf_counter())
+    """
+
+    first_run = allure_pytest_runner.run_docstring()
+    second_run = allure_pytest_runner.run_docstring()
+
+    assert __get_history_id(first_run) != __get_history_id(second_run)
+
+
+@allure.issue("743", name="Issue 743")
+def test_history_id_not_affected_by_excluded_parameter(
+    allure_pytest_runner: AllurePytestRunner
+):
+    """
+    >>> import allure
+    >>> from time import perf_counter
+
+    >>> def test_excluded_allure_parameter():
+    ...     allure.dynamic.parameter("time", perf_counter(), excluded=True)
+    """
+
+    first_run = allure_pytest_runner.run_docstring()
+    second_run = allure_pytest_runner.run_docstring()
+
+    assert __get_history_id(first_run) == __get_history_id(second_run)
+
+
+@allure.issue("744", name="Issue 744")
+def test_history_id_not_affected_by_pytest_ids(
+    allure_pytest_runner: AllurePytestRunner
+):
+    # We're using the trick with the same parameter here because it's not easy
+    # to run pytester multiple times with different code due to caching of
+    # python modules. In reality this change happens between runs
+    run_result = allure_pytest_runner.run_pytest(
+        """
+        import pytest
+
+        @pytest.mark.parametrize("v", [
+            pytest.param(1),
+            pytest.param(1, id="a")
+        ])
+        def test_two_allure_parameters(v):
+            pass
+        """
+    )
+
+    assert __get_history_id(run_result, 0) == __get_history_id(run_result, 1)
+
+
+@allure.issue("743", name="Issue 743")
+def test_different_byte_arrays_are_distinguishable(
+    allure_pytest_runner: AllurePytestRunner
+):
+    """
+    The 'allure_commons.utils.represent' function used to convert allure
+    parameter values to strings makes all byte arrays indistinguishable.
+    Some extra effort is required to properly calculate 'historyId' on tests
+    that are parametrized with byte arrays.
+    """
+    run_result = allure_pytest_runner.run_pytest(
+        """
+        import pytest
+
+        @pytest.mark.parametrize("v", [
+            pytest.param(b'a'),
+            pytest.param(b'b')
+        ])
+        def test_two_allure_parameters(v):
+            pass
+        """
+    )
+
+    assert __get_history_id(run_result, 0) != __get_history_id(run_result, 1)
+
+
+def __get_history_id(run, index=0):
+    return run.test_cases[index]["historyId"]

tests/allure_pytest/defects/issue733_test.py

@@ -1,6 +1,8 @@
+import allure
 from allure_pytest.utils import allure_title
 
 
+@allure.issue("733", name="Issue 733")
 def test_no_allure_title_error_if_item_obj_missing():
     item_with_no_obj_attr_stub = object()
 

tests/allure_pytest/unit/__init__.py

@@ -0,0 +1,29 @@
+from allure_pytest.utils import get_history_id
+from allure_commons.model2 import Parameter
+
+
+def test_no_dependency_on_parameters_order():
+    assert get_history_id("my-full-name", [
+        Parameter(name="a", value="1"),
+        Parameter(name="b", value="2")
+    ], {}) == get_history_id("my-full-name", [
+        Parameter(name="b", value="2"),
+        Parameter(name="a", value="1")
+    ], {})
+
+
+def test_original_values_are_used():
+    assert get_history_id("my-full-name", [
+        Parameter(name="a", value="1")
+    ], {"a": "b"}) == get_history_id("my-full-name", [
+        Parameter(name="a", value="b")
+    ], {})
+
+
+def test_excluded_values_are_ignored():
+    assert get_history_id("my-full-name", [
+        Parameter(name="a", value="1")
+    ], {}) == get_history_id("my-full-name", [
+        Parameter(name="a", value="1"),
+        Parameter(name="b", value="2", excluded=True)
+    ], {})

tests/allure_pytest/unit/history_id_test.py

@@ -148,7 +148,7 @@ def find_node_with_docstring(
 
     node = request.node
     while node:
-        docstring = node.obj.__doc__
+        docstring = getattr(node, "obj", None).__doc__
         if docstring:
             break
         node = node.parent