Skip to content

Commit

Permalink
Fix/simplify more mobile vs desktop test assertions
Browse files Browse the repository at this point in the history
- DOM no longer renders for mobile vs desktop whatsoever, so we get to remove a bunch of test utils and just mock a mobile window width instead
  • Loading branch information
cee-chen committed Apr 11, 2024
1 parent c3eeb08 commit 78cefcd
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import React from 'react';
import { JourneyStep } from '../../../../../common/runtime_types/ping';
import { StepsList } from './steps_list';
import { render, forDesktopOnly, forMobileOnly } from '../../../lib/helper/rtl_helpers';
import { render } from '../../../lib/helper/rtl_helpers';
import { VIEW_PERFORMANCE } from '../../monitor/synthetics/translations';

describe('StepList component', () => {
Expand Down Expand Up @@ -83,7 +83,7 @@ describe('StepList component', () => {
<StepsList data={[steps[0]]} loading={false} allStepsLoaded={true} />
);
expect(getByTestId('step-detail-link')).toHaveAttribute('href', '/journey/fake-group/step/1');
expect(forDesktopOnly(getByTitle, 'title')(`Failed`));
expect(getByTitle(`Failed`)).toBeInTheDocument();
});

it.each([
Expand All @@ -94,7 +94,7 @@ describe('StepList component', () => {
const step = steps[0];
step.synthetics!.payload!.status = status;
const { getByText } = render(<StepsList data={[step]} loading={false} allStepsLoaded={true} />);
expect(forDesktopOnly(getByText)(expectedStatus));
expect(getByText(expectedStatus)).toBeInTheDocument();
});

it('creates expected message for all succeeded', () => {
Expand Down Expand Up @@ -156,24 +156,27 @@ describe('StepList component', () => {
});

describe('Mobile Designs', () => {
// We don't need to resize the window here because EUI
// does all the manipulation of what is displayed through
// CSS. Therefore, it's enough to check what's actually
// rendered and its classes.
const jestWindowWidth = window.innerWidth;
beforeAll(() => {
window.innerWidth = 600;
});
afterAll(() => {
window.innerWidth = jestWindowWidth;
});

it('renders the step name and index', () => {
const { getByText } = render(
<StepsList data={steps} loading={false} allStepsLoaded={true} />
);
expect(forMobileOnly(getByText)('1. load page')).toBeInTheDocument();
expect(forMobileOnly(getByText)('2. go to login')).toBeInTheDocument();
expect(getByText('1. load page')).toBeInTheDocument();
expect(getByText('2. go to login')).toBeInTheDocument();
});

it('does not render the link to view step details', async () => {
const { queryByText } = render(
<StepsList data={steps} loading={false} allStepsLoaded={true} />
);
expect(forMobileOnly(queryByText)(VIEW_PERFORMANCE)).not.toBeInTheDocument();
expect(queryByText(VIEW_PERFORMANCE)).not.toBeInTheDocument();
});

it('renders the status label', () => {
Expand All @@ -183,8 +186,8 @@ describe('StepList component', () => {
const { getByText } = render(
<StepsList data={steps} loading={false} allStepsLoaded={true} />
);
expect(forMobileOnly(getByText)('Succeeded')).toBeInTheDocument();
expect(forMobileOnly(getByText)('Skipped')).toBeInTheDocument();
expect(getByText('Succeeded')).toBeInTheDocument();
expect(getByText('Skipped')).toBeInTheDocument();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -356,38 +356,3 @@ export const makeUptimePermissionsCore = (
},
};
};

// This function filters out the queried elements which appear only
// either on mobile or desktop.
//
// It does so by filtering those with the class passed as the `classWrapper`.
// For mobile, we filter classes which tell elements to be hidden on desktop.
// For desktop, we do the opposite.
//
// We have this function because EUI will manipulate the visibility of some
// elements through pure CSS, which we can't assert on tests. Therefore,
// we look for the corresponding class wrapper.
const finderWithClassWrapper =
(classWrapper: string) =>
(
getterFn: (f: MatcherFunction) => HTMLElement | null,
customAttribute?: keyof Element | keyof HTMLElement
) =>
(text: string): HTMLElement | null =>
getterFn((_content: string, node: Element | null) => {
if (!node) return false;
// There are actually properties that are not in Element but which
// appear on the `node`, so we must cast the customAttribute as a keyof Element
const content = node[(customAttribute as keyof Element) ?? 'innerHTML'];
if (content === text && wrappedInClass(node, classWrapper)) return true;
return false;
});

const wrappedInClass = (element: HTMLElement | Element, classWrapper: string): boolean => {
if (element.className.includes(classWrapper)) return true;
if (element.parentElement) return wrappedInClass(element.parentElement, classWrapper);
return false;
};

export const forMobileOnly = finderWithClassWrapper('hideForDesktop');
export const forDesktopOnly = finderWithClassWrapper('hideForMobile');
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,6 @@ import { TimelineTabs } from '../../../../common/types/timeline';

jest.mock('../../lib/kibana');

jest.mock('@elastic/eui', () => {
const original = jest.requireActual('@elastic/eui');
return {
...original,
EuiScreenReaderOnly: () => <></>,
};
});

jest.mock('../../hooks/use_get_field_spec');

jest.mock('@kbn/cell-actions/src/hooks/use_load_actions', () => {
Expand Down

0 comments on commit 78cefcd

Please sign in to comment.