-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve unit tests coverage for api code #287
Comments
The logger is an wrapper around winston I guess it should be covered with contract test. |
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 |
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. |
The point of the wrapper is to be ready for the new API with minimum effort. |
Description
if you:
./api
folderyarn test:cov:watch
inside it./api/coverage/lcov-report/index.html
file in your browseryou 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
./api/src/logger/service.ts
, similar to config/service.spec.ts./api/src/app/middlewares/error.ts
../api/src/app/middlewares/logger.ts
../api/src/app/middlewares/security.ts
../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.
The text was updated successfully, but these errors were encountered: