add basic test suites for hooks folder #476
Conversation
There was a problem hiding this comment.
Hey @cescoferraro I left some comments. Let me know what you think.
A separate question: Could we move the test files under a __tests__ folder inside of the same hooks one? Thanks!
web/package.json
Outdated
| @@ -39,7 +39,8 @@ | |||
| "scripts": { | |||
| "start": "craco start", | |||
| "build": "craco build", | |||
| "test:unit": "craco test --watchAll=false", | |||
| "test:work": "craco test --watchAll=false --coverage ./src/hooks --collectCoverageFrom='src/hooks/**/*.(tsx|ts)'", | |||
There was a problem hiding this comment.
Did this leak into the commit?
web/package.json
Outdated
| @@ -78,7 +79,8 @@ | |||
| "jest": { | |||
| "transformIgnorePatterns": [ | |||
| "/node_modules/(?!antd|@ant-design|rc-.+?|@babel/runtime).+(js|jsx)$" | |||
| ] | |||
| ], | |||
| "testMatch": [ "<rootDir>/src/hooks/**/*.test.tsx" ] | |||
There was a problem hiding this comment.
Shouldn't this match everything not only hooks?
| export const useDAGChart = ( | ||
| spanMap: TSpanMap = {} | ||
| ): void | { | ||
| dag: Dag<{id: string; parentIds: string[]}, undefined>; |
There was a problem hiding this comment.
Should we encapsulate this into an interface?
web/src/hooks/usePolling.ts
Outdated
| useEffect(() => { | ||
| let interval: any = null; | ||
|
|
||
| console.log(1); |
There was a problem hiding this comment.
I guess these logs aren't supposed to be here right?
web/src/hooks/usePolling.test.tsx
Outdated
| const payload = {callback, isPolling: true, delay: 0}; | ||
| await renderHook(() => usePolling(payload)); | ||
|
|
||
| expect(callback).toBeCalledTimes(0); |
There was a problem hiding this comment.
Shouldn't the tests be more extensive to cover the different scenarios, like when the isPolling could change, etc?
There was a problem hiding this comment.
i am focusing on getting test suites present.
testing setInterval is a bit tricky.
I am aiming on getting coverage up on those 3 folders.
This PR creates basic test suites for the hooks placed at the hooks folder
Changes
Checklist