Skip to content

chore: test django with WSGI - #163

Open
ryanking13 wants to merge 1 commit into
mainfrom
gyeongjae/django-sync
Open

chore: test django with WSGI#163
ryanking13 wants to merge 1 commit into
mainfrom
gyeongjae/django-sync

Conversation

@ryanking13

Copy link
Copy Markdown
Contributor

Run the same django test suite that I added in #147 with wsgi.py

@ask-bonk

ask-bonk Bot commented Jul 30, 2026

Copy link
Copy Markdown

I've completed my review. The PR is well-structured. The changes consolidate django-async into django, add a WSGI path with sync views, and correctly handle the asgi_only marker for tests that only apply to async. The wsgi.py fix for null-body statuses is correct per the Fetch spec. The functools.cache on get_suite_results correctly includes mode in its cache key since it's a parameter.

LGTM

github run

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed to /django/src/test_caching.py. It seems like github is not smart enough to detect renaming on this file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same. renamed to /django/src/test_streaming.py.



class CustomAsyncMiddleware:
class CustomMiddleware:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making this middleware work on both sync and async mode.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

django's async and sync views need to be registered differently. The only difference between urls.py and urls_sync.py is that whether the views are synchronous or asynchronous (async def vs def)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is splitted off to #162

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 this pull request may close these issues.

1 participant