search

viniciuskneves / berlinglish

Berlin in English Twitter BOT
https://twitter.com/berlinglish
MIT License
11 stars 2 forks source link

Add missing tests #10

Closed viniciuskneves closed 4 years ago

viniciuskneves commented 4 years ago

There are no tests so far 🙈 Use Jest or any other option to add tests to the project. Important: special attention to failure/retry and alerts!

leonardopavanrocha commented 4 years ago

@viniciuskneves I am interested in getting into testing. Would it be ok if I tried implementing these? Are we aiming for unit tests? Integration tests?

viniciuskneves commented 4 years ago

Yeah, of course!

Are you familiar with any Javascript testing framework?

I would like to try AVA here but if you're familiar with Jest feel free to use it.

Bear in mind that you could refactor any piece of code to help your tests.

leonardopavanrocha commented 4 years ago

I used jest already, but don`t have a lot of experience with it. I took a look in AVA and it seems an interesting choice.

My suggestion on how to tackle this issue would be to define the most important test scenarios and after that we will have a clearer vision of what we need to implement.

I will try to list in this issue the scenarios I think would be interesting to test. Since you have more experience in testing than me, I would ask you to review the scenarios and when we reach a solid plan, I can start implementing. What do you think about this?

viniciuskneves commented 4 years ago

Yeah, for sure!

Feel free to play with the tools if you want as well =]

leonardopavanrocha commented 4 years ago

@viniciuskneves I am already getting familiar with the basics of AVA and was able to set up the initial tests. However, while defining the main test scenarios, a lot of doubts regarding third party coupling appeared.

For example, the first test scenario I created was to assert that articles with one/multiple images would have the correct images. This tests our function, but since it has a third party dependency (Berlin news website), we can not fully trust it if the test fails.

The same occurs if testing twitter-related dependencies. Should we change our implementation to allow dependency injections such as mock-up objects? Or should we test the third party services as well?

Main advantages of the first approach would be the fact that our tests would be indeed isolated and would not depend on third party services. As for the second approach, the main advantages would be that if the tests fail, this could mean that any of the third party services changed their behavior.

Perhaps, we could even test both approaches at the same time. What are your thoughts on this subject?

viniciuskneves commented 4 years ago

Hey @leonardopavanrocha could you open PR with your ideas so we can even discuss there?

I mean, if you're focusing on unit testing the dependencies should be mocked. There is an issue with the current project that it is not split into modules which makes it hard to test. Ideally you should mock the fetch request (to berlin.de) and the Twitter API call as we won't test if it works and we're not currently dealing with error handling, if it fails, it throws and Lambda fires automatically again.

Honestly I don't know if AVA has some mock built in (You can try Sinon). You can also try different test runners if you are more familiar with. (Apparently some discussion here: https://github.com/avajs/ava/issues/1829, they mention Jest that has a good support for it, you can also try it if you want).

The first thing I would suggest is: Break it into a smaller module and test it isolated.