Skip to content

chore: adds more fastapi threading tests - #192

Merged
dom96 merged 1 commit into
mainfrom
dominik/fastapi-threads-tests
Aug 11, 2026
Merged

chore: adds more fastapi threading tests#192
dom96 merged 1 commit into
mainfrom
dominik/fastapi-threads-tests

Conversation

@dom96

@dom96 dom96 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Just a few more test cases which end up creating a thread via starlette:

  • Sync routes
  • Streaming
  • File upload

Follow up to #190

@ask-bonk

ask-bonk Bot commented Aug 10, 2026

Copy link
Copy Markdown

I've reviewed all the changes thoroughly. The code is clean test infrastructure. The only minor observation is that post_json is defined but never used, and build_multipart has a subtle issue with binary content handling. However, the post_json helper is reasonable to add proactively for future tests, and the build_multipart function works correctly for the test cases at hand.

The tests are well-structured, cover the documented call-sites for anyio.to_thread.run_sync, and the assertions are meaningful. The python-multipart dependency addition is correct — it's required by FastAPI for file upload handling. No security, correctness, or compatibility issues.

LGTM

github run

@dom96
dom96 requested review from hoodmane and ryanking13 August 10, 2026 16:47

@ryanking13 ryanking13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

@dom96
dom96 force-pushed the dominik/fastapi-threads-tests branch from 5a1af3e to ac26791 Compare August 11, 2026 12:53
@dom96
dom96 merged commit 4c68fdf into main Aug 11, 2026
18 checks passed
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.

2 participants