Skip to content

Commit 6b8fad0

Browse files
committed
fix: address CI failures and code review comments
- Add permissions: contents: read to CI workflow (security best practice) - Exclude test_project_django5 from main pytest run to avoid name collision - Fix ruff E712: use truth check instead of == True - Add noqa: E402 for unavoidable Django setup imports - Configure testpaths to only run posthog/test in main CI
1 parent 2b4847e commit 6b8fad0

File tree

14 files changed

+157
-101
lines changed

14 files changed

+157
-101
lines changed

.claude/settings.local.json

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"permissions": {
3+
"allow": [
4+
"Bash(uv run mypy:*)",
5+
"WebFetch(domain:code.djangoproject.com)",
6+
"Bash(uv run pytest:*)",
7+
"Bash(gh issue view:*)",
8+
"Read(//Users/julian/workspace/posthog/**)",
9+
"Bash(flox activate:*)",
10+
"Bash(curl:*)",
11+
"Bash(pip show:*)",
12+
"Bash(uvx:*)",
13+
"Bash(uv sync:*)",
14+
"Bash(DJANGO_SETTINGS_MODULE=testdjango.settings uv --project /Users/julian/workspace/posthog-python/test_project_django5 run uvicorn testdjango.asgi:application --host 127.0.0.1 --port 8001)",
15+
"Read(//tmp/**)",
16+
"Bash(DJANGO_SETTINGS_MODULE=testdjango.settings uv run uvicorn:*)",
17+
"Bash(uv run:*)",
18+
"Bash(uv pip list:*)",
19+
"Bash(ruff check:*)"
20+
],
21+
"deny": [],
22+
"ask": []
23+
}
24+
}

.github/workflows/ci.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ name: CI
33
on:
44
- pull_request
55

6+
permissions:
7+
contents: read
8+
69
jobs:
710
code-quality:
811
name: Code quality checks

posthog/client.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import os
44
import sys
55
from datetime import datetime, timedelta
6-
from typing import Any, Callable, Dict, Optional, Union
6+
from typing import Any, Dict, Optional, Union
77
from typing_extensions import Unpack
88
from uuid import uuid4
99

@@ -60,7 +60,6 @@
6060
SizeLimitedDict,
6161
clean,
6262
guess_timezone,
63-
remove_trailing_slash,
6463
system_context,
6564
)
6665
from posthog.version import VERSION

posthog/test/ai/anthropic/test_anthropic.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import os
21
from unittest.mock import patch
32

43
import pytest

pyproject.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,3 +96,5 @@ version = { attr = "posthog.version.VERSION" }
9696
[tool.pytest.ini_options]
9797
asyncio_mode = "auto"
9898
asyncio_default_fixture_loop_scope = "function"
99+
testpaths = ["posthog/test"]
100+
norecursedirs = ["test_project_django5"]

test_project_django5/manage.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
#!/usr/bin/env python
22
"""Django's command-line utility for administrative tasks."""
3+
34
import os
45
import sys
56

67

78
def main():
89
"""Run administrative tasks."""
9-
os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'testdjango.settings')
10+
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "testdjango.settings")
1011
try:
1112
from django.core.management import execute_from_command_line
1213
except ImportError as exc:
@@ -18,5 +19,5 @@ def main():
1819
execute_from_command_line(sys.argv)
1920

2021

21-
if __name__ == '__main__':
22+
if __name__ == "__main__":
2223
main()

test_project_django5/pyproject.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,8 @@ dependencies = [
1212
"httpx~=0.28.1",
1313
]
1414

15+
[tool.uv]
16+
required-version = ">=0.5"
17+
1518
[tool.uv.sources]
1619
posthog = { path = "..", editable = true }

test_project_django5/test_exception_capture.py

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,17 @@
88
With process_exception(), Django calls this method to capture exceptions before
99
converting them to 500 responses.
1010
"""
11+
1112
import os
1213
import django
1314

1415
# Setup Django before importing anything else
1516
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "testdjango.settings")
1617
django.setup()
1718

18-
import pytest
19-
from httpx import AsyncClient, ASGITransport
20-
from django.core.asgi import get_asgi_application
19+
import pytest # noqa: E402
20+
from httpx import AsyncClient, ASGITransport # noqa: E402
21+
from django.core.asgi import get_asgi_application # noqa: E402
2122

2223

2324
@pytest.fixture(scope="session")
@@ -41,27 +42,31 @@ async def test_async_exception_is_captured(asgi_app):
4142

4243
def mock_capture(exception, **kwargs):
4344
"""Mock capture_exception to record calls."""
44-
captured.append({
45-
'exception': exception,
46-
'type': type(exception).__name__,
47-
'message': str(exception)
48-
})
45+
captured.append(
46+
{
47+
"exception": exception,
48+
"type": type(exception).__name__,
49+
"message": str(exception),
50+
}
51+
)
4952

5053
# Patch at the posthog module level where middleware imports from
51-
with patch('posthog.capture_exception', side_effect=mock_capture):
52-
async with AsyncClient(transport=ASGITransport(app=asgi_app), base_url="http://testserver") as ac:
54+
with patch("posthog.capture_exception", side_effect=mock_capture):
55+
async with AsyncClient(
56+
transport=ASGITransport(app=asgi_app), base_url="http://testserver"
57+
) as ac:
5358
response = await ac.get("/test/async-exception")
5459

5560
# Django returns 500
5661
assert response.status_code == 500
5762

5863
# CRITICAL: Verify PostHog captured the exception
59-
assert len(captured) > 0, f"Exception was NOT captured to PostHog!"
64+
assert len(captured) > 0, "Exception was NOT captured to PostHog!"
6065

6166
# Verify it's the right exception
6267
exception_data = captured[0]
63-
assert exception_data['type'] == 'ValueError'
64-
assert 'Test exception from Django 5 async view' in exception_data['message']
68+
assert exception_data["type"] == "ValueError"
69+
assert "Test exception from Django 5 async view" in exception_data["message"]
6570

6671

6772
@pytest.mark.asyncio
@@ -79,24 +84,28 @@ async def test_sync_exception_is_captured(asgi_app):
7984

8085
def mock_capture(exception, **kwargs):
8186
"""Mock capture_exception to record calls."""
82-
captured.append({
83-
'exception': exception,
84-
'type': type(exception).__name__,
85-
'message': str(exception)
86-
})
87+
captured.append(
88+
{
89+
"exception": exception,
90+
"type": type(exception).__name__,
91+
"message": str(exception),
92+
}
93+
)
8794

8895
# Patch at the posthog module level where middleware imports from
89-
with patch('posthog.capture_exception', side_effect=mock_capture):
90-
async with AsyncClient(transport=ASGITransport(app=asgi_app), base_url="http://testserver") as ac:
96+
with patch("posthog.capture_exception", side_effect=mock_capture):
97+
async with AsyncClient(
98+
transport=ASGITransport(app=asgi_app), base_url="http://testserver"
99+
) as ac:
91100
response = await ac.get("/test/sync-exception")
92101

93102
# Django returns 500
94103
assert response.status_code == 500
95104

96105
# CRITICAL: Verify PostHog captured the exception
97-
assert len(captured) > 0, f"Exception was NOT captured to PostHog!"
106+
assert len(captured) > 0, "Exception was NOT captured to PostHog!"
98107

99108
# Verify it's the right exception
100109
exception_data = captured[0]
101-
assert exception_data['type'] == 'ValueError'
102-
assert 'Test exception from Django 5 sync view' in exception_data['message']
110+
assert exception_data["type"] == "ValueError"
111+
assert "Test exception from Django 5 sync view" in exception_data["message"]

test_project_django5/test_middleware.py

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,17 @@
88
99
Tests run directly against the ASGI application without needing a server.
1010
"""
11+
1112
import os
1213
import django
1314

1415
# Setup Django before importing anything else
1516
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "testdjango.settings")
1617
django.setup()
1718

18-
import pytest
19-
from httpx import AsyncClient, ASGITransport
20-
from django.core.asgi import get_asgi_application
19+
import pytest # noqa: E402
20+
from httpx import AsyncClient, ASGITransport # noqa: E402
21+
from django.core.asgi import get_asgi_application # noqa: E402
2122

2223

2324
@pytest.fixture(scope="session")
@@ -38,7 +39,9 @@ async def test_async_user_access(asgi_app):
3839
trigger the lazy loading bug. This test verifies the middleware works
3940
in the common case.
4041
"""
41-
async with AsyncClient(transport=ASGITransport(app=asgi_app), base_url="http://testserver") as ac:
42+
async with AsyncClient(
43+
transport=ASGITransport(app=asgi_app), base_url="http://testserver"
44+
) as ac:
4245
response = await ac.get("/test/async-user")
4346

4447
assert response.status_code == 200
@@ -74,13 +77,13 @@ async def test_async_authenticated_user_access(asgi_app):
7477
@sync_to_async
7578
def create_or_get_user():
7679
user, created = User.objects.get_or_create(
77-
username='testuser',
80+
username="testuser",
7881
defaults={
79-
'email': 'test@example.com',
80-
}
82+
"email": "test@example.com",
83+
},
8184
)
8285
if created:
83-
user.set_password('testpass123')
86+
user.set_password("testpass123")
8487
user.save()
8588
return user
8689

@@ -91,7 +94,7 @@ def create_or_get_user():
9194
def create_session():
9295
client = Client()
9396
client.force_login(user)
94-
return client.cookies.get('sessionid')
97+
return client.cookies.get("sessionid")
9598

9699
session_cookie = await create_session()
97100

@@ -104,14 +107,14 @@ def create_session():
104107
async with AsyncClient(
105108
transport=ASGITransport(app=asgi_app),
106109
base_url="http://testserver",
107-
cookies={"sessionid": session_cookie.value}
110+
cookies={"sessionid": session_cookie.value},
108111
) as ac:
109112
response = await ac.get("/test/async-user")
110113

111114
assert response.status_code == 200
112115
data = response.json()
113116
assert data["status"] == "success"
114-
assert data["user_authenticated"] == True
117+
assert data["user_authenticated"]
115118

116119

117120
@pytest.mark.asyncio
@@ -121,7 +124,9 @@ async def test_sync_user_access(asgi_app):
121124
122125
This should always work regardless of middleware version.
123126
"""
124-
async with AsyncClient(transport=ASGITransport(app=asgi_app), base_url="http://testserver") as ac:
127+
async with AsyncClient(
128+
transport=ASGITransport(app=asgi_app), base_url="http://testserver"
129+
) as ac:
125130
response = await ac.get("/test/sync-user")
126131

127132
assert response.status_code == 200
@@ -139,7 +144,9 @@ async def test_async_exception_capture(asgi_app):
139144
causes a 500 response. See test_exception_capture.py for tests that verify
140145
actual exception capture to PostHog.
141146
"""
142-
async with AsyncClient(transport=ASGITransport(app=asgi_app), base_url="http://testserver") as ac:
147+
async with AsyncClient(
148+
transport=ASGITransport(app=asgi_app), base_url="http://testserver"
149+
) as ac:
143150
response = await ac.get("/test/async-exception")
144151

145152
# Django returns 500 for unhandled exceptions
@@ -154,7 +161,9 @@ async def test_sync_exception_capture(asgi_app):
154161
The middleware's process_exception() method captures view exceptions to PostHog.
155162
This test verifies the exception causes a 500 response.
156163
"""
157-
async with AsyncClient(transport=ASGITransport(app=asgi_app), base_url="http://testserver") as ac:
164+
async with AsyncClient(
165+
transport=ASGITransport(app=asgi_app), base_url="http://testserver"
166+
) as ac:
158167
response = await ac.get("/test/sync-exception")
159168

160169
# Django returns 500 for unhandled exceptions

test_project_django5/testdjango/asgi.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@
1111

1212
from django.core.asgi import get_asgi_application
1313

14-
os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'testdjango.settings')
14+
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "testdjango.settings")
1515

1616
application = get_asgi_application()

0 commit comments

Comments
 (0)