Grommash9 / tg_chat_mate

https://grommash9.github.io/tg_chat_mate/
MIT License
5 stars 6 forks source link

Issue 173: Add unittest travis stage for PR checks #175

Closed Jonatan-Chaverri closed 9 months ago

Jonatan-Chaverri commented 9 months ago

This PR is solving part of issue https://github.com/Grommash9/tg_chat_mate/issues/173 . It just a start. I wil be posting more unittest in the future, but at least the unittest stage is setup now.

Grommash9 commented 9 months ago

Pr looks great btw :)

Jonatan-Chaverri commented 9 months ago

@Grommash9 I added this tests as an example https://github.com/Grommash9/tg_chat_mate/pull/175/files#diff-903bde27d96d3f9d038e8278a4284c1ac43f33e1574c9157d77a1df91198ea0eR37 is that what you mean? You can try to follow similar logic for other tests you want to add. Or feel free to contact me if you need more help.

If you or others want to write unittest as well, we can divide the tests per file in /routes folder, so we don't end up doing duplicate work. Maybe just post a message in telegram indicating which route file you are going to write tests for.

Grommash9 commented 9 months ago

Maybe I am not fully understand it really but I wanted to have something like these

image

But now I checked your code again and it's something new for me but looks very good

Should we add more checks like what fuctions was called and how much times? It would be great to have it in these MR as example for future tests

image

Yes it's good idea to divide endpoints between contributors, I think I will create issues for it and make notification in chat, and there is a lot of people who are really new in testing so maybe we will have even a call about it. I am not an expert too, so maybe you will have a time to join us ? :D

Thanks a lot for you contributions into these, you are adding amazing code and it will help a lot

I am happy to merge it after you add called and call_count checks if you are happy to but we can talk about it as well

Jonatan-Chaverri commented 9 months ago

@Grommash9 I think I got what you mean. Changed the test in order to use a test client instead of calling the function directly. You are right, its better this way.

Also added the call_count assert (I only have one mock in the test so calling call_count and called is redundant, I think just call_count is enough, but let me know your thoughts).

Grommash9 commented 9 months ago

image

So my main idea is:

We already have web app with routes and it's running by aiohttp basically aiogram library just adding some route for telegram to make us able to getting updates from it

We can modify misc.py and run aiohttp client without mocking it?

So we will be able to run real one routes, and will be able to call them, we will mock out the database and bot parts and check called and call_count

We will be able to pass wrong data to check status codes related to missing params as example

image image

What are you think about it? I believe it will add much more logic and make our tests much more useful, because we will know what endpoints are exists, we are using right method, and calling right functions inside

Jonatan-Chaverri commented 9 months ago

@Grommash9 I changed the aiohttp app by the app on misc.py in that way the routes are not going to be added manually which is an improvement yes. I'm not sure I fully understand your idea.

My opinion is that when working with unittest issolation is better. The purpose is to test your code and the code thats being introduce, not the whole application at once. Thats why its called "unit" cause you want to test "units". If you want to test functions inside "misc.py" then you can have a separate test for that, not trying to test it in every unittest thats being run.

There are other types of testing, like "functional tests" which purpose is to test several components at once, functionalities and relations. Typically for that you will have like a cron-job running at certain time intervals during the day, calling a real version of the project (like an image from master), making http requests and checking responses. Similar to what you have right now.

Grommash9 commented 9 months ago

It looks very good now, let me run it locally and play around with it.

Thanks a lot for contributing

Grommash9 commented 9 months ago

Tested it locally and that what I wish to have, yes, it's totally great!