-
-
Notifications
You must be signed in to change notification settings - Fork 758
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow app-dir parameter on the run() function #1271
Conversation
@HansBrende You recommended the solution, so feel free to review as well. 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@euri10 ping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good, the only small thing that may make it better is adding a test, I realize original PR that introduced the feature doesn't have one either so we're naked here and this will hit when working again on coverage anyway.
So approved, but feel free to add tests ;)
I will add, thanks. ✌️ |
How would you test this? I started with this thought: def test_app_dir(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None:
app_dir = tmp_path / "dir" / "app_dir"
app_file = app_dir / "main.py"
app_dir.mkdir(parents=True)
app_file.touch()
app_file.write_text(
dedent(
"""
async def app(scope, receive, send):
...
"""
)
)
run("main:app", app_dir=str(app_dir)) But the server will run, and we don't want that. We use After that I thought: "we'll, let's mock the Ideas? |
Hi! You can try to use the way I did for the cli tools https://github.com/encode/uvicorn/blob/master/tests/test_cli.py#L69-L78 Then you can check |
I'm going to add the test suggested by @humrochagf (thanks). But that's not fully what I want. I want to be able to know that |
* Allow app-dir parameter on the run() function * Add default value to pop() call * Update GitHub templates with new forms * Add test
Closes #1126