test(text-helper): add test for emojifyText, abbreviateNumber #568
Conversation
There was a problem hiding this comment.
Hey @ibotdotout thanks for your PR, can you please:
- Remove nested
describes emojifyTextshould be tested in another way, we should mock theemojipackage and then check if we calledemoji.emojifywith the right params. Right now, ifemojichanges one of its contents this test will break.- Please add
constto each constant/variable for example,expected = ...should beconst expected = ....
There was a problem hiding this comment.
Looks good! Just a couple small changes.
Also @alejandronanez – I talked about this in Gitter, but I think nested describes are fine in this context.
|
|
||
| describe('Text Helper', () => { | ||
| describe('emojifyText', () => { | ||
| it('should get correctly display :caffee: with emoji', () => { |
| describe('emojifyText', () => { | ||
| it('should get correctly display :caffee: with emoji', () => { | ||
| expected = 'I need more ☕️'; | ||
| result = emojifyText('I need more :coffee:'); |
| it('should get 1 when give 1', () => { | ||
| input = 1; | ||
| expected = 1; | ||
| result = abbreviateNumber(input); |
| it('should get 1k when give 1000', () => { | ||
| input = 1000; | ||
| expected = '1k'; | ||
| result = abbreviateNumber(input); |
| input = 1100; | ||
| expected = '1.1k'; | ||
| result = abbreviateNumber(input); | ||
| expect(result).toEqual(expected); |
| expected = '1.1k'; | ||
| result = abbreviateNumber(input); | ||
| expect(result).toEqual(expected); | ||
| }); |
There was a problem hiding this comment.
I would also add a test for something huge and random like 96234, which should equal 96.2k I think.
- mock emojifyText instead direct test output - insert const for variables - add huge and random 96234 -> 96.2k
|
Changes Unknown when pulling c54a9cc on ibotdotout:utility-tests into ** on gitpoint:master**. |
|
thank you @alejandronanez @andrewda for your reviewed. I improve
|
|
@ibotdotout coluld you add a new line before |
|
Changes Unknown when pulling 0a4ef41 on ibotdotout:utility-tests into ** on gitpoint:master**. |
| describe('emojifyText', () => { | ||
| it('should call correcly with text params', () => { | ||
| const emojify = sinon.spy(emoji, 'emojify'); | ||
| const input = 'I need more :coffee'; |
|
Changes Unknown when pulling c7af126 on ibotdotout:utility-tests into ** on gitpoint:master**. |
|
@alejandronanez already change to use jest for spy with |
|
Changes Unknown when pulling 90467ea on ibotdotout:utility-tests into ** on gitpoint:master**. |
| }); | ||
|
|
||
| describe('abbreviateNumber', () => { | ||
| it('should get 1 when give 1', () => { |
There was a problem hiding this comment.
Change this to: 'should return 1 when given 1'
| expect(result).toEqual(expected); | ||
| }); | ||
|
|
||
| it('should get 1k when give 1000', () => { |
| expect(result).toEqual(expected); | ||
| }); | ||
|
|
||
| it('should get 1.1k when give 1100', () => { |
| expect(result).toEqual(expected); | ||
| }); | ||
|
|
||
| it('should get 96.2k when give 96234', () => { |
| describe('Text Helper', () => { | ||
| describe('emojifyText', () => { | ||
| it('should call correcly with text params', () => { | ||
| const emojify = jest.spyOn(emoji, 'emojify'); |
|
Changes Unknown when pulling fd2b6a6 on ibotdotout:utility-tests into ** on gitpoint:master**. |
#518