bpo-32972: Document IsolatedAsyncioTestCase of unittest module by tirkarthi · Pull Request #15878 · python/cpython (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation27 Commits6 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

tirkarthi changed the title~~bpo32972: Document IsolatedAsyncioTestCase of unittest.async_case module~~ bpo-32972: Document IsolatedAsyncioTestCase of unittest.async_case module

Sep 10, 2019

asvetlov changed the title~~bpo-32972: Document IsolatedAsyncioTestCase of unittest.async_case module~~ bpo-32972: Document IsolatedAsyncioTestCase of unittest.async_case module

Sep 10, 2019

asvetlov changed the title~~bpo-32972: Document IsolatedAsyncioTestCase of unittest.async_case module~~ bpo-32972: Document IsolatedAsyncioTestCase of unittest.async_case module

Sep 10, 2019

Choose a reason for hiding this comment

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

Looks good, thank you.

Please fix a couple notes

Choose a reason for hiding this comment

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

import paths should be fixed before the merge

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Thanks Andrew, I didn't know about the import. It makes it more clean and user doesn't need to know about async_case module too. I have fixed the imports as per comments. I think this feature also deserves a better attention so I have added it to the What's new document under unittest section with a simple example. Thoughts?

I think this feature also deserves a better attention so I have added it to the What's new document under unittest section with a simple example. Thoughts?

Excellent idea! Please do

Choose a reason for hiding this comment

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

The example is fine. Please address my last comments.

Thanks Andrew, I have reworded ExpensiveAsyncConnection to AsyncConnection . I have removed the sync_connection example and moved the events assertion out to docs with a statement as suggested.

Yes, test discovery works better and friendly. I must admit I copied the test from async_case as example which was the reason and I just learned about TestCase instantiation while writing this example :) Your suggestion is better and changed it to test discovery.

Choose a reason for hiding this comment

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

Perfect!

Choose a reason for hiding this comment

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

there are two extra lines in example that should be dropped

Choose a reason for hiding this comment

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

Thank you very much!
Done well

Ah sorry, I removed the connection example and overlooked the setup and teardown which is not used. Thanks.

tirkarthi changed the title~~bpo-32972: Document IsolatedAsyncioTestCase of unittest.async_case module~~ bpo-32972: Document IsolatedAsyncioTestCase of unittest module

Sep 11, 2019

Thank you very much Andrew for all the guidance :)

@tirkarthi: Status check is done, and it's a success ✅ .

Thanks @tirkarthi for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Sep 11, 2019

…nGH-15878)

Document unittest.IsolatedAsyncioTestCase API
Add a simple example with respect to order of evaluation of setup and teardown calls.

https://bugs.python.org/issue32972

Automerge-Triggered-By: @asvetlov (cherry picked from commit 6a9fd66)

Co-authored-by: Xtreak tir.karthi@gmail.com

asvetlov pushed a commit that referenced this pull request

Sep 11, 2019

) (GH-15918)

Document unittest.IsolatedAsyncioTestCase API
Add a simple example with respect to order of evaluation of setup and teardown calls.

https://bugs.python.org/issue32972

Automerge-Triggered-By: @asvetlov (cherry picked from commit 6a9fd66)

Co-authored-by: Xtreak tir.karthi@gmail.com

DinoV pushed a commit to DinoV/cpython that referenced this pull request

Sep 12, 2019

websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request

Jul 20, 2020