pytest icon indicating copy to clipboard operation
pytest copied to clipboard

Fixture record_testsuite_property does not work with pytest-xdist

Open marincandenza opened this issue 5 years ago • 14 comments

According to the docs it is possible to add a junit property at test suite level. This will work fine without pytest-xdist but fails when adding -n2 or similiar to pytest args.

Error description The test suite properties ARCH, STORAGE_TYPE shown below are not contained in the junit.xml when running pytest-xdist.

Note I read through the existing test code which does not execute pytest-xdist. Is it wanted to call pytest-xdist in this repository?

How to reproduce

# conftest.py
import pytest

@pytest.fixture(scope="session", autouse=True)
def log_global_env_facts(record_testsuite_property):
    record_testsuite_property("ARCH", "PPC")
    record_testsuite_property("STORAGE_TYPE", "CEPH")
# test_me.py
class TestMe:
    def test_foo(self):
        assert True
(venv) pip list
Package        Version
-------------- -------
apipkg         1.5
atomicwrites   1.4.0
attrs          20.2.0
colorama       0.4.3
execnet        1.7.1
iniconfig      1.0.1
more-itertools 8.5.0
packaging      20.4
pip            20.2.3
pluggy         0.13.1
py             1.9.0
pyparsing      2.4.7
pytest         6.0.2
pytest-forked  1.3.0
pytest-xdist   2.1.0
setuptools     50.3.0
six            1.15.0
toml           0.10.1

marincandenza avatar Sep 17 '20 14:09 marincandenza

@marincandenza could you elaborate more on the problem? Xdist by default will create a session per worker, this is by design so an auto use session scopes fixture for example will execute on each of the workers, this can be confusing at first as you expect one pytest execution, however that is not the case as all workers run pytest end to end

symonk avatar Sep 17 '20 14:09 symonk

@symonk I have edited my bug description. The xml property on testsuite level will not be logged when invoking with pytest-xdist args n2.

marincandenza avatar Sep 18 '20 17:09 marincandenza

Related to #5205.

Zac-HD avatar Sep 19 '20 06:09 Zac-HD

Thanks @marincandenza,

What happens is:

  1. pytest-xdist works by having the main pytest process spawning multiple workers, which do all the collection/running. The main pytest process (master) does not do any collection/test running.
  2. Each worker runs the tests (which creates fixtures as needed), and report back the results to master, which takes care of reporting: showing results in the terminal, and importantly here, writing the junitxml file.
  3. The record_testsuite_property works internally by setting the property into the xml file handler.

So the record_testsuite_property executes on the workers, but they don't generate any reports, the master node is responsible for writing the XML file. This is the reason why the properties don't show up when using pytest-xdist.

To fix this, we need to make the properties part of the TestReport, so they can be correctly transferred to the master node when using pytest-xdist.

nicoddemus avatar Sep 19 '20 14:09 nicoddemus

Created a PR adding a warning to the docs: https://github.com/pytest-dev/pytest/pull/7773

FTR: I played around a bit but I don't see an easy way to fix this, I pushed to https://github.com/nicoddemus/pytest/tree/xml-properties-xdist-7767.

nicoddemus avatar Sep 19 '20 15:09 nicoddemus

Here is a workaround that I use (dirty hack, not a solution):

def _global_property(config, name, value):
    """Temporary hack as record_global_property doesn't work with xdist"""

    from _pytest.junitxml import xml_key
    xml = config._store.get(xml_key, None)
    if xml:
        xml.add_global_property(name, value)

Obviously inspired by junitxml.py and will work as long as the interface will remain unchanged (noticed that it is "bit live" piece of code). It isn't fixture! Personally I call it from pytest_report_header, but not an issue for me, everything what should be recorded is already known and it does the job for me (until real fix). Maybe this helps to others.

It seems obvious that this doesn't solve propagation from slaves

mganisin avatar Sep 24 '20 20:09 mganisin

Is there any planned work to fix this? I was looking for a way to add user info to the junitxml output.

sjalloq avatar Feb 18 '22 10:02 sjalloq

Hi @sjalloq, AFAIK nobody is working on it.

nicoddemus avatar Feb 18 '22 13:02 nicoddemus

@nicoddemus Thanks. Is it a fixable issue? I.E. is it worth me trying to understand the code base and look at it or is it not worth it? I tried @mganisin's hack and that seems to work okay.

sjalloq avatar Feb 18 '22 13:02 sjalloq

Hi @sjalloq, TBH I don't remember the details, but it seems like it isn't trivial/obvious to fix however: https://github.com/pytest-dev/pytest/issues/7767#issuecomment-695280043

nicoddemus avatar Feb 18 '22 13:02 nicoddemus

To add context, this is an issue when using hypothesis with pytest-xdist with junitxml (#5202, https://github.com/HypothesisWorks/hypothesis/issues/1935)

brandonchinn178 avatar Mar 15 '22 19:03 brandonchinn178

So it turns out the Hypothesis pytest plugin actually didn't work with --junit-xml, regardless of whether pytest-xdist is enabled (PR: https://github.com/HypothesisWorks/hypothesis/pull/3277). I got it working without pytest-xdist, and now it's running into the same issue as this ticket. The workaround mentioned by @mganisin seems to not be necessary anymore; record_testsuite_property now does the exact same thing. https://github.com/pytest-dev/pytest/blob/00ad12b9db8d3309c361061fe1c4878b2e0cd51c/src/_pytest/junitxml.py#L373-L376

The remaining issue with pytest-xdist is that record_testsuite_property is now a noop with pytest-xdist, since xml_key isn't in config.stash when pytest-xdist is on: https://github.com/pytest-dev/pytest/blob/00ad12b9db8d3309c361061fe1c4878b2e0cd51c/src/_pytest/junitxml.py#L429-L440

Which goes back to this comment:

So the record_testsuite_property executes on the workers, but they don't generate any reports, the master node is responsible for writing the XML file. This is the reason why the properties don't show up when using pytest-xdist.

To fix this, we need to make the properties part of the TestReport, so they can be correctly transferred to the master node when using pytest-xdist.

brandonchinn178 avatar Mar 29 '22 22:03 brandonchinn178

Thinking about this in bit more depth, "expected" behavior shouldn't be expected probably. Having record_testsuite_property as fixture means that it is ran n-times with xdist and that's unlikely desired behavior.

Maybe it could be better if there was a hook provided by junitxml to solve this (with current fixture kept for compatibility and cases without xdist).

Probably even better would be to solely depend on pytest-metadata when recording custom properties and let either junitxml or pytest-metadata (this tries to do it) to record metadata to junit output (with this for example same values will be recorded in junit as well as in pytest-html -> a bit of consistency). Actually this is a practice that I am going to start following since now.

mganisin avatar Aug 01 '22 10:08 mganisin

has there ever been a solution for this?

KarimVocantas avatar Apr 05 '24 17:04 KarimVocantas