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.

Sign up for GitHub

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

Jump to bottom

chore: Fix incorrect mock application in app layout tests #1586

Merged
merged 1 commit into from
Sep 27, 2023
Merged

chore: Fix incorrect mock application in app layout tests #1586

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
68 changes: 36 additions & 32 deletions src/app-layout/__tests__/runtime-drawers.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
// SPDX-License-Identifier: Apache-2.0
import React from 'react';
import { act, render } from '@testing-library/react';
import { describeEachAppLayout, singleDrawer } from './utils';
import AppLayout from '../../../lib/components/app-layout';
Comment on lines +5 to 6
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the root cause of the issue. ./utils file should be imported before app layout to initialize the module mock.

Thanks to the new test, it will be now captured in builds

Copy link
Member

Choose a reason for hiding this comment

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

Non-blocking: What are the chances that a new test file is added and someone imported these utils after the AppLayout module? Can we add maybe a comment or so for these utils to point out this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I added test('mocks applied correctly', () => {...}) specifically to capture this issue and fail the build when it happens.

Did you not see the test, or do you think this test does not work?

Copy link
Member

Choose a reason for hiding this comment

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

ahh, I see now, looks good to me.

I saw the mock test you added but I overlooked how the describeEachAppLayout and describeEachThemeAppLayout and that it will have that test running each time the utility is used, my bad.

import { InternalDrawerProps } from '../../../lib/components/app-layout/drawer/interfaces';
import { TOOLS_DRAWER_ID } from '../../../lib/components/app-layout/utils/use-drawers';
import { awsuiPlugins, awsuiPluginsInternal } from '../../../lib/components/internal/plugins/api';
import { DrawerConfig } from '../../../lib/components/internal/plugins/controllers/drawers';
import createWrapper from '../../../lib/components/test-utils/dom';
import { describeEachAppLayout, singleDrawer } from './utils';

beforeEach(() => {
awsuiPluginsInternal.appLayout.clearRegisteredDrawers();
Expand Down Expand Up @@ -38,7 +38,7 @@ const drawerDefaults: DrawerConfig = {
unmountContent: () => {},
};

describeEachAppLayout(() => {
describeEachAppLayout(size => {
test('does not render runtime drawers when it is explicitly disabled', async () => {
awsuiPlugins.appLayout.registerDrawer(drawerDefaults);
const { wrapper } = await renderComponent(<AppLayout {...({ __disableRuntimeDrawers: true } as any)} />);
Expand Down Expand Up @@ -150,18 +150,39 @@ describeEachAppLayout(() => {
expect(onToolsChange).not.toHaveBeenCalled();
});

test('allows switching drawers when toolsOpen is controlled', async () => {
const onToolsChange = jest.fn();
awsuiPlugins.appLayout.registerDrawer(drawerDefaults);
const { wrapper } = await renderComponent(
<AppLayout tools="Tools content" toolsOpen={false} onToolsChange={event => onToolsChange(event.detail)} />
);
wrapper.findDrawerTriggerById(drawerDefaults.id)!.click();
expect(onToolsChange).toHaveBeenCalledWith({ open: false });
expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('runtime drawer content');
// skipping these on mobile, because drawers toggles are hidden when mobile mode is used
(size === 'desktop' ? describe : describe.skip)('switching drawers', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Regrouped and skipped some tests which are not supposed to pass on mobile by design

test('drawer content updates when switching active drawers', async () => {
awsuiPlugins.appLayout.registerDrawer({
...drawerDefaults,
id: 'first',
mountContent: container => (container.textContent = 'first drawer content'),
});
awsuiPlugins.appLayout.registerDrawer({
...drawerDefaults,
id: 'second',
mountContent: container => (container.textContent = 'second drawer content'),
});
const { wrapper } = await renderComponent(<AppLayout />);
wrapper.findDrawerTriggerById('first')!.click();
expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('first drawer content');
wrapper.findDrawerTriggerById('second')!.click();
expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('second drawer content');
});

wrapper.findToolsToggle().click();
expect(onToolsChange).toHaveBeenCalledWith({ open: true });
test('allows switching drawers when toolsOpen is controlled', async () => {
const onToolsChange = jest.fn();
awsuiPlugins.appLayout.registerDrawer(drawerDefaults);
const { wrapper } = await renderComponent(
<AppLayout tools="Tools content" toolsOpen={false} onToolsChange={event => onToolsChange(event.detail)} />
);
wrapper.findDrawerTriggerById(drawerDefaults.id)!.click();
expect(onToolsChange).toHaveBeenCalledWith({ open: false });
expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('runtime drawer content');

wrapper.findToolsToggle().click();
expect(onToolsChange).toHaveBeenCalledWith({ open: true });
});
});

test('updates active drawer if multiple are registered', async () => {
Expand Down Expand Up @@ -295,25 +316,8 @@ describeEachAppLayout(() => {
expect(unmountContent).toHaveBeenCalledTimes(1);
});

test('drawer content updates when switching active drawers', async () => {
awsuiPlugins.appLayout.registerDrawer({
...drawerDefaults,
id: 'first',
mountContent: container => (container.textContent = 'first drawer content'),
});
awsuiPlugins.appLayout.registerDrawer({
...drawerDefaults,
id: 'second',
mountContent: container => (container.textContent = 'second drawer content'),
});
const { wrapper } = await renderComponent(<AppLayout />);
wrapper.findDrawerTriggerById('first')!.click();
expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('first drawer content');
wrapper.findDrawerTriggerById('second')!.click();
expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('second drawer content');
});

describe('ordering', () => {
// skip these tests on mobile mode, because triggers will overflow
(size === 'desktop' ? describe : describe.skip)('ordering', () => {
test('renders multiple drawers in alphabetical order by default', async () => {
awsuiPlugins.appLayout.registerDrawer({ ...drawerDefaults, id: 'bbb', ariaLabels: { triggerButton: 'bbb' } });
awsuiPlugins.appLayout.registerDrawer({ ...drawerDefaults, id: 'aaa', ariaLabels: { triggerButton: 'aaa' } });
Expand Down
32 changes: 13 additions & 19 deletions src/app-layout/__tests__/utils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0
import * as React from 'react';
import { render } from '@testing-library/react';
import AppLayout from '../../../lib/components/app-layout';
import { SplitPanelProps } from '../../../lib/components/split-panel';
import createWrapper, { ElementWrapper } from '../../../lib/components/test-utils/dom';
import { useMobile } from '../../../lib/components/internal/hooks/use-mobile';
Expand Down Expand Up @@ -34,30 +35,13 @@ export function renderComponent(jsx: React.ReactElement) {
const wrapper = createWrapper(container).findAppLayout()!;

const isUsingGridLayout = wrapper.getElement().classList.contains(visualRefreshStyles.layout);
const isUsingMobile = !!wrapper.findByClassName(testutilStyles['mobile-bar']);

const contentElement = isUsingGridLayout
? wrapper.getElement()
: wrapper.findByClassName(styles['layout-wrapper'])!.getElement();

return { wrapper, rerender, isUsingGridLayout, contentElement, container };
}

export function describeDesktopAppLayout(callback: () => void) {
describe('Desktop', () => {
beforeEach(() => {
(useMobile as jest.Mock).mockReturnValue(false);
});
callback();
});
}

export function describeMobileAppLayout(callback: () => void) {
describe('Mobile', () => {
beforeEach(() => {
(useMobile as jest.Mock).mockReturnValue(true);
});
callback();
});
Comment on lines -45 to -60
Copy link
Member Author

Choose a reason for hiding this comment

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

These utils are unused, removing

return { wrapper, rerender, isUsingGridLayout, isUsingMobile, contentElement, container };
}

export function describeEachThemeAppLayout(isMobile: boolean, callback: (theme: string) => void) {
Expand All @@ -71,6 +55,11 @@ export function describeEachThemeAppLayout(isMobile: boolean, callback: (theme:
(useMobile as jest.Mock).mockReset();
(useVisualRefresh as jest.Mock).mockReset();
});
test('mocks applied correctly', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

This test ensures that whenever you use this utility it renders app layout with the correct environment

const { isUsingGridLayout, isUsingMobile } = renderComponent(<AppLayout />);
expect(isUsingGridLayout).toEqual(theme === 'refresh');
expect(isUsingMobile).toEqual(isMobile);
});
callback(theme);
});
}
Expand All @@ -88,6 +77,11 @@ export function describeEachAppLayout(callback: (size: 'desktop' | 'mobile') =>
(useMobile as jest.Mock).mockReset();
(useVisualRefresh as jest.Mock).mockReset();
});
test('mocks applied correctly', () => {
const { isUsingGridLayout, isUsingMobile } = renderComponent(<AppLayout />);
expect(isUsingGridLayout).toEqual(theme === 'refresh');
expect(isUsingMobile).toEqual(size === 'mobile');
});
callback(size);
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/app-layout/mobile-toolbar/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export function MobileToolbar({
return (
<div
ref={mobileBarRef}
className={clsx(styles['mobile-bar'], unfocusable && sharedStyles.unfocusable)}
className={clsx(styles['mobile-bar'], testutilStyles['mobile-bar'], unfocusable && sharedStyles.unfocusable)}
style={{ top: topOffset }}
>
{!navigationHide && (
Expand Down
Loading