21jake / nimble-scraper

4 stars 0 forks source link

[Feature] Missing test #36

Closed longnd closed 1 year ago

longnd commented 1 year ago

Issue

While some tests were added, e.g. the Scraper, the core business logic of the domains in services is not fully tested: the file upload, keyword parsing and firing the event, etc The goal is not to always get 100 test coverage (it's a nice and motivating goal for a team), and I also value the opinion of focusing on critical tests first, but in this case, the test coverage is not sufficient. It is not satisfying in the way that these parts are hard to test, and you could have demonstrated your unit testing skills better.

May I know the reason is that you did not have enough time or there were difficulties when writing the tests?

Expected

The core business logic is fully tested.

21jake commented 1 year ago

The main reason that the FileService is under tested is that I haven't found a way to mock file upload with Jest/Multer. I thought it's unwise to spend an unfair amount of time for a relatively simple logic, hence decided to spend efforts on testing other parts of the code and leave this file upload logic for last.

I will spend more time trying to cover the rest of the code base.