Skip to content

Commit

Permalink
fix: fix broken datetime picker tests
Browse files Browse the repository at this point in the history
  • Loading branch information
tthvo committed Sep 10, 2024
1 parent 0477781 commit db007da
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 165 deletions.
7 changes: 6 additions & 1 deletion TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ In order to render the component under test into its HTML DOM representation and

* To search for a localized text, for example, with `getByText`, use `testTranslate` function from `@test/Common.tsx` to return a translated text. Tests are configued to use `en` locale as default.

* If any of the test codes cause the changes in React states (e.g. open/close a action menu or modal), wrap them in [`act`](https://testing-library.com/docs/react-testing-library/api/#act) function, imported from `@testing-library/react`. The `act` function allows flushing all pending state changes.

## MOCKING

### Overview
Expand Down Expand Up @@ -83,6 +85,8 @@ Where the `-u` flag tells Jest to update the snapshot and the `-t` flag specifie

* Some PatternFly components use random, dynamic strings as `ids` which will then be displayed as elements in the rendered React virtual DOM. These strings change upon every render, causing snapshots to fail even though the component under test is still functionally the same. This can be remedied by supplying [custom `ids` as props](https://github.com/patternfly/patternfly-react/issues/3518) to the culprit PatternFly child components inside the source file of the component under test.

* Since `react-test-renderer` does not support the use of `ref` (see [reference](https://github.com/facebook/react/issues/7740)), the tests might fail if any third-party codes use `ref` unsafely (without checking `null`). To workaround that, use [`createNodeMock`](https://legacy.reactjs.org/docs/test-renderer.html#ideas) to construct a mock `ref` when calling `renderSnapshot`.


## INTEGRATION TESTING

Expand All @@ -107,6 +111,7 @@ This will automatically start a Mirage dev server, run the integration tests on

### Tips
* Running the integration tests will open a Firefox browser and simulate any actions that you instruct the browser to perform. That means we must first navigate to the local Cryostat Web page, before performing any useful testing.

* In our `beforeAll` jest declaration, we setup our web driver with the [default configurations](src/itest/util.ts)), and then use that driver to create our first **Page Object**. A Page Object is an abstraction that acts as an interface to your web pages. For more info on the **Page Object Model** in Selenium, see https://www.selenium.dev/documentation/test_practices/encouraged/page_object_models/.
```ts
beforeAll(async function () {
Expand Down Expand Up @@ -135,4 +140,4 @@ Add more methods to each PO, to test more actions. The point is, we want to abst
In the code, we first tell the driver to wait, until an element is located by the css selector ('button[data-action="skip"]'), and assign it to a variable. If not found, we assign null. Then if the variable is non-null, we click it. To find a good query to use, it is recommended to use the [Selenium IDE](https://addons.mozilla.org/en-CA/firefox/addon/selenium-ide/) extension on your browser. The extension allows you to easily see queries that can be used to select an element you want.
* Integration tests are found in [src/itest](src/itest).
* All code is asynchronous which entails the use of the `async/await` pattern.
* Follow the Selenium testing practices when writing integration tests: https://www.selenium.dev/documentation/test_practices/.
* Follow the Selenium testing practices when writing integration tests: https://www.selenium.dev/documentation/test_practices/.
1 change: 1 addition & 0 deletions locales/en/public.json
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,7 @@
},
"TimezonePicker": {
"ARIA_LABELS": {
"MENU_TOGGLE": "timezone-select-toggle",
"SELECT": "Select a timezone",
"TYPE_AHEAD": "Search a timezone"
},
Expand Down
4 changes: 3 additions & 1 deletion src/app/DateTimePicker/DateTimePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ export const DateTimePicker: React.FC<DateTimePickerProps> = ({
<Level hasGutter>
<LevelItem>
<CalendarMonth
id="calendar-month-view"
className="datetime-picker__calendar"
isDateFocused
locale={dayjs.locale()}
Expand All @@ -128,6 +129,7 @@ export const DateTimePicker: React.FC<DateTimePickerProps> = ({
<Divider orientation={{ default: 'vertical' }}></Divider>
<LevelItem>
<TimePicker
id="time-picker"
selected={{
hour24: dayjs(datetime).hour(), // 24hr format
minute: dayjs(datetime).minute(),
Expand All @@ -151,7 +153,7 @@ export const DateTimePicker: React.FC<DateTimePickerProps> = ({
/>
</FormGroup>
<FormGroup label={t('TIMEZONE', { ns: 'common' })}>
<TimezonePicker onTimezoneChange={setTimezone} selected={timezone} />
<TimezonePicker id="timezone-picker" onTimezoneChange={setTimezone} selected={timezone} />
</FormGroup>
<ActionGroup style={{ marginTop: 0 }}>
<Button variant="primary" onClick={handleSubmit}>
Expand Down
4 changes: 3 additions & 1 deletion src/app/DateTimePicker/TimePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import * as React from 'react';
import { useTranslation } from 'react-i18next';

export interface TimePickerProps {
id?: string;
selected: {
// controlled
hour24: number; // In 24h format
Expand All @@ -57,6 +58,7 @@ export const TimePicker: React.FC<TimePickerProps> = ({
onSecondSelect,
onMeridiemSelect,
selected,
id,
}) => {
const { t } = useTranslation();
const [is24h, setIs24h] = React.useState(true);
Expand All @@ -73,7 +75,7 @@ export const TimePicker: React.FC<TimePickerProps> = ({

return (
<>
<Panel className="datetime-picker">
<Panel className="datetime-picker" id={id}>
<PanelHeader>
<ToggleGroup>
<ToggleGroupItem
Expand Down
6 changes: 5 additions & 1 deletion src/app/DateTimePicker/TimezonePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,14 @@ const DEFAULT_NUM_OPTIONS = 10;
const OPTION_INCREMENT = 15;

export interface TimezonePickerProps {
id?: string;
isFlipEnabled?: boolean;
onTimezoneChange?: (timezone: Timezone) => void;
selected: Timezone;
}

export const TimezonePicker: React.FC<TimezonePickerProps> = ({
id,
isFlipEnabled = true,
selected,
onTimezoneChange = (_) => undefined,
Expand Down Expand Up @@ -115,6 +117,8 @@ export const TimezonePicker: React.FC<TimezonePickerProps> = ({
const toggle = React.useCallback(
(toggleRef: React.Ref<MenuToggleElement>) => (
<MenuToggle
aria-label={t('TimezonePicker.ARIA_LABELS.MENU_TOGGLE')}
id={id}
ref={toggleRef}
onClick={onToggle}
isExpanded={isTimezoneOpen}
Expand All @@ -128,7 +132,7 @@ export const TimezonePicker: React.FC<TimezonePickerProps> = ({
{selected.full}
</MenuToggle>
),
[onToggle, isTimezoneOpen, selected.full],
[onToggle, t, isTimezoneOpen, selected.full, id],
);

return (
Expand Down
47 changes: 12 additions & 35 deletions src/test/DateTimePicker/DateTimePicker.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { render, testT } from '../utils';
const onSelect = jest.fn((_: Date) => undefined);
const onDismiss = jest.fn();

const prefilledDate = new Date('14 Sep 2022 00:00:00');
const prefilledDate = new Date('10 Sep 2024 00:00:00');

jest.spyOn(defaultServices.settings, 'datetimeFormat').mockReturnValue(of(defaultDatetimeFormat));

Expand All @@ -36,8 +36,8 @@ describe('<DateTimePicker/>', () => {

afterEach(cleanup);

it('should show date tab as default', async () => {
render({
it('should show calendar, time and timezone select', async () => {
const { container } = render({
routerConfigs: {
routes: [
{
Expand All @@ -48,13 +48,14 @@ describe('<DateTimePicker/>', () => {
},
});

const dateTab = screen.getByRole('tab', { name: testT('DATE', { ns: 'common' }) });
expect(dateTab).toBeInTheDocument();
expect(dateTab).toBeVisible();
expect(dateTab.getAttribute('aria-selected')).toBe('true');
['calendar-month-view', 'time-picker', 'selected-datetime', 'timezone-picker'].forEach((id) => {
const element = container.querySelector(`#${id}`);
expect(element).toBeInTheDocument();
expect(element).toBeVisible();
});
});

it('should render currently selected date in calendar', async () => {
it('should show currently selected date in calendar', async () => {
render({
routerConfigs: {
routes: [
Expand All @@ -66,12 +67,12 @@ describe('<DateTimePicker/>', () => {
},
});

const selectedDate = screen.getByLabelText('14 September 2022');
const selectedDate = screen.getByLabelText('10 September 2024');
expect(selectedDate).toBeInTheDocument();
expect(selectedDate).toBeVisible();
});

it('should render currently selected datetime', async () => {
it('should show currently selected datetime', async () => {
render({
routerConfigs: {
routes: [
Expand All @@ -93,30 +94,6 @@ describe('<DateTimePicker/>', () => {
expect(timezoneDisplay).toBeVisible();
});

it('should switch to time tab when a date is selected', async () => {
const { user } = render({
routerConfigs: {
routes: [
{
path: '/recordings',
element: <DateTimePicker prefilledDate={prefilledDate} onSelect={onSelect} onDismiss={onDismiss} />,
},
],
},
});

const selectedDate = screen.getByLabelText('14 September 2022');
expect(selectedDate).toBeInTheDocument();
expect(selectedDate).toBeVisible();

await user.click(selectedDate);

const timeTab = screen.getByRole('tab', { name: testT('TIME', { ns: 'common' }) });
expect(timeTab).toBeInTheDocument();
expect(timeTab).toBeVisible();
expect(timeTab.getAttribute('aria-selected')).toBe('true');
});

it('should update selected datetime when date or time is seleted', async () => {
const { user } = render({
routerConfigs: {
Expand All @@ -129,7 +106,7 @@ describe('<DateTimePicker/>', () => {
},
});

const selectedDate = screen.getByLabelText('13 September 2022');
const selectedDate = screen.getByLabelText('13 September 2024');
expect(selectedDate).toBeInTheDocument();
expect(selectedDate).toBeVisible();

Expand Down
11 changes: 5 additions & 6 deletions src/test/DateTimePicker/TimePicker.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe('<TimePicker/>', () => {
expect(tree?.toJSON()).toMatchSnapshot();
});

it('should render correctly in 12hr mode', async () => {
it('should show correctly in 12hr mode when switched', async () => {
const { user } = render({
routerConfigs: {
routes: [
Expand All @@ -85,12 +85,11 @@ describe('<TimePicker/>', () => {
},
});

const switchenable24h = screen.getByLabelText(testT('TimePicker.24HOUR'));
expect(switchenable24h).toBeInTheDocument();
expect(switchenable24h).toBeVisible();
expect(switchenable24h).toBeChecked();
const toggleBtn12hr = screen.getByText(testT('TimePicker.12_H'));
expect(toggleBtn12hr).toBeInTheDocument();
expect(toggleBtn12hr).toBeVisible();

await user.click(switchenable24h);
await user.click(toggleBtn12hr);

const hInput = within(screen.getByLabelText(testT('HOUR', { ns: 'common' }))).getByLabelText(
testT('TimeSpinner.INPUT_HOUR12_VALUE'),
Expand Down
61 changes: 3 additions & 58 deletions src/test/DateTimePicker/TimezonePicker.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ describe('<TimezonePicker/>', () => {
},
});

const displaySelected = screen.getByText(
`(UTC${dayjs().tz(supportedTimezones()[0].full).format('Z')}) ${supportedTimezones()[0].full}`,
);
const displaySelected = screen.getByText(supportedTimezones()[0].full);
expect(displaySelected).toBeInTheDocument();
expect(displaySelected).toBeVisible();
});
Expand All @@ -62,7 +60,7 @@ describe('<TimezonePicker/>', () => {
},
});

const optionMenu = screen.getByLabelText('Options menu');
const optionMenu = screen.getByLabelText(testT('TimezonePicker.ARIA_LABELS.MENU_TOGGLE'));
expect(optionMenu).toBeInTheDocument();
expect(optionMenu).toBeVisible();

Expand All @@ -83,59 +81,6 @@ describe('<TimezonePicker/>', () => {
});
});

it('should correctly show selected in compact mode', async () => {
render({
routerConfigs: {
routes: [
{
path: '/recordings',
element: <TimezonePicker selected={supportedTimezones()[0]} onTimezoneChange={onTimezoneChange} />,
},
],
},
});

const displaySelected = screen.getByText(supportedTimezones()[0].short);
expect(displaySelected).toBeInTheDocument();
expect(displaySelected).toBeVisible();
});

it('should show correct timezone options in compact mode', async () => {
const { user } = render({
routerConfigs: {
routes: [
{
path: '/recordings',
element: <TimezonePicker selected={supportedTimezones()[0]} onTimezoneChange={onTimezoneChange} />,
},
],
},
});

const optionMenu = screen.getByLabelText('Options menu');
expect(optionMenu).toBeInTheDocument();
expect(optionMenu).toBeVisible();

await act(async () => {
await user.click(optionMenu);
});

const ul = screen.getByLabelText(testT('TimezonePicker.ARIA_LABELS.SELECT'));
expect(ul).toBeInTheDocument();
expect(ul).toBeVisible();

supportedTimezones()
.map((t) => t.short)
.forEach((short) => {
// Might be same full and short (e.g. UTC)
const optionDetails = within(ul).getAllByText(short);
optionDetails.forEach((d) => {
expect(d).toBeInTheDocument();
expect(d).toBeVisible();
});
});
});

it('should select a timezone when an option is clicked', async () => {
const { user } = render({
routerConfigs: {
Expand All @@ -148,7 +93,7 @@ describe('<TimezonePicker/>', () => {
},
});

const optionMenu = screen.getByLabelText('Options menu');
const optionMenu = screen.getByLabelText(testT('TimezonePicker.ARIA_LABELS.MENU_TOGGLE'));
expect(optionMenu).toBeInTheDocument();
expect(optionMenu).toBeVisible();

Expand Down
Loading

0 comments on commit db007da

Please sign in to comment.