Commit Graph

3288 Commits

Author SHA1 Message Date
Tamir Duberstein 2e65f4e3ac
Initialize cache directory in isolation
Creating and initializing the cache directory is interruptible; this
avoids a pathological case where interrupting a cache write can cause
the cache directory to never be properly initialized with its supporting
files.

Unify `Cache.mkdir` with `Cache.set` while I'm here so the former also
properly initializes the cache directory.

Closes #12167.
2024-04-03 16:26:43 +01:00
Pierre Sassoulas 1125296b53 Small performance/readability improvments when iterating dictionnary with ``keys()``
Based on pylint's message ``consider-iterating-dictionary`` suggestion.
Surprisingly using a dict or set comprehension instead of a new temp var is
actually consistently slower here, which was not intuitive for me.

```python
from timeit import timeit

families = {1: {"testcase": [1, 2, 3, 5, 8]}}
attrs = {1: "a", 2: "b", 3: "c", 4: "d", 5: "e", 6: "f", 7: "g", 8: "h"}

class Old:
    def old(self):
        self.attrs = attrs
        temp_attrs = {}
        for key in self.attrs.keys():
            if key in families[1]["testcase"]:
                temp_attrs[key] = self.attrs[key]
        self.attrs = temp_attrs

class OldBis:
    def old(self):
        self.attrs = attrs
        temp_attrs = {}
        for key in self.attrs:
            if key in families[1]["testcase"]:
                temp_attrs[key] = self.attrs[key]
        self.attrs = temp_attrs

class New:
    def new(self):
        self.attrs = attrs
        self.attrs = { # Even worse with k: v for k in self.attrs.items()
            k: self.attrs[k] for k in self.attrs if k in families[1]["testcase"]
        }

if __name__ == "__main__":
    n = 1000000
    print(f"Old: {timeit(Old().old, number=n)}")
    print(f"Just removing the keys(): {timeit(OldBis().old, number=n)}")
    print(f"List comp, no temp var: {timeit(New().new, number=n)}")
```

Result:
Old: 0.9493889989680611
Just removing the keys(): 0.9042672360083088
List comp, no temp var: 0.9916125109884888

It's also true for the other example with similar benchmark, but the exact
code probably does not need to be in the commit message.
2024-03-31 14:43:07 +02:00
John Litborn e64efd8653
Don't reregister subfixture finalizer in requested fixture if value is cached (#12136) 2024-03-31 15:02:09 +03:00
Ran Benita 3eb16b34be fixtures: stop using `request.param_index` in fixture cache key
When `param` is not defined, `param_index` is always 0 (see
`_compute_fixture_value`), so no point in using it besides adding some
confusion.
2024-03-23 12:07:24 +02:00
Sebastian Meyer e7bf216516
doc: add versionadded to `ExceptionInfo.group_contains` (#12141) 2024-03-19 19:54:26 -03:00
John Litborn 70c11582aa
Don't add fixture finalizer if the value is cached (#11833)
Fixes #1489
2024-03-16 23:45:56 +02:00
pre-commit-ci[bot] c0532dda18
[pre-commit.ci] pre-commit autoupdate (#12115)
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Co-authored-by: Ran Benita <ran@unusedvar.com>
2024-03-13 15:30:18 +02:00
Ran Benita 7eaaf370bb
doc: add versionadded to `Stash` and `StashKey`
Fixes #12107
2024-03-11 18:22:16 +02:00
Ran Benita 0dc0360351 python: fix instance handling in static and class method tests
and also fixes a regression in pytest 8.0.0 where `setup_method` crashes
if the class has static or class method tests.

It is allowed to have a test class with static/class methods which
request non-static/class method fixtures (including `setup_method`
xunit-fixture). I take it as a given that we need to support this
somewhat odd scenario (stdlib unittest also supports it).

This raises a question -- when a staticmethod test requests a bound
fixture, what is that fixture's `self`?

stdlib unittest says - a fresh instance for the test.

Previously, pytest said - some instance that is shared by all
static/class methods. This is definitely broken since it breaks test
isolation.

Change pytest to behave like stdlib unittest here.

In practice, this means stopping to rely on `self.obj.__self__` to get
to the instance from the test function's binding. This doesn't work
because staticmethods are not bound to anything.

Instead, keep the instance explicitly and use that.

BTW, I think this will allow us to change `Class`'s fixture collection
(`parsefactories`) to happen on the class itself instead of a class
instance, allowing us to avoid one class instantiation. But needs more
work.

Fixes #12065.
2024-03-09 19:35:54 +02:00
Ran Benita 774f0c44e6 fixtures: only call `instance` property once in function
No need to compute the property multiple times.
2024-03-09 10:16:41 +02:00
Ran Benita 006058f1f9 fixtures: update outdated comment
No longer does unittest stuff. Also the rest of the sentence is not
really necessary for a docstring.
2024-03-09 10:16:41 +02:00
Ran Benita 437eb86edd
Merge pull request #12092 from bluetech/fixture-cleanup
fixtures: a few more cleanups
2024-03-09 08:55:37 +02:00
Levon Saldamli 9033d4d3ff
Parse args from file (#12085)
Co-authored-by: Ran Benita <ran@unusedvar.com>
Co-authored-by: Bruno Oliveira <bruno@soliv.dev>
2024-03-09 08:51:52 +02:00
Bruno Oliveira 2ccc73be9a
Merge pull request #12087 from nicoddemus/revert-path-deprecations
Revert legacy path removals
2024-03-08 20:06:47 -03:00
Ran Benita ff551b7685 fixtures: simplify a bit of code 2024-03-09 00:02:06 +02:00
Ran Benita f5de111357 fixtures: check scope mismatch in `getfixturevalue` already-cached case
This makes sure the scope is always compatible, and also allows using
`getfixturevalue` in `pytest_fixture_setup` so less internal magic.
2024-03-09 00:02:06 +02:00
Ran Benita 71671f60b5 fixtures: improve fixture scope mismatch message
- Separate the requesting from the requested.

- Avoid the term "factory", I think most people don't distinguish
  between "fixture" and "fixture function" (i.e. "factory") and would
  find the term "factory" unfamiliar.
2024-03-09 00:02:06 +02:00
Ran Benita a9d1f55a0f fixtures: simplify scope checking
There are two non-optimal things in the current way scope checking is
done:

- It runs on `SubRequest`, but doesn't use the `SubRequest's scope,
  which is confusing. Instead it takes `invoking_scope` and
  `requested_scope`.

- Because `_check_scope` is only defined on `SubRequest` and not
  `TopRequest`, `_compute_fixture_value` first creates the `SubRequest`
  only then checks the scope (hence the need for the previous point).

Instead, also define `_check_scope` on `TopRequest` (always valid), and
remove `invoking_scope`, using `self._scope` instead.
2024-03-09 00:02:06 +02:00
Ran Benita 1a5e0eb71d unittest: make `obj` work more like `Function`/`Class`
Previously, the `obj` of a `TestCaseFunction` (the unittest plugin item
type) was the unbound method. This is unlike regular `Class` where the
`obj` is a bound method to a fresh instance.

This difference necessitated several special cases in in places outside
of the unittest plugin, such as `FixtureDef` and `FixtureRequest`, and
made things a bit harder to understand.

Instead, match how the python plugin does it, including collecting
fixtures from a fresh instance.

The downside is that now this instance for fixture-collection is kept
around in memory, but it's the same as `Class` so nothing new. Users
should only initialize stuff in `setUp`/`setUpClass` and similar
methods, and not in `__init__` which is generally off-limits in
`TestCase` subclasses.

I am not sure why there was a difference in the first place, though I
will say the previous unittest approach is probably the preferable one,
but first let's get consistency.
2024-03-08 14:03:26 +02:00
Bruno Oliveira dacee1f11d Revert "Remove deprecated py.path hook arguments"
This reverts commit a98f02d423.
2024-03-07 19:50:33 -03:00
Bruno Oliveira 303cd0d48a Revert "Remove deprecated py.path (`fspath`) node constructor arguments"
This reverts commit 6c89f9261c.
2024-03-07 19:50:33 -03:00
Bruno Oliveira 03e54712dd
Do not import duplicated modules with --importmode=importlib (#12074)
Regression brought up by #11475.
2024-03-04 12:44:56 -03:00
Ran Benita 00043f7f10
Merge pull request #12038 from bluetech/fixtures-rm-arg2index
fixtures: avoid mutable arg2index state in favor of looking up the request chain
2024-03-03 15:45:34 +02:00
Ran Benita f4e10251a4
Merge pull request #12048 from bluetech/fixture-teardown-excgroup
fixtures: use exception group when multiple finalizers raise in fixture teardown
2024-03-03 15:09:36 +02:00
Bruno Oliveira 89ee4493cc
Merge pull request #11997 from nicoddemus/11475-importlib
Change importlib to first try to import modules using the standard mechanism
2024-03-03 09:43:14 -03:00
mrbean-bremen 8248946a55
Do not collect symlinked tests under Windows (#12050)
The check for short paths under Windows via os.path.samefile, introduced in #11936, also found similar tests in symlinked tests in the GH Actions CI.

Fixes #12039.

Co-authored-by: Bruno Oliveira <bruno@soliv.dev>
2024-03-03 09:41:31 -03:00
Ran Benita 434282e17f fixtures: use exception group when multiple finalizers raise in fixture teardown
Previously, if more than one fixture finalizer raised, only the first
was reported, and the other errors were lost.

Use an exception group to report them all. This is similar to the change
we made in node teardowns (in `SetupState`).
2024-03-02 23:39:11 +02:00
Bruno Oliveira 111c0d910e Add consider_namespace_packages ini option
Fix #11475
2024-03-02 16:13:48 -03:00
Bruno Oliveira 199d4e2b73 pathlib: import signature and docs for import_path 2024-03-02 16:13:48 -03:00
Bruno Oliveira c85fce39b6 Change importlib to first try to import modules using the standard mechanism
As detailed in
https://github.com/pytest-dev/pytest/issues/11475#issuecomment-1937043670,
currently with `--import-mode=importlib` pytest will try to import every
file by using a unique module name, regardless if that module could be
imported using the normal import mechanism without touching `sys.path`.

This has the consequence that non-test modules available in `sys.path`
(via other mechanism, such as being installed into a virtualenv,
PYTHONPATH, etc) would end up being imported as standalone modules,
instead of imported with their expected module names.

To illustrate:

```
.env/
  lib/
    site-packages/
      anndata/
        core.py
```

Given `anndata` is installed into the virtual environment, `python -c
"import anndata.core"` works, but pytest with `importlib` mode would
import that module as a standalone module named
`".env.lib.site-packages.anndata.core"`, because importlib module was
designed to import test files which are not reachable from `sys.path`,
but now it is clear that normal modules should be imported using the
standard mechanisms if possible.

Now `imporlib` mode will first try to import the module normally,
without changing `sys.path`, and if that fails it falls back to
importing the module as a standalone module.

This also makes `importlib` respect namespace packages.

This supersedes #11931.

Fix #11475
Close #11931
2024-03-02 16:13:48 -03:00
Bruno Oliveira 067daf9f7d pathlib: consider namespace packages in `resolve_pkg_root_and_module_name`
This applies to `append` and `prepend` import modes; support for
`importlib` mode will be added in a separate change.
2024-03-02 16:13:48 -03:00
Bruno Oliveira 5867426455 pathlib: handle filenames starting with `.` in `module_name_from_path` 2024-03-02 16:13:48 -03:00
Bruno Oliveira 4dea18308b pathlib: extract a function `_import_module_using_spec`
Will be reused.
2024-03-02 16:13:48 -03:00
Bruno Oliveira 7524e60d8f pathlib: extract a function `resolve_pkg_root_and_module_name`
Will be reused.
2024-03-02 16:13:48 -03:00
Ran Benita 6ed005161d
Merge pull request #12043 from bluetech/pyargs-root
Fix collection failures due to permission errors when using `--pyargs`
2024-03-02 21:09:20 +02:00
Ran Benita 31026a2df2 main: only include package parents in collection tree for --pyargs collection arguments
(diff better viewed ignoring whitespace)

In pytest<8, the collection tree for `pyargs` arguments in an invocation
like this:

    pytest --collect-only --pyargs pyflakes.test.test_undefined_names

looked like this:

```
<Package test>
  <Module test_undefined_names.py>
    <UnitTestCase Test>
      <TestCaseFunction test_annotationUndefined>
      ... snipped ...
```

The pytest 8 collection improvements changed it to this:

```
<Dir pytest>
  <Dir .tox>
    <Dir venv>
      <Dir lib>
        <Dir python3.11>
          <Dir site-packages>
            <Package pyflakes>
              <Package test>
                <Module test_undefined_names.py>
                  <UnitTestCase Test>
                    <TestCaseFunction test_annotationUndefined>
                    ... snipped ...
```

Besides being egregious (and potentially even worse than the above,
going all the way to the root, for system-installed packages, as is
apparently common in CI), this also caused permission errors when trying
to probe some of those intermediate directories.

This change makes `--pyargs` arguments no longer try to add parent
directories to the collection tree according to the `--confcutdir` like
they're regular arguments. Instead, only add the parents that are in the
import path. This now looks like this:

```
<Package .tox/venv/lib/python3.11/site-packages/pyflakes>
  <Package test>
    <Module test_undefined_names.py>
      <UnitTestCase Test>
        <TestCaseFunction test_annotationUndefined>
        ... snipped ...
```

Fix #11904.
2024-03-02 20:26:02 +02:00
Ran Benita f20e32c982 main: slight refactor to collection argument parents logic
No logical change, preparation for the next commit.
2024-03-02 20:26:02 +02:00
Ran Benita 1612d4e393 main: add `module_name` to `CollectionArgument`
This is available when the argument is a `--pyargs` argument (resolved
from a python module path). Will be used in an upcoming commit.
2024-03-02 20:26:02 +02:00
Ran Benita 5e0d11746c main: model the result of `resolve_collection_arguments` as a dataclass
In preparation of adding more info to it.
2024-03-02 20:25:57 +02:00
Ran Benita ff4c3b2873 main: add missing `import importlib.util`
Used in `resolve_collection_argument`. It's implicitly imported by some
other import, but some type checkers don't recognize this.
2024-03-01 13:28:27 +02:00
Ran Benita bd45ccd2ca fixtures: avoid mutable `arg2index` state in favor of looking up the request chain
pytest allows a fixture to request its own name (directly or
indirectly), in which case the fixture with the same name but one level
up is used.

To know which fixture should be used next, pytest keeps a mutable
item-global dict `_arg2index` which maintains this state. This is not
great:

- Mutable state like this is hard to understand and reason about.

- It is conceptually buggy; the indexing is global (e.g. if requesting
  `fix1` and `fix2`, the indexing is shared between them), but actually
  different branches of the subrequest tree should not affect each
  other.

  This is not an issue in practice because pytest keeps a cache of the
  fixturedefs it resolved anyway (`_fixture_defs`), but if the cache is
  removed it becomes evident.

Instead of the `_arg2index` state, count how many `argname`s deep we are
in the subrequest tree ("the fixture stack") and use that for the index.

This way, no global mutable state and the logic is very localized and
easier to understand.

This is slower, however fixture stacks should not be so deep that this
matters much, I hope.
2024-02-27 22:24:26 +02:00
Ran Benita c83c1c4bda fixtures: add `_iter_chain` helper method
Will be reused in the next commit.
2024-02-27 22:24:26 +02:00
Ran Benita 686f9e0720 fixtures: remove unneeded optimization from `_getnextfixturedef`
According to my understanding, this code, which handles obtaining the
relevant fixturedefs when a dynamic `getfixturevalue` is used, has an
optimization where it only grabs fixturedefs that are visible to the
*parent* of the item, instead of the item itself, under the assumption
that a fixturedef can't be visible to a single item, only to a
collector.

Remove this optimization for the following reasons:
- It doesn't save much (one loop iteration in `matchfactories`)
- It slightly complicates the complex fixtures code
- If some plugin wants to make a fixture visible only to a single item,
  why not let it?
- In the static case (`getfixtureclosure`), this optimization is not
  done (despite the confusing name `parentnode`, it is *not* the parent
  node). This is inconsistent.
2024-02-27 22:22:07 +02:00
Patrick Lannigan 84bd31de64
New verbosity_test_case ini option (#11653)
Allow for the output of test case execution to be controlled independently from the application verbosity level. 

`verbosity_test_case` is the new ini setting to adjust this functionality.

Fix #11639
2024-02-24 16:27:54 -03:00
robotherapist c09f74619b runner: add support for `sys.last_exc` for post-mortem debugging on Python>=3.12
Fix #11850
2024-02-23 15:59:32 +02:00
Ran Benita b510adf9ef
Merge pull request #12023 from bluetech/fixtures-cleanups-2
fixtures: remove an unneeded suppress
2024-02-23 15:35:17 +02:00
Ran Benita 93cd7ba857
Merge pull request #12022 from bluetech/revert-11721
Revert "Fix teardown error reporting when `--maxfail=1` (#11721)"
2024-02-23 15:34:32 +02:00
Bruno Oliveira 8d9b95dcdb
Fix collection of short paths on Windows (#11936)
Passing a short path in the command line was causing the matchparts check to fail, because ``Path(short_path) != Path(long_path)``.

Using ``os.path.samefile`` as fallback ensures the comparsion works on Windows when comparing short/long paths.

Fix #11895
2024-02-23 07:51:15 -03:00
Ran Benita ad651ddfce fixtures: remove an unneeded suppress
In `CallSpec2.setmulti` the `params` and `_arg2scope` dicts are always
set together.

Further, the `get_parametrized_fixture_keys` accesses `_arg2scope` for
all argnames without a check, which I think rules out that the code
protects against plugin shenanigans.

After removing the suppress, adjust the comment and code to make more
sense.
2024-02-23 11:51:49 +02:00
Ran Benita 00d9640abc Revert "Fix teardown error reporting when `--maxfail=1` (#11721)"
Fix #12021.
Reopens #11706.

This reverts commit 12b9bd5801.

This change caused a bad regression in pytest-xdist:
https://github.com/pytest-dev/pytest-xdist/issues/1024

pytest-xdist necessarily has special handling of `--maxfail` and session
fixture teardown get executed multiple times with the change.

Since I'm not sure how to adapt pytest-xdist myself, revert for now.

I kept the sticky `shouldstop`/`shouldfail` changes as they are good
ideas regardless I think.
2024-02-23 11:45:26 +02:00