added tests for button#85
added tests for button#85erensunerr wants to merge 1 commit intoutmgdsc:dev/gdsc-open-source-2022from
Conversation
shubhbapna
left a comment
There was a problem hiding this comment.
LGTM, I prefer a singular test directory instead of per component but I haven't worked a lot with client testing. @hiimchrislim thoughts?
|
Oh wait i just noticed this, the base branch is incorrect please change it to dev/open-source one pls |
| import userEvent from "@testing-library/user-event"; | ||
|
|
||
| test("displays value as button label", () => { | ||
| // Arrange |
There was a problem hiding this comment.
Lets remove the // Act, // Arrange and // Assert comments.
There was a problem hiding this comment.
Additionally, we can remove a lot of the whitespaces/blank lines.
| "eslint-plugin-promise": "^5.1.0", | ||
| "eslint-plugin-react": "^7.26.1", | ||
| "husky": "^7.0.2", | ||
| "jest": "^26.6.0", |
There was a problem hiding this comment.
Since this is the first set of tests that we have, can you also add the workflow for running the tests as well?
Yea, per component / file should be fine to have for client testing. I prefer it per component since it'll be a unit test |
hiimchrislim
left a comment
There was a problem hiding this comment.
Overall, it looks pretty good. I just left a few comments to take a look at.
Also take a look at Shubh's comments.
|
|
||
| test("displays value as button label", () => { | ||
| // Arrange | ||
| const value = "Some Label"; |
There was a problem hiding this comment.
Lets make the props for the button globally accessible (throughout the file) so we don't have to keep defining it over and over again. (i.e. callbackMock, value and className should ideally only be defined once and then reused wherever it needs to be reused)
| import userEvent from "@testing-library/user-event"; | ||
|
|
||
| test("displays value as button label", () => { | ||
| // Arrange |
There was a problem hiding this comment.
Additionally, we can remove a lot of the whitespaces/blank lines.
I wrote tests for the button component that tests all the props and functionality required.