Skip to content

Deprecate react-unit-test-utils 1/2 #1750

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

Merged
merged 3 commits into from
Jul 28, 2025
Merged

Deprecate react-unit-test-utils 1/2 #1750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
350be7b
chore: remove react-unit-test-utils
diana-villalvazo-wgu Jul 3, 2025
034646c
chore: remove commented line
diana-villalvazo-wgu Jul 3, 2025
642c3c3
test: remove unnecesary line and add test for close dialog
diana-villalvazo-wgu Jul 28, 2025
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
6 changes: 2 additions & 4 deletions src/courseware/course/sequence/Unit/ContentIFrame.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import PropTypes from 'prop-types';
import React from 'react';

import { ErrorPage } from '@edx/frontend-platform/react';
import { StrictDict } from '@edx/react-unit-test-utils';
import { ModalDialog } from '@openedx/paragon';
import { ContentIFrameLoaderSlot } from '../../../../plugin-slots/ContentIFrameLoaderSlot';

Expand All @@ -22,10 +20,10 @@ export const IFRAME_FEATURE_POLICY = (
'microphone *; camera *; midi *; geolocation *; encrypted-media *; clipboard-write *; autoplay *'
);

export const testIDs = StrictDict({
export const testIDs = {
contentIFrame: 'content-iframe-test-id',
modalIFrame: 'modal-iframe-test-id',
});
};

const ContentIFrame = ({
iframeUrl,
Expand Down
145 changes: 58 additions & 87 deletions src/courseware/course/sequence/Unit/ContentIFrame.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,11 @@
import React from 'react';
import { render, screen } from '@testing-library/react';

import { ErrorPage } from '@edx/frontend-platform/react';
import { ModalDialog } from '@openedx/paragon';
import { shallow } from '@edx/react-unit-test-utils';

import PageLoading from '@src/generic/PageLoading';

import { ContentIFrameLoaderSlot } from '@src/plugin-slots/ContentIFrameLoaderSlot';
import * as hooks from './hooks';
import ContentIFrame, { IFRAME_FEATURE_POLICY, testIDs } from './ContentIFrame';
import ContentIFrame, { IFRAME_FEATURE_POLICY } from './ContentIFrame';

jest.mock('@edx/frontend-platform/react', () => ({ ErrorPage: 'ErrorPage' }));
jest.mock('@edx/frontend-platform/react', () => ({ ErrorPage: () => <div>ErrorPage</div> }));

jest.mock('@openedx/paragon', () => jest.requireActual('@edx/react-unit-test-utils')
.mockComponents({
ModalDialog: {
Body: 'ModalDialog.Body',
},
}));

jest.mock('@src/generic/PageLoading', () => 'PageLoading');
jest.mock('@src/generic/PageLoading', () => jest.fn(() => <div>PageLoading</div>));

jest.mock('./hooks', () => ({
useIFrameBehavior: jest.fn(),
Expand Down Expand Up @@ -67,14 +53,13 @@ const props = {
title: 'test-title',
};

let el;
describe('ContentIFrame Component', () => {
beforeEach(() => {
jest.clearAllMocks();
});
describe('behavior', () => {
beforeEach(() => {
el = shallow(<ContentIFrame {...props} />);
render(<ContentIFrame {...props} />);
});
it('initializes iframe behavior hook', () => {
expect(hooks.useIFrameBehavior).toHaveBeenCalledWith({
Expand All @@ -89,61 +74,61 @@ describe('ContentIFrame Component', () => {
});
});
describe('output', () => {
let component;
describe('if shouldShowContent', () => {
describe('if not hasLoaded', () => {
it('displays errorPage if showError', () => {
hooks.useIFrameBehavior.mockReturnValueOnce({ ...iframeBehavior, showError: true });
el = shallow(<ContentIFrame {...props} />);
expect(el.instance.findByType(ErrorPage).length).toEqual(1);
render(<ContentIFrame {...props} />);
const errorPage = screen.getByText('ErrorPage');
expect(errorPage).toBeInTheDocument();
});
it('displays PageLoading component if not showError', () => {
el = shallow(<ContentIFrame {...props} />);
[component] = el.instance.findByType(ContentIFrameLoaderSlot);
expect(component.props.loadingMessage).toEqual(props.loadingMessage);
render(<ContentIFrame {...props} />);
const pageLoading = screen.getByText('PageLoading');
expect(pageLoading).toBeInTheDocument();
});
});
describe('hasLoaded', () => {
it('does not display PageLoading or ErrorPage', () => {
hooks.useIFrameBehavior.mockReturnValueOnce({ ...iframeBehavior, hasLoaded: true });
el = shallow(<ContentIFrame {...props} />);
expect(el.instance.findByType(PageLoading).length).toEqual(0);
expect(el.instance.findByType(ErrorPage).length).toEqual(0);
render(<ContentIFrame {...props} />);
const pageLoading = screen.queryByText('PageLoading');
expect(pageLoading).toBeNull();
const errorPage = screen.queryByText('ErrorPage');
expect(errorPage).toBeNull();
});
});
it('display iframe with props from hooks', () => {
el = shallow(<ContentIFrame {...props} />);
[component] = el.instance.findByTestId(testIDs.contentIFrame);
expect(component.props).toEqual({
allow: IFRAME_FEATURE_POLICY,
allowFullScreen: true,
scrolling: 'no',
referrerPolicy: 'origin',
title: props.title,
id: props.elementId,
src: props.iframeUrl,
height: iframeBehavior.iframeHeight,
onLoad: iframeBehavior.handleIFrameLoad,
'data-testid': testIDs.contentIFrame,
});
render(<ContentIFrame {...props} />);
const iframe = screen.getByTitle(props.title);
expect(iframe).toBeInTheDocument();
expect(iframe).toHaveAttribute('id', props.elementId);
expect(iframe).toHaveAttribute('src', props.iframeUrl);
expect(iframe).toHaveAttribute('allow', IFRAME_FEATURE_POLICY);
expect(iframe).toHaveAttribute('allowfullscreen', '');
expect(iframe).toHaveAttribute('scrolling', 'no');
expect(iframe).toHaveAttribute('referrerpolicy', 'origin');
});
});
describe('if not shouldShowContent', () => {
it('does not show PageLoading, ErrorPage, or unit-iframe-wrapper', () => {
el = shallow(<ContentIFrame {...{ ...props, shouldShowContent: false }} />);
expect(el.instance.findByType(PageLoading).length).toEqual(0);
expect(el.instance.findByType(ErrorPage).length).toEqual(0);
expect(el.instance.findByTestId(testIDs.contentIFrame).length).toEqual(0);
render(<ContentIFrame {...{ ...props, shouldShowContent: false }} />);
expect(screen.queryByText('PageLoading')).toBeNull();
expect(screen.queryByText('ErrorPage')).toBeNull();
expect(screen.queryByTitle(props.title)).toBeNull();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there ever a scenario where we wouldn't find the title but could still find the unit-iframe-wrapper? Just want to confirm that switching from checking for the contentIFrame test id to this still verifies the same thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the codebase when this component is used, it appends the unit title, only if you don't add a title to a unit that will be applicable 🤔 i can add that use case if needed

Copy link
Contributor

Choose a reason for hiding this comment

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

I just know that the previous version of this was testing for testIDs.contentIFrame, and now it's testing for props.title. If there's a scenario where we would have the test id but not the title then that means we've changed what this test is looking for.

Copy link
Contributor Author

@diana-villalvazo-wgu diana-villalvazo-wgu Jul 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

yeah i made that change in order to follow Braden's suggestion of

I thought get it by title will be the accessible query that fits this use case, but we can bring it back if you think we are missing an edge case here 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

As a reviewer this is something where having added context would be helpful. Something like:

Previously this was testing for testIDs.contentIFrame, which was coming from

frontend-app-learning/src/courseware/course/sequence/Unit/ContentIFrame.jsx

Line 26 in a75c89c

contentIFrame: 'content-iframe-test-id',

In the component, the test id is used here

frontend-app-learning/src/courseware/course/sequence/Unit/ContentIFrame.jsx

Lines 73 to 77 in a75c89c

{shouldShowContent && (
<div className="unit-iframe-wrapper">
<iframe title={title} {...contentIFrameProps} data-testid={testIDs.contentIFrame} />
</div>
)}

this also renders the title, which is preferable to test for to follow RTL best practices.

Since title is a required prop for the ContentIFrame component

frontend-app-learning/src/courseware/course/sequence/Unit/ContentIFrame.jsx

Line 113 in a75c89c

title: PropTypes.node.isRequired,

this should still cover all cases that the previous test id based test covered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure i will have this in mind on following PRs! Thanks Brian 😄

});
});
it('does not display modal if modalOptions returns isOpen: false', () => {
el = shallow(<ContentIFrame {...props} />);
expect(el.instance.findByType(ModalDialog).length).toEqual(0);
render(<ContentIFrame {...props} />);
const modal = screen.queryByRole('dialog');
expect(modal).toBeNull();
});
describe('if modalOptions.isOpen', () => {
const testModalOpenAndHandleClose = () => {
test('Modal component isOpen, with handleModalClose from hook', () => {
expect(component.props.onClose).toEqual(modalIFrameData.handleModalClose);
it('closes modal on close button click', () => {
const closeButton = screen.getByTestId('modal-backdrop');
closeButton.click();
expect(modalIFrameData.handleModalClose).toHaveBeenCalled();
});
};
describe('fullscreen modal', () => {
Expand All @@ -153,14 +138,13 @@ describe('ContentIFrame Component', () => {
...modalIFrameData,
modalOptions: { ...modalOptions.withBody, isFullscreen: true },
});
el = shallow(<ContentIFrame {...props} />);
[component] = el.instance.findByType(ModalDialog);
render(<ContentIFrame {...props} />);
});
it('displays Modal with div wrapping provided body content if modal.body is provided', () => {
const content = component.findByType(ModalDialog.Body)[0].children[0];
expect(content.matches(shallow(
<div className="unit-modal">{modalOptions.withBody.body}</div>,
))).toEqual(true);
const dialog = screen.getByRole('dialog');
expect(dialog).toBeInTheDocument();
const modalBody = screen.getByText(modalOptions.withBody.body);
expect(modalBody).toBeInTheDocument();
});
testModalOpenAndHandleClose();
});
Expand All @@ -171,55 +155,42 @@ describe('ContentIFrame Component', () => {
...modalIFrameData,
modalOptions: { ...modalOptions.withUrl, isFullscreen: true },
});
el = shallow(<ContentIFrame {...props} />);
[component] = el.instance.findByType(ModalDialog);
render(<ContentIFrame {...props} />);
});
testModalOpenAndHandleClose();
it('displays Modal with iframe to provided url if modal.body is not provided', () => {
const content = component.findByType(ModalDialog.Body)[0].children[0];
expect(content.matches(shallow(
<iframe
title={modalOptions.withUrl.title}
allow={IFRAME_FEATURE_POLICY}
frameBorder="0"
src={modalOptions.withUrl.url}
style={{ width: '100%', height: modalOptions.withUrl.height }}
/>,
))).toEqual(true);
const iframe = screen.getByTitle(modalOptions.withUrl.title);
expect(iframe).toBeInTheDocument();
expect(iframe).toHaveAttribute('allow', IFRAME_FEATURE_POLICY);
expect(iframe).toHaveAttribute('src', modalOptions.withUrl.url);
});
testModalOpenAndHandleClose();
});
});
describe('body modal', () => {
beforeEach(() => {
hooks.useModalIFrameData.mockReturnValueOnce({ ...modalIFrameData, modalOptions: modalOptions.withBody });
el = shallow(<ContentIFrame {...props} />);
[component] = el.instance.findByType(ModalDialog);
render(<ContentIFrame {...props} />);
});
it('displays Modal with div wrapping provided body content if modal.body is provided', () => {
const content = component.findByType(ModalDialog.Body)[0].children[0];
expect(content.matches(shallow(<div className="unit-modal">{modalOptions.withBody.body}</div>))).toEqual(true);
const dialog = screen.getByRole('dialog');
expect(dialog).toBeInTheDocument();
const modalBody = screen.getByText(modalOptions.withBody.body);
expect(modalBody).toBeInTheDocument();
});
testModalOpenAndHandleClose();
});
describe('url modal', () => {
beforeEach(() => {
hooks.useModalIFrameData.mockReturnValueOnce({ ...modalIFrameData, modalOptions: modalOptions.withUrl });
el = shallow(<ContentIFrame {...props} />);
[component] = el.instance.findByType(ModalDialog);
render(<ContentIFrame {...props} />);
});
testModalOpenAndHandleClose();
it('displays Modal with iframe to provided url if modal.body is not provided', () => {
const content = component.findByType(ModalDialog.Body)[0].children[0];
expect(content.matches(shallow(
<iframe
title={modalOptions.withUrl.title}
allow={IFRAME_FEATURE_POLICY}
frameBorder="0"
src={modalOptions.withUrl.url}
style={{ width: '100%', height: modalOptions.withUrl.height }}
/>,
))).toEqual(true);
const iframe = screen.getByTitle(modalOptions.withUrl.title);
expect(iframe).toBeInTheDocument();
expect(iframe).toHaveAttribute('allow', IFRAME_FEATURE_POLICY);
expect(iframe).toHaveAttribute('src', modalOptions.withUrl.url);
});
testModalOpenAndHandleClose();
});
});
});
Expand Down
54 changes: 23 additions & 31 deletions src/courseware/course/sequence/Unit/UnitSuspense.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,33 +1,25 @@
import React from 'react';

import { formatMessage, shallow } from '@edx/react-unit-test-utils';
import { render, screen } from '@testing-library/react';
import { IntlProvider } from '@edx/frontend-platform/i18n';

import { useModel } from '@src/generic/model-store';
import PageLoading from '@src/generic/PageLoading';

import { GatedUnitContentMessageSlot } from '@src/plugin-slots/GatedUnitContentMessageSlot';
import messages from '../messages';
import HonorCode from '../honor-code';
import LockPaywall from '../lock-paywall';
import hooks from './hooks';
import { modelKeys } from './constants';

import UnitSuspense from './UnitSuspense';

jest.mock('@edx/frontend-platform/i18n', () => ({
...jest.requireActual('@edx/frontend-platform/i18n'),
defineMessages: m => m,
useIntl: () => ({ formatMessage: jest.requireActual('@edx/react-unit-test-utils').formatMessage }),
}));

jest.mock('react', () => ({
...jest.requireActual('react'),
Suspense: 'Suspense',
}));

jest.mock('../honor-code', () => 'HonorCode');
jest.mock('../lock-paywall', () => 'LockPaywall');
jest.mock('../honor-code', () => jest.fn(() => <div>HonorCode</div>));
jest.mock('../lock-paywall', () => jest.fn(() => <div>LockPaywall</div>));
jest.mock('@src/generic/model-store', () => ({ useModel: jest.fn() }));
jest.mock('@src/generic/PageLoading', () => 'PageLoading');

jest.mock('./hooks', () => ({
useShouldDisplayHonorCode: jest.fn(() => false),
Expand All @@ -46,15 +38,14 @@ const props = {
id: 'test-id',
};

let el;
describe('UnitSuspense component', () => {
beforeEach(() => {
jest.clearAllMocks();
mockModels(false, false);
});
describe('behavior', () => {
it('initializes models', () => {
el = shallow(<UnitSuspense {...props} />);
render(<IntlProvider locale="en"><UnitSuspense {...props} /></IntlProvider>);
const { calls } = useModel.mock;
const [unitCall] = calls.filter(call => call[0] === modelKeys.units);
const [metaCall] = calls.filter(call => call[0] === modelKeys.coursewareMeta);
Expand All @@ -66,8 +57,9 @@ describe('UnitSuspense component', () => {
describe('LockPaywall', () => {
const testNoPaywall = () => {
it('does not display LockPaywall', () => {
el = shallow(<UnitSuspense {...props} />);
expect(el.instance.findByType(LockPaywall).length).toEqual(0);
render(<IntlProvider locale="en"><UnitSuspense {...props} /></IntlProvider>);
const lockPaywall = screen.queryByText('LockPaywall');
expect(lockPaywall).toBeNull();
});
};
describe('gating not enabled', () => { testNoPaywall(); });
Expand All @@ -78,29 +70,29 @@ describe('UnitSuspense component', () => {
describe('gating enabled, gated content included', () => {
beforeEach(() => { mockModels(true, true); });
it('displays LockPaywall in Suspense wrapper with PageLoading fallback', () => {
el = shallow(<UnitSuspense {...props} />);
const [component] = el.instance.findByType(GatedUnitContentMessageSlot);
expect(component.parent.type).toEqual('Suspense');
expect(component.parent.props.fallback)
.toEqual(<PageLoading srMessage={formatMessage(messages.loadingLockedContent)} />);
expect(component.props.courseId).toEqual(props.courseId);
hooks.useShouldDisplayHonorCode.mockReturnValueOnce(false);
render(<IntlProvider locale="en"><UnitSuspense {...props} /></IntlProvider>);
const lockPaywall = screen.getByText('LockPaywall');
expect(lockPaywall).toBeInTheDocument();
const suspenseWrapper = lockPaywall.closest('suspense');
expect(suspenseWrapper).toBeInTheDocument();
});
});
});
describe('HonorCode', () => {
it('does not display HonorCode if useShouldDisplayHonorCode => false', () => {
hooks.useShouldDisplayHonorCode.mockReturnValueOnce(false);
el = shallow(<UnitSuspense {...props} />);
expect(el.instance.findByType(HonorCode).length).toEqual(0);
render(<IntlProvider locale="en"><UnitSuspense {...props} /></IntlProvider>);
const honorCode = screen.queryByText('HonorCode');
expect(honorCode).toBeNull();
});
it('displays HonorCode component in Suspense wrapper with PageLoading fallback if shouldDisplayHonorCode', () => {
hooks.useShouldDisplayHonorCode.mockReturnValueOnce(true);
el = shallow(<UnitSuspense {...props} />);
const [component] = el.instance.findByType(HonorCode);
expect(component.parent.type).toEqual('Suspense');
expect(component.parent.props.fallback)
.toEqual(<PageLoading srMessage={formatMessage(messages.loadingHonorCode)} />);
expect(component.props.courseId).toEqual(props.courseId);
render(<IntlProvider locale="en"><UnitSuspense {...props} /></IntlProvider>);
const honorCode = screen.getByText('HonorCode');
expect(honorCode).toBeInTheDocument();
const suspenseWrapper = honorCode.closest('suspense');
expect(suspenseWrapper).toBeInTheDocument();
});
});
});
Expand Down
Loading