From 86973a6cd51f6b918685dd7cb8c2d1a375b546e3 Mon Sep 17 00:00:00 2001 From: yakimka Date: Wed, 9 Oct 2024 14:04:59 +0300 Subject: [PATCH 1/6] Add tests for detecting race conditions --- .github/workflows/workflow-ci.yml | 44 +++++++++++++++++++++++++++ picodi/_picodi.py | 26 ++++++++-------- poetry.lock | 30 ++++++++++++++++++- pyproject.toml | 2 ++ tests/test_race.py | 49 +++++++++++++++++++++++++++++++ 5 files changed, 136 insertions(+), 15 deletions(-) create mode 100644 tests/test_race.py diff --git a/.github/workflows/workflow-ci.yml b/.github/workflows/workflow-ci.yml index 3cece0c..a98159f 100644 --- a/.github/workflows/workflow-ci.yml +++ b/.github/workflows/workflow-ci.yml @@ -138,6 +138,50 @@ jobs: && .venv/bin/pytest " + free-threading-test: + runs-on: ubuntu-latest + permissions: + contents: read + packages: write + strategy: + matrix: + image-name: [ 'quay.io/pypa/manylinux_2_28_x86_64'] + + steps: + - uses: actions/checkout@v4 + + - name: Prepare Docker + run: | + docker login "$CACHE_REGISTRY" -u "$CACHE_REGISTRY_USERNAME" --password="${CACHE_REGISTRY_TOKEN}" + docker buildx create --use --driver=docker-container + docker --version && docker compose --version + + - name: Load cached venv + id: cached-venv + uses: actions/cache@v4 + with: + path: | + .venv + key: free-threading${{ matrix.image-name }}-${{ hashFiles('./poetry.lock') }} + + - name: Pull docker image + run: | + docker pull ${{ matrix.image-name }} + docker tag ${{ matrix.image-name }} free-threading-image + + - name: Run checks + run: > + docker run -e VIRTUAL_ENV=/opt/code/.venv --rm -v $(pwd):/opt/code -w /opt/code + free-threading-image bash -c " + if [ ! -d .venv ]; then + python3.13t -m venv .venv; + fi + && .venv/bin/pip install pytest pytest-asyncio pytest-cov pytest-randomly pytest-race pytest-repeat + && .venv/bin/pip install --no-deps -e . + && .venv/bin/python -VV + && .venv/bin/pytest --ignore=tests/test_integrations + " + release-package: runs-on: ubuntu-latest needs: [ check-code ] diff --git a/picodi/_picodi.py b/picodi/_picodi.py index 42114ae..29edbaf 100644 --- a/picodi/_picodi.py +++ b/picodi/_picodi.py @@ -14,7 +14,7 @@ Iterable, Iterator, ) -from contextlib import asynccontextmanager, contextmanager +from contextlib import asynccontextmanager, contextmanager, nullcontext from dataclasses import dataclass from typing import ( Annotated, @@ -89,10 +89,9 @@ def add( ) def get(self, dependency: DependencyCallable) -> Provider: - with self._storage.lock: - dependency = self.get_dep_or_override(dependency) - self._storage.touched_dependencies.add(dependency) - return self._storage.deps[dependency] + dependency = self.get_dep_or_override(dependency) + self._storage.touched_dependencies.add(dependency) + return self._storage.deps[dependency] def get_dep_or_override(self, dependency: DependencyCallable) -> DependencyCallable: return self._storage.overrides.get(dependency, dependency) @@ -210,16 +209,14 @@ def clear_overrides(self) -> None: """ Clear all overrides. It will remove all overrides, but keep the dependencies. """ - with self._storage.lock: - self._storage.overrides.clear() + self._storage.overrides.clear() def clear_touched(self) -> None: """ Clear the touched dependencies. It will remove all dependencies resolved during the picodi lifecycle. """ - with self._storage.lock: - self._storage.touched_dependencies.clear() + self._storage.touched_dependencies.clear() def clear(self) -> None: """ @@ -227,10 +224,9 @@ def clear(self) -> None: This method will not close any dependencies. So you need to manually call :func:`shutdown_dependencies` before this method. """ - with self._storage.lock: - self._storage.deps.clear() - self._storage.overrides.clear() - self._storage.touched_dependencies.clear() + self._storage.deps.clear() + self._storage.overrides.clear() + self._storage.touched_dependencies.clear() _registry_storage = RegistryStorage() @@ -241,7 +237,6 @@ def clear(self) -> None: SingletonScope: SingletonScope(), ContextVarScope: ContextVarScope(), } -_lock = threading.RLock() def Provide(dependency: DependencyCallable, /) -> Any: # noqa: N802 @@ -727,6 +722,9 @@ def _extract_and_register_dependency_from_parameter( return None +_lock = nullcontext() + + class LazyResolver: def __init__( self, diff --git a/poetry.lock b/poetry.lock index 4ba3edd..0ee872f 100644 --- a/poetry.lock +++ b/poetry.lock @@ -969,6 +969,20 @@ files = [ markdown-it-py = ">=2.2.0,<4.0" pytest = ">=7.0.0" +[[package]] +name = "pytest-race" +version = "0.2.0" +description = "Race conditions tester for pytest" +optional = false +python-versions = ">=3.2" +files = [ + {file = "pytest-race-0.2.0.tar.gz", hash = "sha256:92a43179ece42366ae2207bf4160bdb64781dc2ca425d1b37dc310c58ad0ade3"}, + {file = "pytest_race-0.2.0-py3-none-any.whl", hash = "sha256:f2ae6cf1feece53619a76534a38863565b95f91436e78fb1bc7090234317eaf9"}, +] + +[package.dependencies] +pytest = ">=2.9.0" + [[package]] name = "pytest-randomly" version = "3.15.0" @@ -983,6 +997,20 @@ files = [ [package.dependencies] pytest = "*" +[[package]] +name = "pytest-repeat" +version = "0.9.3" +description = "pytest plugin for repeating tests" +optional = false +python-versions = ">=3.7" +files = [ + {file = "pytest_repeat-0.9.3-py3-none-any.whl", hash = "sha256:26ab2df18226af9d5ce441c858f273121e92ff55f5bb311d25755b8d7abdd8ed"}, + {file = "pytest_repeat-0.9.3.tar.gz", hash = "sha256:ffd3836dfcd67bb270bec648b330e20be37d2966448c4148c4092d1e8aba8185"}, +] + +[package.dependencies] +pytest = "*" + [[package]] name = "pyyaml" version = "6.0.2" @@ -1324,4 +1352,4 @@ test = ["covdefaults (>=2.3)", "coverage (>=7.2.7)", "coverage-enable-subprocess [metadata] lock-version = "2.0" python-versions = "^3.10" -content-hash = "388636c6be7cc1eae687d98144135f36cd87a9179ddb0bf24cefca9857616401" +content-hash = "b56c180aa42a2299c75d0bc603b2d7fbc9d8c71295a24d987f3083652d71b7ea" diff --git a/pyproject.toml b/pyproject.toml index 4f723e7..4c9802d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -42,6 +42,8 @@ httpx = "^0.27.0" pytest-markdown-docs = "^0.5.1" sphinx = ">=7.3.7,<9.0.0" furo = "^2024.5.6" +pytest-race = "0.2.0" +pytest-repeat = "^0.9.3" [tool.isort] # isort configuration: diff --git a/tests/test_race.py b/tests/test_race.py new file mode 100644 index 0000000..ad04050 --- /dev/null +++ b/tests/test_race.py @@ -0,0 +1,49 @@ +import asyncio +from random import randint + +import pytest + +from picodi import Provide, SingletonScope, dependency, inject + + +@pytest.mark.repeat(5) +def test_scope_resolving_races(start_race): + @dependency(scope_class=SingletonScope) + def get_random_int(): + return randint(1, 10000) + + @inject + def service(num: int = Provide(get_random_int)): + return num + + results = [] + + def actual_test(): + results.append(service()) + + start_race(threads_num=8, target=actual_test) + + assert len(set(results)) == 1, results + + +@pytest.mark.repeat(5) +def test_scope_resolving_races_async(start_race): + @dependency(scope_class=SingletonScope) + async def get_random_int(): + return randint(1, 10000) + + @inject + async def service(num: int = Provide(get_random_int)): + return num + + results = [] + + async def main(): + results.append(await service()) + + def actual_test(): + asyncio.run(main()) + + start_race(threads_num=8, target=actual_test) + + assert len(set(results)) == 1, results From c80057153109e42aecc87de381606ded250dd36c Mon Sep 17 00:00:00 2001 From: yakimka Date: Wed, 9 Oct 2024 15:35:05 +0300 Subject: [PATCH 2/6] Try to install all deps --- .github/workflows/workflow-ci.yml | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/.github/workflows/workflow-ci.yml b/.github/workflows/workflow-ci.yml index a98159f..24c66ee 100644 --- a/.github/workflows/workflow-ci.yml +++ b/.github/workflows/workflow-ci.yml @@ -145,7 +145,7 @@ jobs: packages: write strategy: matrix: - image-name: [ 'quay.io/pypa/manylinux_2_28_x86_64'] + image-name: [ 'quay.io/pypa/musllinux_1_2_x86_64'] steps: - uses: actions/checkout@v4 @@ -174,12 +174,20 @@ jobs: docker run -e VIRTUAL_ENV=/opt/code/.venv --rm -v $(pwd):/opt/code -w /opt/code free-threading-image bash -c " if [ ! -d .venv ]; then + # Remove this after cryptography can be installed in free-threading mode + # (pytest depends on cryptography) + python3.13 -m venv venvpoetry; + ./venvpoetry/bin/pip install poetry; + ./venvpoetry/bin/poetry self add poetry-plugin-export; + ./venvpoetry/bin/poetry export --with=dev --format=requirements.txt --output=requirements.txt; + rm -rf venvpoetry; + python3.13t -m venv .venv; fi - && .venv/bin/pip install pytest pytest-asyncio pytest-cov pytest-randomly pytest-race pytest-repeat + && .venv/bin/pip install -r requirements.txt && .venv/bin/pip install --no-deps -e . && .venv/bin/python -VV - && .venv/bin/pytest --ignore=tests/test_integrations + && .venv/bin/pytest " release-package: From 81c245b5aaf0a3ed6b0153fb8b0e656317203b19 Mon Sep 17 00:00:00 2001 From: yakimka Date: Wed, 9 Oct 2024 15:39:57 +0300 Subject: [PATCH 3/6] Fix tests --- .github/workflows/workflow-ci.yml | 2 +- picodi/_picodi.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/workflow-ci.yml b/.github/workflows/workflow-ci.yml index 24c66ee..51f8ba2 100644 --- a/.github/workflows/workflow-ci.yml +++ b/.github/workflows/workflow-ci.yml @@ -145,7 +145,7 @@ jobs: packages: write strategy: matrix: - image-name: [ 'quay.io/pypa/musllinux_1_2_x86_64'] + image-name: ['quay.io/pypa/manylinux_2_28_x86_64'] steps: - uses: actions/checkout@v4 diff --git a/picodi/_picodi.py b/picodi/_picodi.py index 29edbaf..a118787 100644 --- a/picodi/_picodi.py +++ b/picodi/_picodi.py @@ -14,7 +14,7 @@ Iterable, Iterator, ) -from contextlib import asynccontextmanager, contextmanager, nullcontext +from contextlib import asynccontextmanager, contextmanager from dataclasses import dataclass from typing import ( Annotated, @@ -722,7 +722,7 @@ def _extract_and_register_dependency_from_parameter( return None -_lock = nullcontext() +_lock = threading.RLock() class LazyResolver: From 03ebeb1c0b73d28af73f95292f7ad1e9c2839d31 Mon Sep 17 00:00:00 2001 From: yakimka Date: Wed, 9 Oct 2024 15:52:20 +0300 Subject: [PATCH 4/6] Fix workflow --- .github/workflows/workflow-ci.yml | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/.github/workflows/workflow-ci.yml b/.github/workflows/workflow-ci.yml index 51f8ba2..7b4905d 100644 --- a/.github/workflows/workflow-ci.yml +++ b/.github/workflows/workflow-ci.yml @@ -101,7 +101,7 @@ jobs: packages: write strategy: matrix: - pypy-version: [ '3.10'] + pypy-version: ['3.10'] steps: - uses: actions/checkout@v4 @@ -174,20 +174,12 @@ jobs: docker run -e VIRTUAL_ENV=/opt/code/.venv --rm -v $(pwd):/opt/code -w /opt/code free-threading-image bash -c " if [ ! -d .venv ]; then - # Remove this after cryptography can be installed in free-threading mode - # (pytest depends on cryptography) - python3.13 -m venv venvpoetry; - ./venvpoetry/bin/pip install poetry; - ./venvpoetry/bin/poetry self add poetry-plugin-export; - ./venvpoetry/bin/poetry export --with=dev --format=requirements.txt --output=requirements.txt; - rm -rf venvpoetry; - python3.13t -m venv .venv; fi - && .venv/bin/pip install -r requirements.txt + && .venv/bin/pip install pytest pytest-asyncio pytest-cov pytest-randomly pytest-race pytest-repeat && .venv/bin/pip install --no-deps -e . && .venv/bin/python -VV - && .venv/bin/pytest + && .venv/bin/pytest --ignore=tests/test_integrations " release-package: From 63aba6709e2159cd85154fd12ca536ee4093bdec Mon Sep 17 00:00:00 2001 From: yakimka Date: Wed, 9 Oct 2024 16:11:41 +0300 Subject: [PATCH 5/6] Add needs to release package job --- .github/workflows/workflow-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/workflow-ci.yml b/.github/workflows/workflow-ci.yml index 7b4905d..01cc52f 100644 --- a/.github/workflows/workflow-ci.yml +++ b/.github/workflows/workflow-ci.yml @@ -184,7 +184,7 @@ jobs: release-package: runs-on: ubuntu-latest - needs: [ check-code ] + needs: [check-code, pypy-test, free-threading-test] steps: - uses: actions/checkout@v4 From 082f65d90d21f2a3a413f775bf3d65418e90442c Mon Sep 17 00:00:00 2001 From: yakimka Date: Wed, 9 Oct 2024 16:52:52 +0300 Subject: [PATCH 6/6] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cd9b15..03d64a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ We follow [Semantic Versions](https://semver.org/). - Removed dead code - Added `init_dependencies` marker for pytest +- Added tests for detecting race conditions +- Added Python 3.13-free-threading to CI ## Version 0.27.0