Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect docs: Making session-scoped fixtures execute only once #578

Closed
brandon-leapyear opened this issue Aug 14, 2020 · 1 comment · Fixed by #582
Closed

Incorrect docs: Making session-scoped fixtures execute only once #578

brandon-leapyear opened this issue Aug 14, 2020 · 1 comment · Fixed by #582

Comments

@brandon-leapyear
Copy link

brandon-leapyear commented Aug 14, 2020

https://github.com/pytest-dev/pytest-xdist/blob/master/README.rst#making-session-scoped-fixtures-execute-only-once

worker_id will never be a falsey value to satisfy the if not worker_id check. If tests are not running in distributed workers, worker_id will be set to master.

https://github.com/pytest-dev/pytest-xdist/blob/master/src/xdist/plugin.py#L221-L233

I'm not sure what the correct solution is here.

  • If worker_id == 'master' guarantees that tests are not running in distributed workers, then one should just change not worker_id to worker_id == 'master'.
  • Otherwise, the worker_id fixture should be changed (or another fixture should be added) to distinguish between a non-distributed master node and a distributed master node.
  • Alternatively, we can not special case the master node and just do something like
 @pytest.fixture(scope="session")
 def session_data(tmp_path_factory, worker_id):
-    if not worker_id:
-        # not executing in with multiple workers, just produce the data and let
-        # pytest's fixture caching do its job
-        return produce_expensive_data()
-
-    # get the temp directory shared for by all workers
-    root_tmp_dir = tmp_path_factory.getbasetemp().parent
+    tmp_dir = tmp_path_factory.getbasetemp()
+    root_tmp_dir = tmp_dir if worker_id == 'master' else tmp_dir.parent

     fn = root_tmp_dir / "data.json"
     with FileLock(str(fn) + ".lock"):
         if fn.is_file():
             data = json.loads(fn.read_text())
         else:
             data = produce_expensive_data()
             fn.write_text(json.dumps(data))
     return data
@nicoddemus
Copy link
Member

If worker_id == 'master' guarantees that tests are not running in distributed workers, then one should just change not worker_id to worker_id == 'master'.

This is the correct one: when distributing tests, the master node will never execute any tests, so it is not possible for worker_id be "master" when distributing tests. I'm fixing that in #582.

Thanks for the report!

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 a pull request may close this issue.

2 participants