mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-08-17 00:03:22 +00:00
Idea of using spec= for mocks further
This commit is contained in:
Trenton Holmes
@@ -0,0 +1,773 @@
|
||||
# Add `spec=` to Unspecced Mocks Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Add `spec=` or `spec_set=` parameters to ~176 bare `MagicMock()`/`Mock()` usages across the backend test suite so that typos in attribute/method names fail at test time instead of silently passing.
|
||||
|
||||
**Architecture:** Each task targets one test file. For every bare mock, identify the real class being simulated from context (patch target, variable name, how attributes are set), then add `spec=TheRealClass`. Where the real class is a third-party object (llama-index, pikepdf), import it directly in the test module. No production code changes -- this is tests only.
|
||||
|
||||
**Tech Stack:** Python, pytest, `unittest.mock` (MagicMock/Mock `spec=`/`spec_set=`), Django models, llama-index, pikepdf, requests
|
||||
|
||||
---
|
||||
|
||||
## Background: How to choose the right spec
|
||||
|
||||
- `spec=SomeClass` -- mock has all attributes/methods that `SomeClass` has; setting unexpected ones raises `AttributeError`
|
||||
- `spec_set=SomeClass` -- stricter: also prevents _setting_ undeclared attributes (use for value objects / data classes)
|
||||
- For Django QuerySets use `spec=QuerySet` from `django.db.models`
|
||||
- For `requests.Response` use `spec=requests.Response`
|
||||
- For llama-index: `spec=VectorStoreIndex`, `spec=VectorIndexRetriever`, `spec=NodeWithScore` (from `llama_index.core`)
|
||||
- For pikepdf: `spec=pikepdf.Pdf`, `spec=pikepdf.Page`
|
||||
- For Django models: `spec=Document`, `spec=Tag`, etc. (already imported in most test files)
|
||||
- When a mock is created with keyword attributes it doesn't own (e.g. `MagicMock(ids=[...], similarities=[...])`), use `spec=` on the enclosing object and pass real-typed return values; or if the object is a plain data carrier with no real class, leave it as-is and add a comment.
|
||||
|
||||
## Identifying mocks that should be left alone
|
||||
|
||||
Skip adding `spec=` when:
|
||||
|
||||
- The mock is patching a module-level function (patch target is a function, not a class) and the return value's type doesn't matter
|
||||
- The mock is a `PropertyMock` or `patch.dict`
|
||||
- The mock is an intermediate chain (e.g. `mock_obj.some_method.return_value = MagicMock()`) where the intermediate value is never directly asserted on
|
||||
|
||||
---
|
||||
|
||||
## Task 1: `paperless_ai/tests/test_ai_indexing.py` -- QuerySet and config mocks
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `src/paperless_ai/tests/test_ai_indexing.py`
|
||||
|
||||
**Context:**
|
||||
`mock_queryset = MagicMock()` appears 5+ times, simulating `Document.objects.all()` or `.filter()` return values (Django `QuerySet`).
|
||||
`mock_config = MagicMock()` simulates `AIConfig`.
|
||||
|
||||
- [ ] **Step 1: Add QuerySet and AIConfig imports**
|
||||
|
||||
At the top of the file, alongside existing imports, add:
|
||||
|
||||
```python
|
||||
from django.db.models import QuerySet
|
||||
from paperless.config import AIConfig
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Add `spec=` to all `mock_queryset = MagicMock()` instances**
|
||||
|
||||
Search for the pattern and replace each one:
|
||||
|
||||
```bash
|
||||
rg -n "mock_queryset = MagicMock\(\)" src/paperless_ai/tests/test_ai_indexing.py
|
||||
```
|
||||
|
||||
Change each occurrence from:
|
||||
|
||||
```python
|
||||
mock_queryset = MagicMock()
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```python
|
||||
mock_queryset = MagicMock(spec=QuerySet)
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Add `spec=` to `mock_config = MagicMock()`**
|
||||
|
||||
```python
|
||||
mock_config = MagicMock(spec=AIConfig)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the tests for this file**
|
||||
|
||||
```bash
|
||||
uv run pytest --override-ini="addopts=" src/paperless_ai/tests/test_ai_indexing.py -v
|
||||
```
|
||||
|
||||
Expected: all previously passing tests still pass. Any `AttributeError` on a mock attribute indicates a real interface mismatch -- fix by either correcting the test or verifying the real attribute exists on the class.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add src/paperless_ai/tests/test_ai_indexing.py
|
||||
git commit -m "Test: add spec= to QuerySet and AIConfig mocks in test_ai_indexing"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2: `paperless_ai/tests/test_ai_indexing.py` -- VectorStoreIndex, Retriever, and Node mocks
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `src/paperless_ai/tests/test_ai_indexing.py`
|
||||
|
||||
**Context:**
|
||||
The file also contains `mock_index = MagicMock()`, `mock_retriever = MagicMock()`, `mock_node1/2 = MagicMock()` that simulate llama-index objects. These are the highest-value targets since llama-index interfaces change between versions.
|
||||
|
||||
- [ ] **Step 1: Add llama-index imports**
|
||||
|
||||
```python
|
||||
from llama_index.core import VectorStoreIndex
|
||||
from llama_index.core.retrievers import VectorIndexRetriever
|
||||
from llama_index.core.schema import NodeWithScore
|
||||
from documents.models import Document
|
||||
```
|
||||
|
||||
(`Document` may already be imported -- check first.)
|
||||
|
||||
- [ ] **Step 2: Replace unspecced index, retriever, and node mocks**
|
||||
|
||||
```bash
|
||||
rg -n "mock_index = MagicMock\(\)\|mock_retriever = MagicMock\(\)\|mock_node[12] = MagicMock\(\)\|allowed_node = MagicMock\(\)\|private_node = MagicMock\(\)" src/paperless_ai/tests/test_ai_indexing.py
|
||||
```
|
||||
|
||||
Replace each:
|
||||
|
||||
```python
|
||||
# Before
|
||||
mock_index = MagicMock()
|
||||
mock_retriever = MagicMock()
|
||||
mock_node1 = MagicMock()
|
||||
mock_node2 = MagicMock()
|
||||
allowed_node = MagicMock()
|
||||
private_node = MagicMock()
|
||||
|
||||
# After
|
||||
mock_index = MagicMock(spec=VectorStoreIndex)
|
||||
mock_retriever = MagicMock(spec=VectorIndexRetriever)
|
||||
mock_node1 = MagicMock(spec=NodeWithScore)
|
||||
mock_node2 = MagicMock(spec=NodeWithScore)
|
||||
allowed_node = MagicMock(spec=NodeWithScore)
|
||||
private_node = MagicMock(spec=NodeWithScore)
|
||||
```
|
||||
|
||||
For `MagicMock(pk=1)` or `MagicMock(pk=2)` (document-like mocks):
|
||||
|
||||
```python
|
||||
# Before
|
||||
mock_filtered_docs = [MagicMock(pk=1), MagicMock(pk=2)]
|
||||
|
||||
# After
|
||||
mock_filtered_docs = [MagicMock(spec=Document, pk=1), MagicMock(spec=Document, pk=2)]
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Handle `nodes=[MagicMock()]` in patch call**
|
||||
|
||||
Find the `nodes=[MagicMock()]` usage (around L252) and replace:
|
||||
|
||||
```python
|
||||
# Before
|
||||
nodes=[MagicMock()],
|
||||
|
||||
# After
|
||||
nodes=[MagicMock(spec=NodeWithScore)],
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run tests**
|
||||
|
||||
```bash
|
||||
uv run pytest --override-ini="addopts=" src/paperless_ai/tests/test_ai_indexing.py -v
|
||||
```
|
||||
|
||||
Expected: all pass. If a `spec=NodeWithScore` mock fails because the test sets `.metadata` on it -- `NodeWithScore` has `.node` which has `.metadata`, so fix the test to use `.node.metadata` if that's the real path, OR use `spec=TextNode` from `llama_index.core.schema` for node mocks.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add src/paperless_ai/tests/test_ai_indexing.py
|
||||
git commit -m "Test: add spec= to llama-index mocks in test_ai_indexing"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3: `paperless_ai/tests/test_chat.py` -- AIClient, index, and query engine mocks
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `src/paperless_ai/tests/test_chat.py`
|
||||
|
||||
**Context:**
|
||||
`mock_client = MagicMock()` simulates `AIClient`. `mock_index = MagicMock()` simulates `VectorStoreIndex`. `mock_query_engine = MagicMock()` simulates `RetrieverQueryEngine`. `mock_response_stream = MagicMock()` simulates a streaming response.
|
||||
|
||||
- [ ] **Step 1: Add imports**
|
||||
|
||||
```python
|
||||
from llama_index.core import VectorStoreIndex
|
||||
from llama_index.core.query_engine import RetrieverQueryEngine
|
||||
from llama_index.core.schema import NodeWithScore
|
||||
from paperless_ai.client import AIClient
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Identify all unspecced mocks**
|
||||
|
||||
```bash
|
||||
rg -n "MagicMock()" src/paperless_ai/tests/test_chat.py
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Add spec to each mock**
|
||||
|
||||
```python
|
||||
# Before
|
||||
mock_client = MagicMock()
|
||||
mock_client.llm = MagicMock()
|
||||
mock_index = MagicMock()
|
||||
mock_response_stream = MagicMock()
|
||||
mock_query_engine = MagicMock()
|
||||
|
||||
# After
|
||||
mock_client = MagicMock(spec=AIClient)
|
||||
# mock_client.llm is set on the instance -- leave as-is or use spec=LLM if the type is known
|
||||
mock_index = MagicMock(spec=VectorStoreIndex)
|
||||
mock_response_stream = MagicMock() # streaming response object -- leave unspecced if no real class
|
||||
mock_query_engine = MagicMock(spec=RetrieverQueryEngine)
|
||||
```
|
||||
|
||||
For `doc = MagicMock()` (L38) and `doc1`, `doc2 = MagicMock(pk=...)` (L244):
|
||||
|
||||
```python
|
||||
from documents.models import Document # already present
|
||||
|
||||
doc = MagicMock(spec=Document)
|
||||
doc1 = MagicMock(spec=Document, pk=1)
|
||||
doc2 = MagicMock(spec=Document, pk=2)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: For `MagicMock(ids=[...], similarities=[...])` (L108-109)**
|
||||
|
||||
These mock the return value of `vector_store.query()`. Check what class that returns in llama-index:
|
||||
|
||||
```bash
|
||||
python3 -c "from llama_index.core.vector_stores.types import VectorStoreQueryResult; print(VectorStoreQueryResult.__doc__)"
|
||||
```
|
||||
|
||||
If a real class exists, use it:
|
||||
|
||||
```python
|
||||
from llama_index.core.vector_stores.types import VectorStoreQueryResult
|
||||
MagicMock(spec=VectorStoreQueryResult, ids=["0", "2"], similarities=[0.9, 0.8])
|
||||
```
|
||||
|
||||
If not, leave as-is with a comment:
|
||||
|
||||
```python
|
||||
MagicMock(ids=["0", "2"], similarities=[0.9, 0.8]) # no real class for vector store query result
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Run tests**
|
||||
|
||||
```bash
|
||||
uv run pytest --override-ini="addopts=" src/paperless_ai/tests/test_chat.py -v
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/paperless_ai/tests/test_chat.py
|
||||
git commit -m "Test: add spec= to AIClient and llama-index mocks in test_chat"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 4: `paperless_ai/tests/test_ai_classifier.py` and `test_embedding.py` -- Document/Tag/model mocks
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `src/paperless_ai/tests/test_ai_classifier.py`
|
||||
- Modify: `src/paperless_ai/tests/test_embedding.py`
|
||||
|
||||
**Context:**
|
||||
Both files mock Django model objects (Document, Tag, Correspondent, DocumentType, CustomField). Since these have real classes already imported in the test files, `spec=` is straightforward.
|
||||
|
||||
- [ ] **Step 1: Check existing imports in each file**
|
||||
|
||||
```bash
|
||||
rg "^from documents.models|^from paperless_ai" src/paperless_ai/tests/test_ai_classifier.py
|
||||
rg "^from documents.models|^from paperless_ai" src/paperless_ai/tests/test_embedding.py
|
||||
```
|
||||
|
||||
Note which model classes are already imported.
|
||||
|
||||
- [ ] **Step 2: Add any missing model imports**
|
||||
|
||||
Add what's missing from each file -- typically `Tag`, `Correspondent`, `DocumentType`, `CustomField`, `CustomFieldInstance`. Example for `test_ai_classifier.py`:
|
||||
|
||||
```python
|
||||
from documents.models import Correspondent
|
||||
from documents.models import CustomField
|
||||
from documents.models import DocumentType
|
||||
from documents.models import Tag
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Replace model mocks in test_ai_classifier.py**
|
||||
|
||||
```bash
|
||||
rg -n "MagicMock()" src/paperless_ai/tests/test_ai_classifier.py
|
||||
```
|
||||
|
||||
Pattern (lines ~26-64):
|
||||
|
||||
```python
|
||||
# Before
|
||||
tag1 = MagicMock()
|
||||
tag2 = MagicMock()
|
||||
doc1 = MagicMock()
|
||||
doc.tags.all = MagicMock(return_value=[tag1, tag2])
|
||||
document_type = MagicMock()
|
||||
correspondent = MagicMock()
|
||||
cf1 = MagicMock()
|
||||
cf2 = MagicMock()
|
||||
|
||||
# After
|
||||
tag1 = MagicMock(spec=Tag)
|
||||
tag2 = MagicMock(spec=Tag)
|
||||
doc1 = MagicMock(spec=Document)
|
||||
doc.tags.all = MagicMock(return_value=[tag1, tag2]) # leave -- it's a method replacement, not a class mock
|
||||
document_type = MagicMock(spec=DocumentType)
|
||||
correspondent = MagicMock(spec=Correspondent)
|
||||
cf1 = MagicMock(spec=CustomField)
|
||||
cf2 = MagicMock(spec=CustomField)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Replace model mocks in test_embedding.py**
|
||||
|
||||
Same pattern -- apply `spec=Tag`, `spec=Document`, `spec=Correspondent`, `spec=CustomField` to every bare `MagicMock()` that clearly simulates a model object.
|
||||
|
||||
For `MagicMock(note=...)` (L240-241), check the real type:
|
||||
|
||||
```bash
|
||||
rg "class.*Note\|note.*field" src/documents/models.py | head -5
|
||||
```
|
||||
|
||||
Use `spec=` if a real class exists; otherwise add a comment.
|
||||
|
||||
- [ ] **Step 5: Run tests for both files**
|
||||
|
||||
```bash
|
||||
uv run pytest --override-ini="addopts=" src/paperless_ai/tests/test_ai_classifier.py src/paperless_ai/tests/test_embedding.py -v
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/paperless_ai/tests/test_ai_classifier.py src/paperless_ai/tests/test_embedding.py
|
||||
git commit -m "Test: add spec= to Django model mocks in test_ai_classifier and test_embedding"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 5: `documents/tests/test_bulk_edit.py` -- pikepdf mocks
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `src/documents/tests/test_bulk_edit.py`
|
||||
|
||||
**Context:**
|
||||
`fake_pdf = mock.MagicMock()` simulates `pikepdf.Pdf` (opened via `pikepdf.open(...)`). `mock.Mock()` in `fake_pdf.pages = [mock.Mock()]` simulates `pikepdf.Page`. `sig = mock.Mock()` simulates a pikepdf form signature object.
|
||||
|
||||
- [ ] **Step 1: Add pikepdf imports**
|
||||
|
||||
```python
|
||||
import pikepdf
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Find all occurrences**
|
||||
|
||||
```bash
|
||||
rg -n "fake_pdf = mock.MagicMock\(\)\|mock\.Mock()\|sig = mock\.Mock\(\)" src/documents/tests/test_bulk_edit.py
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Replace PDF and page mocks**
|
||||
|
||||
```python
|
||||
# Before
|
||||
fake_pdf = mock.MagicMock()
|
||||
fake_pdf.pages = [mock.Mock()]
|
||||
|
||||
# After
|
||||
fake_pdf = mock.MagicMock(spec=pikepdf.Pdf)
|
||||
fake_pdf.pages = [mock.MagicMock(spec=pikepdf.Page)]
|
||||
```
|
||||
|
||||
For multi-page cases:
|
||||
|
||||
```python
|
||||
# Before
|
||||
fake_pdf.pages = [mock.Mock(), mock.Mock()]
|
||||
|
||||
# After
|
||||
fake_pdf.pages = [mock.MagicMock(spec=pikepdf.Page), mock.MagicMock(spec=pikepdf.Page)]
|
||||
```
|
||||
|
||||
For `output_pdf = mock.MagicMock()` (context where it's a new pikepdf.Pdf):
|
||||
|
||||
```python
|
||||
output_pdf = mock.MagicMock(spec=pikepdf.Pdf)
|
||||
```
|
||||
|
||||
For `sig = mock.Mock()` -- check what type signatures are in pikepdf:
|
||||
|
||||
```bash
|
||||
python3 -c "import pikepdf; print([x for x in dir(pikepdf) if 'sig' in x.lower() or 'form' in x.lower()])"
|
||||
```
|
||||
|
||||
If no clear class, leave `sig = mock.Mock()` and add a comment noting no spec was available.
|
||||
|
||||
- [ ] **Step 4: Run tests**
|
||||
|
||||
```bash
|
||||
uv run pytest --override-ini="addopts=" src/documents/tests/test_bulk_edit.py -v
|
||||
```
|
||||
|
||||
Expected: all pass. If `pikepdf.Pdf` spec breaks access to `.pdf_version` or `.pages`, verify those are real attributes:
|
||||
|
||||
```bash
|
||||
python3 -c "import pikepdf; pdf = pikepdf.new(); print(dir(pdf))" | tr ',' '\n' | grep -E "version|pages"
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add src/documents/tests/test_bulk_edit.py
|
||||
git commit -m "Test: add spec=pikepdf.Pdf/Page to PDF mocks in test_bulk_edit"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 6: `documents/tests/test_workflows.py` -- HTTP response mocks
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `src/documents/tests/test_workflows.py`
|
||||
|
||||
**Context:**
|
||||
`mock.Mock(json=mock.Mock(...), raise_for_status=mock.Mock())` simulates `requests.Response`. This is the most common pattern. The mock is assigned as the return value of `mock_post.return_value`.
|
||||
|
||||
- [ ] **Step 1: Add requests import**
|
||||
|
||||
```bash
|
||||
rg "^import requests\|^from requests" src/documents/tests/test_workflows.py
|
||||
```
|
||||
|
||||
If not present, add:
|
||||
|
||||
```python
|
||||
import requests
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Find all HTTP response mock patterns**
|
||||
|
||||
```bash
|
||||
rg -n "mock\.Mock\(json=\|mock\.Mock\(raise_for_status" src/documents/tests/test_workflows.py
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Replace with specced mocks**
|
||||
|
||||
```python
|
||||
# Before
|
||||
mock_post.return_value = mock.Mock(
|
||||
json=mock.Mock(return_value={"status": "ok"}),
|
||||
)
|
||||
|
||||
# After
|
||||
mock_response = mock.MagicMock(spec=requests.Response)
|
||||
mock_response.json.return_value = {"status": "ok"}
|
||||
mock_post.return_value = mock_response
|
||||
```
|
||||
|
||||
For mocks that also set `raise_for_status`:
|
||||
|
||||
```python
|
||||
# Before
|
||||
mock_post.return_value = mock.Mock(
|
||||
json=mock.Mock(return_value={"status": "ok"}),
|
||||
raise_for_status=mock.Mock(),
|
||||
)
|
||||
|
||||
# After
|
||||
mock_response = mock.MagicMock(spec=requests.Response)
|
||||
mock_response.json.return_value = {"status": "ok"}
|
||||
mock_response.raise_for_status.return_value = None
|
||||
mock_post.return_value = mock_response
|
||||
```
|
||||
|
||||
For the `request=mock.Mock(), response=mock.Mock()` pattern (exception construction), check the exception class:
|
||||
|
||||
```bash
|
||||
rg -n "request=mock\.Mock\(\)" src/documents/tests/test_workflows.py
|
||||
```
|
||||
|
||||
Use `spec=requests.PreparedRequest` and `spec=requests.Response` respectively if constructing a `requests.exceptions.HTTPError`.
|
||||
|
||||
- [ ] **Step 4: Run tests**
|
||||
|
||||
```bash
|
||||
uv run pytest --override-ini="addopts=" src/documents/tests/test_workflows.py -v
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add src/documents/tests/test_workflows.py
|
||||
git commit -m "Test: add spec=requests.Response to HTTP response mocks in test_workflows"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 7: `documents/tests/test_api_document_versions.py` -- QuerySet and async task mocks
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `src/documents/tests/test_api_document_versions.py`
|
||||
|
||||
**Context:**
|
||||
`mock_backend = mock.MagicMock()` (L268) simulates a search backend. `async_task = mock.Mock()` (L571, L606) simulates `PaperlessTask`. `queryset = mock.Mock()`, `fallback_queryset = mock.Mock()` simulate Django QuerySets.
|
||||
|
||||
- [ ] **Step 1: Check existing imports**
|
||||
|
||||
```bash
|
||||
rg "^from\|^import" src/documents/tests/test_api_document_versions.py | head -20
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Add missing imports**
|
||||
|
||||
```python
|
||||
from django.db.models import QuerySet
|
||||
from documents.models import PaperlessTask
|
||||
```
|
||||
|
||||
For the backend mock, check what class it simulates:
|
||||
|
||||
```bash
|
||||
rg "mock_backend" src/documents/tests/test_api_document_versions.py | head -5
|
||||
```
|
||||
|
||||
Look up the backend class (likely from `documents.search._backend`) and add its import if simple.
|
||||
|
||||
- [ ] **Step 3: Replace mocks**
|
||||
|
||||
```python
|
||||
# QuerySet mocks
|
||||
queryset = mock.MagicMock(spec=QuerySet)
|
||||
fallback_queryset = mock.MagicMock(spec=QuerySet)
|
||||
|
||||
# PaperlessTask mocks
|
||||
async_task = mock.MagicMock(spec=PaperlessTask)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run tests**
|
||||
|
||||
```bash
|
||||
uv run pytest --override-ini="addopts=" src/documents/tests/test_api_document_versions.py -v
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add src/documents/tests/test_api_document_versions.py
|
||||
git commit -m "Test: add spec= to QuerySet and PaperlessTask mocks in test_api_document_versions"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 8: `documents/tests/test_api_tasks.py` -- Celery result mocks
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `src/documents/tests/test_api_tasks.py`
|
||||
|
||||
**Context:**
|
||||
`mock_async_result = mock.Mock()` simulates Celery's `AsyncResult`. `mock_apply_async = mock.Mock()` simulates the return value of `.apply_async()` (also an `AsyncResult`).
|
||||
|
||||
- [ ] **Step 1: Add Celery import**
|
||||
|
||||
```python
|
||||
from celery.result import AsyncResult
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Find the mocks**
|
||||
|
||||
```bash
|
||||
rg -n "mock_async_result\|mock_apply_async" src/documents/tests/test_api_tasks.py | head -10
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Replace mocks**
|
||||
|
||||
```python
|
||||
# Before
|
||||
mock_async_result = mock.Mock()
|
||||
mock_apply_async = mock.Mock()
|
||||
|
||||
# After
|
||||
mock_async_result = mock.MagicMock(spec=AsyncResult)
|
||||
mock_apply_async = mock.MagicMock(spec=AsyncResult)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run tests**
|
||||
|
||||
```bash
|
||||
uv run pytest --override-ini="addopts=" src/documents/tests/test_api_tasks.py -v
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add src/documents/tests/test_api_tasks.py
|
||||
git commit -m "Test: add spec=AsyncResult to Celery mocks in test_api_tasks"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 9: Quick-win files (1-4 instances each)
|
||||
|
||||
These files each have a small number of high-value unspecced mocks. Tackle them together in one pass.
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `src/documents/tests/test_caching.py`
|
||||
- Modify: `src/documents/tests/test_api_status.py`
|
||||
- Modify: `src/paperless_ai/tests/test_client.py`
|
||||
- Modify: `src/paperless/tests/test_checks.py`
|
||||
- Modify: `src/paperless_mail/tests/test_mail.py`
|
||||
- Modify: `src/documents/tests/test_task_signals.py`
|
||||
|
||||
**For each file:**
|
||||
|
||||
- [ ] **Step 1: Find the unspecced mocks and identify what each simulates**
|
||||
|
||||
```bash
|
||||
for f in \
|
||||
src/documents/tests/test_caching.py \
|
||||
src/documents/tests/test_api_status.py \
|
||||
src/paperless_ai/tests/test_client.py \
|
||||
src/paperless/tests/test_checks.py \
|
||||
src/paperless_mail/tests/test_mail.py \
|
||||
src/documents/tests/test_task_signals.py; do
|
||||
echo "=== $f ==="; rg -n "MagicMock()\|= mock\.Mock()" "$f"; done
|
||||
```
|
||||
|
||||
- [ ] **Step 2: For each mock, determine spec from context**
|
||||
|
||||
Common patterns to resolve:
|
||||
|
||||
| Variable name | Likely spec |
|
||||
| ------------------- | --------------------------------------------------------------------- |
|
||||
| `mock_backend` | Check what module/class it's patching; often a search backend |
|
||||
| `mock_config` | `AIConfig` or `ApplicationConfiguration` |
|
||||
| `mock_llm_instance` | Check the patch target; likely an LLM class from `litellm` or similar |
|
||||
| `sociallogin` | `allauth.socialaccount.models.SocialLogin` |
|
||||
| `mock_connection` | `django.db.backends.base.base.BaseDatabaseWrapper` |
|
||||
|
||||
- [ ] **Step 3: Apply spec= to each identified mock in each file**
|
||||
|
||||
- [ ] **Step 4: Run tests for all modified files**
|
||||
|
||||
```bash
|
||||
uv run pytest --override-ini="addopts=" \
|
||||
src/documents/tests/test_caching.py \
|
||||
src/documents/tests/test_api_status.py \
|
||||
src/paperless_ai/tests/test_client.py \
|
||||
src/paperless/tests/test_checks.py \
|
||||
src/paperless_mail/tests/test_mail.py \
|
||||
src/documents/tests/test_task_signals.py \
|
||||
-v
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add \
|
||||
src/documents/tests/test_caching.py \
|
||||
src/documents/tests/test_api_status.py \
|
||||
src/paperless_ai/tests/test_client.py \
|
||||
src/paperless/tests/test_checks.py \
|
||||
src/paperless_mail/tests/test_mail.py \
|
||||
src/documents/tests/test_task_signals.py
|
||||
git commit -m "Test: add spec= to unspecced mocks in quick-win test files"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 10: `documents/tests/test_consumer.py` -- parser and status manager mocks
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `src/documents/tests/test_consumer.py`
|
||||
|
||||
**Context:**
|
||||
`m.return_value = MagicMock()` (appearing several times) replaces parser return values. `input_doc = ...`, `status_mgr=mock.Mock()` simulates consumer internal objects.
|
||||
|
||||
- [ ] **Step 1: Identify each unspecced mock and its target class**
|
||||
|
||||
```bash
|
||||
rg -n "MagicMock()\|= mock\.Mock()" src/documents/tests/test_consumer.py | head -20
|
||||
```
|
||||
|
||||
For `status_mgr`, check what class is expected:
|
||||
|
||||
```bash
|
||||
rg "status_mgr\|StatusManager\|ConsumerStatus" src/documents/consumer.py | head -10
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Add required imports and apply spec**
|
||||
|
||||
The status manager class, likely from `documents/consumer.py`:
|
||||
|
||||
```python
|
||||
from documents.consumer import ConsumerStatusManager # adjust to real class name
|
||||
```
|
||||
|
||||
Then:
|
||||
|
||||
```python
|
||||
status_mgr = mock.MagicMock(spec=ConsumerStatusManager)
|
||||
```
|
||||
|
||||
For parser return values, find the parser protocol/class:
|
||||
|
||||
```bash
|
||||
rg "class.*Parser\|ParserProtocol" src/paperless/parsers/__init__.py | head -5
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Apply spec to all identified mocks**
|
||||
|
||||
- [ ] **Step 4: Run tests**
|
||||
|
||||
```bash
|
||||
uv run pytest --override-ini="addopts=" src/documents/tests/test_consumer.py -v
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add src/documents/tests/test_consumer.py
|
||||
git commit -m "Test: add spec= to parser and status manager mocks in test_consumer"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 11: Final lint pass
|
||||
|
||||
- [ ] **Step 1: Run ruff over all modified test files**
|
||||
|
||||
```bash
|
||||
uv run ruff check src/paperless_ai/tests/ src/documents/tests/ src/paperless/tests/ src/paperless_mail/tests/
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Fix any unused imports introduced**
|
||||
|
||||
```bash
|
||||
uv run ruff check --fix src/paperless_ai/tests/ src/documents/tests/
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Run the full test suite for the affected apps**
|
||||
|
||||
```bash
|
||||
uv run pytest --override-ini="addopts=" src/paperless_ai/tests/ src/documents/tests/ -v --tb=short
|
||||
```
|
||||
|
||||
Expected: no regressions.
|
||||
|
||||
- [ ] **Step 4: Final commit**
|
||||
|
||||
```bash
|
||||
git add -u
|
||||
git commit -m "Test: lint cleanup after mock spec additions"
|
||||
```
|
||||
Reference in New Issue
Block a user