smarie / python-pytest-harvest

Store data created during your `pytest` tests execution, and retrieve it at the end of the session, e.g. for applicative benchmarking purposes.
https://smarie.github.io/python-pytest-harvest/
BSD 3-Clause "New" or "Revised" License
63 stars 9 forks source link

Bug? FileNotFound on cleanup #46

Open jmrgibson opened 3 years ago

jmrgibson commented 3 years ago

Neat plugin, great docs. I'm running into the following issue, outlined below:

Scenario:

stacktrace:

  File "/home/builduser/.shiv/pytest_f6cdbae7-eadd-41ab-aef2-dd73926c7212/site-packages/pytest_harvest/plugin.py", line 435, in pytest_harvest_xdist_cleanup
    rmtree(self.results_path)
  File "/usr/lib/python3.7/shutil.py", line 485, in rmtree
    onerror(os.lstat, path, sys.exc_info())
  File "/usr/lib/python3.7/shutil.py", line 483, in rmtree
    orig_st = os.lstat(path)
FileNotFoundError: [Errno 2] No such file or directory: '.xdist_harvested'

How is self.results_path generated? it it unique per process? or would it be shared if I was running two copies of pytest simultaneously, thus leading to the following error?

EDIT: I should probably mention I'm only using the following hook in my root conftest.py

def pytest_sessionfinish(session):
    """ Gather all results and save them to a csv.
    Works both on worker and master nodes, and also with xdist disabled"""

    session_results_df = get_session_results_df(session)
    suffix = 'all' if is_main_process(session) else get_xdist_worker_id(session)
    session_results_df.to_csv('results_%s.csv' % suffix)
smarie commented 3 years ago

Hi @jmrgibson, thanks for submitting this issue ! Indeed results_path is not unique per process: https://github.com/smarie/python-pytest-harvest/blob/86a259bcb7ac09a643d42a447d7c2e791e403159/pytest_harvest/plugin.py#L398

That is currently documented here: https://smarie.github.io/python-pytest-harvest/#pytest-x-dist

We'll probably need to change this. Do you have an alternative in mind ? Maybe as simple as appending os.getpid() to that path ?

jmrgibson commented 3 years ago

I've implement the hooks on my side using a tempdir, which also helps with cleanup as the space gets reclaimed when the process exits.

I can make that the default and put up a PR if that suits.

On Thu, Dec 10, 2020, 9:33 PM Sylvain Marié notifications@github.com wrote:

Hi @jmrgibson https://github.com/jmrgibson, thanks for submitting this issue ! Indeed results_path is not unique per process: https://github.com/smarie/python-pytest-harvest/blob/86a259bcb7ac09a643d42a447d7c2e791e403159/pytest_harvest/plugin.py#L398

That is currently documented here: https://smarie.github.io/python-pytest-harvest/#pytest-x-dist

We'll probably need to change this. Do you have an alternative in mind ? Maybe as simple as appending os.getpid() to that path ? If so, do not hesitate to propose a PR. Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/smarie/python-pytest-harvest/issues/46#issuecomment-742367746, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4NBMYHJ6G3RUALLNCUNJLSUCBVDANCNFSM4US3B4DA .

smarie commented 3 years ago

This seems like a viable alternative. However I am concerned with debugging. Most users won't be aware that this temp dir is created behind the scenes, and therefore will not be able to perform a bit of debugging themselves by inspecting the files created inside.

Even if this is less elegant, is there any drawback that you might see with appending the process number to this dir name ?

I'm trying to weight one against the other... puzzling