Added testcases for Tokens & TotalAmount Component #1629
Conversation
|
Fixed the snapshot testcases to compare HTML instead of vm of the component. |
| let wrapper; | ||
|
|
||
| describe('TokenAmount', () => { | ||
| beforeEach(() => { |
There was a problem hiding this comment.
There is no need to have a beforeEach and a wrapper variable deinfed out of scope, since you have only one test.
| mocks: { | ||
| $store: { | ||
| state: {}, | ||
| getters: {}, |
There was a problem hiding this comment.
You can import and use particular getters instead of mocking them, in order to cover all the cases in the TokenAmount component.
| @@ -21,6 +21,7 @@ module.exports = { | |||
| '<rootDir>/config/jest/setEnvVars.js', | |||
| '<rootDir>/config/jest/setup.js', | |||
| ], | |||
| collectCoverage: true, | |||
There was a problem hiding this comment.
I don't think we need to collect coverage on every run of test:unit command. I think it would be better to track coverage differently.
#1778
| /tests/e2e/screenshots | ||
| /tests/e2e/videos/ | ||
| /tests/e2e/screenshots/ | ||
| /coverage |
There was a problem hiding this comment.
I think all changes to this file should be undone.
| @@ -0,0 +1,34 @@ | |||
| import { shallowMount } from '@vue/test-utils'; | |||
| import Tokens from '../../src/popup/components/Tokens.vue'; | |||
There was a problem hiding this comment.
This test is not covering all the potential cases of Tokens component.
|
The PR should be properly rebased to consist only of meaningful commits, also commit names should be align with the template we are using in this repo. |
|
In general, I would skip matching the snapshot and would rather test the logic of the component, because:
|
|
@NitinRamnani There are few suggestions, would you work on fixing it? We can merge this once the PR is approved. |
I've added testcases for Tokens & TotalAmount Components which have increased code coverage of those components.
Before PR:
After PR: