Skip to content
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

Test logging #964

Merged
merged 6 commits into from
May 29, 2021
Merged

Test logging #964

merged 6 commits into from
May 29, 2021

Conversation

euri10
Copy link
Member

@euri10 euri10 commented Feb 24, 2021

Alternative to #887 which gone stale, which was an alternative to #885

While #859 bug went unnoticed this test would have revealed it, nowhere we seem to test what the output of our default logging should be, this is a small addition to do it, it leverages @florimondmanca idea implementation in #887

Copy link
Sponsor Member

@Kludex Kludex left a comment

Choose a reason for hiding this comment

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

I've added some suggestions because black doesn't remove them by default. But LGTM.

tests/middleware/test_logging.py Outdated Show resolved Hide resolved
tests/middleware/test_logging.py Outdated Show resolved Hide resolved
tests/middleware/test_logging.py Outdated Show resolved Hide resolved
euri10 and others added 3 commits May 29, 2021 07:04
@euri10 euri10 merged commit 3e1e33e into encode:master May 29, 2021
@euri10 euri10 deleted the test_logging branch May 29, 2021 05:08
Kludex added a commit to sephioh/uvicorn that referenced this pull request Oct 29, 2022
* Remowrked test logging ala new test sauce

* Lint

* Update tests/middleware/test_logging.py

Co-authored-by: Marcelo Trylesinski <[email protected]>

* Update tests/middleware/test_logging.py

Co-authored-by: Marcelo Trylesinski <[email protected]>

* Update tests/middleware/test_logging.py

Co-authored-by: Marcelo Trylesinski <[email protected]>

Co-authored-by: Marcelo Trylesinski <[email protected]>
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