hyochan / dooboo-ui-legacy

React Native UI Components with react-hook (web, ios, android)
MIT License
140 stars 72 forks source link

[DatePicker] Date, locale mocking error #337

Closed JeffGuKang closed 3 years ago

JeffGuKang commented 4 years ago

Describe the bug

Tests always fail due to date issues.

To Reproduce Steps to reproduce the behavior:

npx jest DatePicker --coverage

Expected behavior

Mock date to specific day for test.

Screenshots

-----------------------|---------|----------|---------|---------|-------------------
File                   | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
-----------------------|---------|----------|---------|---------|-------------------
All files              |    91.3 |       75 |      75 |   91.46 |                   
 DatePicker            |   88.89 |    72.41 |      60 |    87.5 |                   
  DateInput.tsx        |   94.87 |    77.78 |   83.33 |   94.29 | 106,132           
  PickerCalendar.tsx   |   90.91 |     62.5 |      60 |   90.48 | 49,87             
  index.tsx            |      75 |    66.67 |      25 |   68.75 | 35-37,48-59       
 DatePicker/Calendar   |    93.2 |    78.26 |   88.24 |   94.57 |                   
  Calendar.tsx         |   85.11 |       50 |   77.78 |   87.18 | 78,100-103        
  CalendarDate.tsx     |     100 |    93.75 |     100 |     100 | 44                
  CalendarMonth.tsx    |     100 |       75 |     100 |     100 | 65                
  CalendarWeekDays.tsx |     100 |      100 |     100 |     100 |                   
-----------------------|---------|----------|---------|---------|-------------------
Snapshot Summary
 › 6 snapshots failed from 1 test suite. Inspect your code changes or re-run jest with `-u` to update them.

Desktop (please complete the following information):

X

Smartphone (please complete the following information):

X

Additional context

X

JeffGuKang commented 4 years ago

I think it will be solved by mocking as like

  jest
      .spyOn(Date, 'now')
      .mockImplementation(() => new Date('2020-09-01').valueOf())
oh3vci commented 4 years ago

Because date values are used a lot in test cases, I set a standard date instead of using spyOn.

JeffGuKang commented 4 years ago

@oh3vci It is not solved yet. Snapshots change everyday.

JeffGuKang commented 4 years ago

spyOn is not working with Date.

Related issues:

I finally mocked global.Date through

const standardDate = new Date('2020-09-01');
const _Date = Date;

// @ts-ignore
global.Date = jest.fn(() => standardDate);
global.Date.UTC = _Date.UTC;
global.Date.parse = _Date.parse;
global.Date.now = _Date.now;

But I couldn't mocking toLocaleString.

Already tried

  1. jest.spyOn
    
    const localeOption = {
    year: '2-digit',
    month: '2-digit',
    day: '2-digit',
    };

const dateSpy = jest.spyOn(global.Date.prototype, 'toLocaleString') .mockImplementation(() => new Date().toLocaleString('UTC', localeOption));


2. [installing full-icu for node 12 and earlier](https://stackoverflow.com/a/56624712/515932): not working
JeffGuKang commented 4 years ago

I think it's good to exclude snapshots from the Datepicker test.

0916dhkim commented 4 years ago

@JeffGuKang How about we add today as a props for DatePicker? That would eliminate the need for mocking global.Date, and also allow us to use snapshot testing.

JeffGuKang commented 4 years ago

@JeffGuKang How about we add today as a props for DatePicker? That would eliminate the need for mocking global.Date, and also allow us to use snapshot testing.

But I couldn't control locale in dooboo-ui node environment for testing. It seems to be dependent on the user's OS.

For example,

https://github.com/dooboolab/dooboo-ui/blob/29d45036764e2ab79566b13e6bb99a35d291d78b/main/DatePicker/Calendar/CalendarWeekDays.tsx#L37-L62

Weekdays would be 'Mon', 'Tue'... in en-EN. But in Korean OS, this appears to be '월', '화'... And we cannot control locale manually as I said https://github.com/dooboolab/dooboo-ui/issues/337#issuecomment-692028016.

JeffGuKang commented 4 years ago

We can except validation from specific area of snapshots through matcher. It can be the one of the solutions.

0916dhkim commented 4 years ago

@JeffGuKang I suggest we use react-intl package for internationalization instead of toLocaleString and add today props to DatePicker component. react-intl (from FormatJS) allows developers to wrap components with IntlProvider, so we can provide { locale: 'en' } for testing and provide user-specific locale for production.

Here's the relevant documentation : https://formatjs.io/docs/guides/testing#jest Excerpt:

import React from 'react'
import renderer from 'react-test-renderer'
import {IntlProvider} from 'react-intl'

const createComponentWithIntl = (children, props = {locale: 'en'}) => {
  return renderer.create(<IntlProvider {...props}>{children}</IntlProvider>)
}

export default createComponentWithIntl
import React from 'react'
import createComponentWithIntl from '../../utils/createComponentWithIntl'
import AppMain from '../AppMain'

test('app main should be rendered', () => {
  const component = createComponentWithIntl(<AppMain />)

  let tree = component.toJSON()

  expect(tree).toMatchSnapshot()

  tree.props.onClick()

  tree = component.toJSON()

  expect(tree).toMatchSnapshot()
})
oh3vci commented 4 years ago

As adding locale prop to component, I implemented already this approach without react-intl. Can you check this? #347

0916dhkim commented 4 years ago

@oh3vci It looks good to me. So I guess we just need to test DatePicker with locale and today props defined. BTW do we have any timezone related issue with DatePicker snapshot testing?

oh3vci commented 4 years ago

I think timezone issue is already solved problem, because I set en to the value of the locale prop that is the locale parameter of toLocaleString method in test code. So the test snapshot will no longer be affected by the OS.

JeffGuKang commented 4 years ago

@oh3vci Did you try locale props with 'kr-KR' or other timezones when you test? Did it work as like '월', '화'...?

0916dhkim commented 4 years ago

@JeffGuKang I tried setting locale props to ko-KR in storybook. Korean characters show up correctly on web, but it does not work on Android. The same thing happens for locale="es-ES".

oh3vci commented 4 years ago

@oh3vci Did you try locale props with 'kr-KR' or other timezones when you test? Did it work as like '월', '화'...?

Isn't it 'ko-KR', not 'kr-KR'?

@JeffGuKang I tried setting locale props to ko-KR in storybook. Korean characters show up correctly on web, but it does not work on Android. The same thing happens for locale="es-ES".

I guess it seems to be another bug of android or storybook. it works well in iOS.

JeffGuKang commented 4 years ago

@oh3vci How about testing environment? Testing is not work as expectation with locale control or mocking.

0916dhkim commented 4 years ago

@JeffGuKang locale props works inside jest. I tested this first,

<DatePicker selectedDate={standardDate} locale={'ko-KR'} />

then this,

<DatePicker selectedDate={standardDate} locale={'en-US'} />

Here's the snapshot diff:

- Snapshot  - 7
+ Received  + 7

@@ -247,11 +247,11 @@
                              "color": "#ff424c",
                            },
                          ]
                        }
                      >
-                       일
+                       S
                      </Text>
                    </View>
                    <View
                      style={
                        Array [
@@ -279,11 +279,11 @@
                              "color": "#565656",
                            },
                          ]
                        }
                      >
-                       월
+                       M
                      </Text>
                    </View>
                    <View
                      style={
                        Array [
@@ -311,11 +311,11 @@
                              "color": "#565656",
                            },
                          ]
                        }
                      >
-                       화
+                       T
                      </Text>
                    </View>
                    <View
                      style={
                        Array [
@@ -343,11 +343,11 @@
                              "color": "#565656",
                            },
                          ]
                        }
                      >
-                       수
+                       W
                      </Text>
                    </View>
                    <View
                      style={
                        Array [
@@ -375,11 +375,11 @@
                              "color": "#565656",
                            },
                          ]
                        }
                      >
-                       목
+                       T
                      </Text>
                    </View>
                    <View
                      style={
                        Array [
@@ -407,11 +407,11 @@
                              "color": "#565656",
                            },
                          ]
                        }
                      >
-                       금
+                       F
                      </Text>
                    </View>
                    <View
                      style={
                        Array [
@@ -439,11 +439,11 @@
                              "color": "#565656",
                            },
                          ]
                        }
                      >
-                       토
+                       S
                      </Text>
                    </View>
                  </View>
                  <ScrollView
                    data={
JeffGuKang commented 4 years ago

@0916dhkim Cool. We can use snapshot test again for DatePicker.

oh3vci commented 4 years ago

@oh3vci Did you try locale props with 'kr-KR' or other timezones when you test? Did it work as like '월', '화'...?

Isn't it 'ko-KR', not 'kr-KR'?

@JeffGuKang I tried setting locale props to ko-KR in storybook. Korean characters show up correctly on web, but it does not work on Android. The same thing happens for locale="es-ES".

I guess it seems to be another bug of android or storybook. it works well in iOS.

Maybe, this is related issue in android.

JeffGuKang commented 3 years ago

@oh3vci It is working well now with #351. 👍