Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: fix TimezoneSelector tests on daylight saving time #19156

Merged
merged 4 commits into from
Mar 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion superset-frontend/cypress-base/cypress.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"baseUrl": "http://localhost:8088",
"chromeWebSecurity": false,
"defaultCommandTimeout": 5000,
"defaultCommandTimeout": 8000,
"numTestsKeptInMemory": 0,
"experimentalFetchPolyfill": true,
"requestTimeout": 10000,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,29 @@ interface QueryString {
native_filters_key: string;
}

describe('nativefiler url param key', () => {
xdescribe('nativefiler url param key', () => {
// const urlParams = { param1: '123', param2: 'abc' };
before(() => {
cy.login();
cy.visit(WORLD_HEALTH_DASHBOARD);
WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
cy.wait(1000); // wait for key to be published (debounced)
});

let initialFilterKey: string;
it('should have cachekey in nativefilter param', () => {
// things in `before` will not retry and the `waitForChartLoad` check is
// especically flaky and may need more retries
cy.visit(WORLD_HEALTH_DASHBOARD);
WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
cy.wait(1000); // wait for key to be published (debounced)
cy.location().then(loc => {
const queryParams = qs.parse(loc.search) as QueryString;
expect(typeof queryParams.native_filters_key).eq('string');
});
});

it('should have different key when page reloads', () => {
cy.visit(WORLD_HEALTH_DASHBOARD);
WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
cy.wait(1000); // wait for key to be published (debounced)
cy.location().then(loc => {
const queryParams = qs.parse(loc.search) as QueryString;
expect(queryParams.native_filters_key).not.equal(initialFilterKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@
*/
import React from 'react';
import { useArgs } from '@storybook/client-api';
import TimezoneSelector, { TimezoneProps } from './index';
import TimezoneSelector, { TimezoneSelectorProps } from './index';

export default {
title: 'TimezoneSelector',
component: TimezoneSelector,
};

// eslint-disable-next-line @typescript-eslint/no-unused-vars
export const InteractiveTimezoneSelector = (args: TimezoneProps) => {
export const InteractiveTimezoneSelector = (args: TimezoneSelectorProps) => {
const [{ timezone }, updateArgs] = useArgs();
const onTimezoneChange = (value: string) => {
updateArgs({ timezone: value });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,42 @@ import React from 'react';
import moment from 'moment-timezone';
import { render, screen, waitFor } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import TimezoneSelector from './index';
import type { TimezoneSelectorProps } from './index';

jest.spyOn(moment.tz, 'guess').mockReturnValue('America/New_York');
const loadComponent = (mockCurrentTime?: string) => {
if (mockCurrentTime) {
jest.useFakeTimers('modern');
jest.setSystemTime(new Date(mockCurrentTime));
}
return new Promise<React.FC<TimezoneSelectorProps>>(resolve => {
jest.isolateModules(() => {
const { default: TimezoneSelector } = module.require('./index');
resolve(TimezoneSelector);
jest.useRealTimers();
});
});
};

const getSelectOptions = () =>
waitFor(() => document.querySelectorAll('.ant-select-item-option-content'));

it('use the timezone from `moment` if no timezone provided', () => {
const openSelectMenu = async () => {
const searchInput = screen.getByRole('combobox');
userEvent.click(searchInput);
};

jest.spyOn(moment.tz, 'guess').mockReturnValue('America/New_York');

test('use the timezone from `moment` if no timezone provided', async () => {
const TimezoneSelector = await loadComponent('2022-01-01');
const onTimezoneChange = jest.fn();
render(<TimezoneSelector onTimezoneChange={onTimezoneChange} />);
expect(onTimezoneChange).toHaveBeenCalledTimes(1);
expect(onTimezoneChange).toHaveBeenCalledWith('America/Nassau');
});

it('update to closest deduped timezone when timezone is provided', async () => {
test('update to closest deduped timezone when timezone is provided', async () => {
const TimezoneSelector = await loadComponent('2022-01-01');
const onTimezoneChange = jest.fn();
render(
<TimezoneSelector
Expand All @@ -46,7 +67,8 @@ it('update to closest deduped timezone when timezone is provided', async () => {
expect(onTimezoneChange).toHaveBeenLastCalledWith('America/Vancouver');
});

it('use the default timezone when an invalid timezone is provided', async () => {
test('use the default timezone when an invalid timezone is provided', async () => {
const TimezoneSelector = await loadComponent('2022-01-01');
const onTimezoneChange = jest.fn();
render(
<TimezoneSelector onTimezoneChange={onTimezoneChange} timezone="UTC" />,
Expand All @@ -55,49 +77,65 @@ it('use the default timezone when an invalid timezone is provided', async () =>
expect(onTimezoneChange).toHaveBeenLastCalledWith('Africa/Abidjan');
});

it.skip('can select a timezone values and returns canonical value', async () => {
test('render timezones in correct oder for standard time', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit (if you happen to need to push more changes):

Suggested change
test('render timezones in correct oder for standard time', async () => {
test('render timezones in correct order for standard time', async () => {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like I don't... Will fix it in some other PR.

const TimezoneSelector = await loadComponent('2022-01-01');
const onTimezoneChange = jest.fn();
render(
<TimezoneSelector
onTimezoneChange={onTimezoneChange}
timezone="America/Nassau"
/>,
);

const searchInput = screen.getByRole('combobox', {
name: 'Timezone selector',
});
expect(searchInput).toBeInTheDocument();
userEvent.click(searchInput);
const isDaylight = moment(moment.now()).isDST();

const selectedTimezone = isDaylight
? 'GMT -04:00 (Eastern Daylight Time)'
: 'GMT -05:00 (Eastern Standard Time)';

// selected option ranks first
await openSelectMenu();
const options = await getSelectOptions();
expect(options[0]).toHaveTextContent(selectedTimezone);

// others are ranked by offset
expect(options[0]).toHaveTextContent('GMT -05:00 (Eastern Standard Time)');
expect(options[1]).toHaveTextContent('GMT -11:00 (Pacific/Pago_Pago)');
expect(options[2]).toHaveTextContent('GMT -10:00 (Hawaii Standard Time)');
expect(options[3]).toHaveTextContent('GMT -10:00 (America/Adak)');
});

test('render timezones in correct order for daylight saving time', async () => {
const TimezoneSelector = await loadComponent('2022-07-01');
const onTimezoneChange = jest.fn();
render(
<TimezoneSelector
onTimezoneChange={onTimezoneChange}
timezone="America/Nassau"
/>,
);
await openSelectMenu();
const options = await getSelectOptions();
// first option is always current timezone
expect(options[0]).toHaveTextContent('GMT -04:00 (Eastern Daylight Time)');
expect(options[1]).toHaveTextContent('GMT -11:00 (Pacific/Pago_Pago)');
expect(options[2]).toHaveTextContent('GMT -10:00 (Hawaii Standard Time)');
expect(options[3]).toHaveTextContent('GMT -09:30 (Pacific/Marquesas)');
});

test('can select a timezone values and returns canonical timezone name', async () => {
const TimezoneSelector = await loadComponent('2022-01-01');
const onTimezoneChange = jest.fn();
render(
<TimezoneSelector
onTimezoneChange={onTimezoneChange}
timezone="Africa/Abidjan"
/>,
);

await openSelectMenu();

const searchInput = screen.getByRole('combobox');
// search for mountain time
await userEvent.type(searchInput, 'mou', { delay: 10 });

const findTitle = isDaylight
? 'GMT -06:00 (Mountain Daylight Time)'
: 'GMT -07:00 (Mountain Standard Time)';
const findTitle = 'GMT -07:00 (Mountain Standard Time)';
const selectOption = await screen.findByTitle(findTitle);
expect(selectOption).toBeInTheDocument();
userEvent.click(selectOption);
expect(onTimezoneChange).toHaveBeenCalledTimes(1);
expect(onTimezoneChange).toHaveBeenLastCalledWith('America/Cambridge_Bay');
});

it('can update props and rerender with different values', async () => {
test('can update props and rerender with different values', async () => {
const TimezoneSelector = await loadComponent('2022-01-01');
const onTimezoneChange = jest.fn();
const { rerender } = render(
<TimezoneSelector
Expand Down
19 changes: 10 additions & 9 deletions superset-frontend/src/components/TimezoneSelector/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,6 @@ const getTimezoneName = (name: string) => {
);
};

export interface TimezoneProps {
onTimezoneChange: (value: string) => void;
timezone?: string | null;
}

const ALL_ZONES = moment.tz
.countries()
.map(country => moment.tz.zonesForCountry(country, true))
Expand Down Expand Up @@ -106,7 +101,15 @@ const matchTimezoneToOptions = (timezone: string) =>
TIMEZONE_OPTIONS.find(option => option.offsets === getOffsetKey(timezone))
?.value || DEFAULT_TIMEZONE.value;

const TimezoneSelector = ({ onTimezoneChange, timezone }: TimezoneProps) => {
export type TimezoneSelectorProps = {
onTimezoneChange: (value: string) => void;
timezone?: string | null;
};

export default function TimezoneSelector({
onTimezoneChange,
timezone,
}: TimezoneSelectorProps) {
const validTimezone = useMemo(
() => matchTimezoneToOptions(timezone || moment.tz.guess()),
[timezone],
Expand All @@ -129,6 +132,4 @@ const TimezoneSelector = ({ onTimezoneChange, timezone }: TimezoneProps) => {
sortComparator={TIMEZONE_OPTIONS_SORT_COMPARATOR}
/>
);
};

export default TimezoneSelector;
}
2 changes: 1 addition & 1 deletion superset/utils/async_query_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class AsyncQueryManager:

def __init__(self) -> None:
super().__init__()
self._redis: redis.Redis # type: ignore
self._redis: redis.Redis
self._stream_prefix: str = ""
self._stream_limit: Optional[int]
self._stream_limit_firehose: Optional[int]
Expand Down