search

dzcode-io / dzcode.io

Website & mobile app for Algerian open-source community
https://dzcode.io
MIT License
113 stars 41 forks source link

Improve unit tests coverage for api code #287

Open ZibanPirate opened 3 years ago

ZibanPirate commented 3 years ago

Description

if you:

you will see that some directories are marked as red, which means test coverage is below 80%. you can also see the same results in codecov.io

In this task we will make sure that all dirs are marked green ✅.

Check List

  1. [ ] add unit test for ./api/src/logger/service.ts, similar to config/service.spec.ts
  2. [ ] add unit test for ./api/src/app/middlewares/error.ts.
  3. [ ] add unit test for ./api/src/app/middlewares/logger.ts.
  4. [ ] add unit test for ./api/src/app/middlewares/security.ts.
  5. [ ] add unit test for ./api/src/app/middlewares/docs.ts.

after #286 is done, more tasks will be added here...

Additional Comments

Please create new PR for each task.

Also, it would be great for transparency if you comment below which task you want to take up.

cherifGsoul commented 3 years ago

The logger is an wrapper around winston I guess it should be covered with contract test.

ZibanPirate commented 3 years ago

well @cherifGsoul , since in this case the provider is a npm package (winston) which follows the semantic versioning, its API will always stay the same, and in case where we upgrade to a major version, then Typescript will report errors if we have API incompatibility.

Now that you brought this up, i'm thinking the GithubService needs a contract test don't you think? as it's does REST calls to Github APIs, if so, then we can create new Task ticket about contract testing where we define where we place them, be it GithubService or any other place

cherifGsoul commented 3 years ago

Semantic versioning doesn't guarantee that the library API will remain the same forever, major version allows breaking changes.

I would split GithHub service in two classes, the service itself just uses collaborators like the module that ensures communication with GitHub web services, the latter is candidate for contract tests.

That's my two cents, best of luck.

cherifGsoul commented 3 years ago

and in case where we upgrade to a major version, then Typescript will report errors if we have API incompatibility.

The point of the wrapper is to be ready for the new API with minimum effort.