-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[Cosmos] migrate from using unittest #32076
Conversation
using this one as the test file
/azp run python - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
API change check APIView has identified API level changes in this PR and created following API reviews. |
/azp run python - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run python - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run python - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run python - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run python - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run python - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run python - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run python - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run python - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run python - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run python - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
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
/azp run python - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run python - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run python - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Requesting change on the changelog update, others are just nit comments and are not blockers, thanks @simorenoh
It seems as though our Cosmos async client tests are actually not running in the pipelines:
![image](https://private-user-images.githubusercontent.com/30335873/267787716-790d6f98-9bca-4b91-b411-046c845254f7.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2MjQwMTIsIm5iZiI6MTczOTYyMzcxMiwicGF0aCI6Ii8zMDMzNTg3My8yNjc3ODc3MTYtNzkwZDZmOTgtOWJjYS00YjkxLWI0MTEtMDQ2Yzg0NTI1NGY3LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDEyNDgzMlomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWZhMDY5ZGU5ZGQxODg3MmZkYjg3YWJmMjE4Njc0Y2E4ZWYwN2YyMzhmNTBhYjk5MGJiYzQxMjgwZDU4YzNmZWEmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.Hfifbxaq_a1bucr2_xJv7lHTPgU6yPNDWfuo1IE5uVE)
![image](https://private-user-images.githubusercontent.com/30335873/267787820-0a4bec8a-eb6d-404f-82be-02379c5e272d.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2MjQwMTIsIm5iZiI6MTczOTYyMzcxMiwicGF0aCI6Ii8zMDMzNTg3My8yNjc3ODc4MjAtMGE0YmVjOGEtZWI2ZC00MDRmLTgyYmUtMDIzNzljNWUyNzJkLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDEyNDgzMlomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTkwNjJlN2NiODA0ZjQxMzA0NzA0M2JhMDcxNzQwYTNlMGIwNWQ1M2M1MDFhM2FkM2MxYjFlNWUyYWY4YjIwYzAmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.yAnQNvSWo3fCSJ_Y5hJhNJeLOB9HuLuFdkFNEGjfilw)
This is due to the fact that using the unit test package does not support async, and as such we have been missing coverage for a while. Most other SDKs are actually not even using unit test anymore since the new test proxy does not use it, so moving away from it in general should be the move. This PR takes care of migrating all of our existing tests to use the default pytest package, which should allow our pipelines to run async tests. This PR also fixes some tests.
I'll also be renaming the tests to have an
async
suffix at the end since otherwise it makes the debugging a bit rougher (from images above, you wouldn't know which is which if the time was the same.)EDIT: This is what the pipeline looks like now, with both renaming and async tests actually running:
![image](https://private-user-images.githubusercontent.com/30335873/267843601-88926bdd-4c7d-4ef7-bb75-b1dc7925cb68.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2MjQwMTIsIm5iZiI6MTczOTYyMzcxMiwicGF0aCI6Ii8zMDMzNTg3My8yNjc4NDM2MDEtODg5MjZiZGQtNGM3ZC00ZWY3LWJiNzUtYjFkYzc5MjVjYjY4LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDEyNDgzMlomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTliOTM5N2IxODI4YWI3ZDAzODk2NmY2ZjdkOGRlZGM4YmIwYjJmOWNiYTRjMDZmMTY4N2MwNzg4OGFiMjk4MTUmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.Cq_rixOur_uaMGv-1jRAtaFxJCDfF9InLTMjFITrCzY)