Cb 5753 u tests #2964
Cb 5753 u tests #2964
Conversation
devnaumov
requested review from
sergeyteleshev and
Wroud
and removed request for
sergeyteleshev
| clientActivityService.updateActivity(); | ||
| expect(clientActivityService.isActive).toBe(true); | ||
|
|
||
| jest.advanceTimersByTime(1000 * 60); |
There was a problem hiding this comment.
I think it is fine to use INACTIVE_PERIOD_TIME const here
| clientActivityService.updateActivity(); | ||
| expect(setTimeout).toHaveBeenCalledTimes(1); | ||
|
|
||
| clientActivityService.updateActivity(); | ||
| expect(clearTimeout).toHaveBeenCalledTimes(1); | ||
| expect(setTimeout).toHaveBeenCalledTimes(2); |
There was a problem hiding this comment.
would be nice to emulate running time here
There was a problem hiding this comment.
you can randomly tick in range from 0 to INACTIVE_PERIOD_TIME - 1
| clientActivityService.updateActivity(); | ||
| clientActivityService.resetActivity(); | ||
|
|
||
| expect(clientActivityService.isActive).toBe(false); | ||
| expect(clearTimeout).toHaveBeenCalled(); |
There was a problem hiding this comment.
same here for running time. I suggest to emulate cases like it is actually happening
| const onActiveStateChangeSpy = jest.spyOn(clientActivityService.onActiveStateChange, 'execute'); | ||
|
|
||
| clientActivityService.updateActivity(); | ||
| expect(onActiveStateChangeSpy).toHaveBeenCalledWith(true); | ||
|
|
||
| jest.advanceTimersByTime(1000 * 60); | ||
| expect(onActiveStateChangeSpy).toHaveBeenCalledWith(false); |
There was a problem hiding this comment.
dont forget to mock Executor also so the test is more moduled. cause if something goes wrong in Executor logic, this test is gonna be fallen as well
| it('should set supported languages and default to first supported language', () => { | ||
| const locales: ILocale[] = [ | ||
| { isoCode: 'de', name: 'German', nativeName: 'Deutsch' }, | ||
| { isoCode: 'es', name: 'Spanish', nativeName: 'Español' }, | ||
| ]; | ||
|
|
||
| service.setSupportedLanguages(locales); | ||
| expect(service.supportedLanguages.length).toBe(2); | ||
| expect(service.currentLanguage).toBe('de'); | ||
| }); |
There was a problem hiding this comment.
please, split this to 2 different tests:
- should set supported languages
- should set default supported language
|
|
||
| it('should throw error if there are no supported languages', () => { | ||
| service.supportedLanguages = []; | ||
| expect(() => service.currentLanguage).toThrowError('No language is available'); |
There was a problem hiding this comment.
this seems weird that getter can throw an error. I suggest to add TODO to refactor this logic somewhen in the LocalizationService.ts
No description provided.