Count feature, related test and import optimisation #18
Conversation
|
Hi, @rednaks, Can you please review the PR? and please let me know if you think I have made any mistake. |
|
Thank you, looks good for me, can you just update the README file for the count and none ? Thanks ! |
|
Sure! Can I add some tests for some other features which are active but the test isn't available? |
|
Hi, @rednaks, I have updated the README file and added some tests also. Please review this and let me know if you want me to add something else. |
|
Hi, @rednaks, Can you please review the changes and let me know if you want any more here? I want to contribute more. |
tests/test_django_async_orm.py
Outdated
|
|
||
| await all_created.async_delete() | ||
| all_after_delete = await TestModel.objects.async_all() | ||
| self.assertEqual(len(all_after_delete), 0) | ||
| count = await all_created.async_count() |
There was a problem hiding this comment.
you should use all_after_delete rather than all_created
tests/test_django_async_orm.py
Outdated
| qs = await qs.async_first() | ||
| self.assertEqual(qs.name, "setup 2") | ||
|
|
||
| await test_async_order_by_ascending(self) |
There was a problem hiding this comment.
would be better to split then into two different tests, always one assert per unit test
There was a problem hiding this comment.
I have divided this into 2 different tests. Thanks for your advice, I'm trying to learn the best coding practices from developers like you.
tests/test_django_async_orm.py
Outdated
| self.assertEqual(result.name, 'test') | ||
|
|
||
| @tag('ci') | ||
| async def test_async_bulk_create(self): |
There was a problem hiding this comment.
why did you remove this test ?! we need to test async_bulk_create !!
README.md
Outdated
| @@ -130,8 +132,6 @@ You can still call `Model.object.async_raw()` but you will be unable to access t | |||
| | methods | supported | comments | | |||
| |----------------------------|------------|----------| | |||
| | `Model.async_save` | ❌ | | | |||
| | `Model.async_update` | ❌ | | | |||
| | `Model.async_delete` | ❌ | | | |||
|
|
||
| @tag('dev') | ||
| async def test_async_bulk_update(self): |
|
Thank you for your PR, but I don't know why you removed so many tests ... please revert them and make the changes requested in the comments. Thank you |
|
Hi, @rednaks, thanks for your valuable feedback. I have changed the test file according to your comments but there is no change in the readme file. Both the features are available in the module that's why I removed them from the readme file. Thanks again for your valuable feedback. |
|
Hi, @rednaks did you check this? any feedback? |
Hi, @rednaks,
I have added the count feature and test for the same, and with that, I have also changed some other tests accordingly and removed unused imports from the file. Please have a look and give your feedback.