Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
feat: refactor tests
  • Loading branch information
vladislavkeblysh authored and filippovskii09 committed Oct 23, 2025
commit 0bda1d03c5f74c1ea59daf7a842141619e493e2c
77 changes: 77 additions & 0 deletions src/course-home/live-tab/LiveTab.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import React from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this import?

Copy link
Author

Choose a reason for hiding this comment

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

removed

import { render } from '@testing-library/react';
Copy link
Contributor

Choose a reason for hiding this comment

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

[question]: Is it possible to use function render from file setupTests https://github.com/openedx/frontend-app-learning/blob/master/src/setupTest.js#L222 in these tests?

Copy link
Author

Choose a reason for hiding this comment

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

yes, fixed, i used render from setupTest

import { useSelector } from 'react-redux';
import LiveTab from './LiveTab';

jest.mock('react-redux', () => ({
useSelector: jest.fn(),
}));

describe('LiveTab', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why we are adding these tests to this PR?
It looks like we are proposing accessibility improvements in this PR, but we are writing tests for the functionality of the LiveTab component.

Copy link
Author

@filippovskii09 filippovskii09 Oct 24, 2025

Choose a reason for hiding this comment

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

These tests were cherry-picked along with the related changes from a previously closed PR.
The tests themselves are not directly related to the accessibility improvements.
I can separate them if needed, but for now, they ensure we don’t lose existing coverage.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but these tests seem to test functionality that is not relevant to this PR. Will we have coverage issues if we remove them?

Copy link
Author

Choose a reason for hiding this comment

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

deleted test and no problem occurred

afterEach(() => {
jest.clearAllMocks();
document.body.innerHTML = '';
});

it('renders iframe from liveModel using dangerouslySetInnerHTML', () => {
useSelector.mockImplementation((selector) => selector({
courseHome: { courseId: 'course-v1:test+id+2024' },
models: {
live: {
'course-v1:test+id+2024': {
iframe: '<iframe id="lti-tab-embed" src="about:blank"></iframe>',
},
},
},
}));

render(<LiveTab />);

const iframe = document.getElementById('lti-tab-embed');
expect(iframe).toBeInTheDocument();
expect(iframe.src).toBe('about:blank');
});

it('adds classes to iframe after mount', () => {
document.body.innerHTML = `
<div id="live_tab">
<iframe id="lti-tab-embed" class=""></iframe>
</div>
`;

useSelector.mockImplementation((selector) => selector({
courseHome: { courseId: 'course-v1:test+id+2024' },
models: {
live: {
'course-v1:test+id+2024': {
iframe: '<iframe id="lti-tab-embed"></iframe>',
},
},
},
}));

render(<LiveTab />);

const iframe = document.getElementById('lti-tab-embed');
expect(iframe.className).toContain('vh-100');
expect(iframe.className).toContain('w-100');
expect(iframe.className).toContain('border-0');
});

it('does not throw if iframe is not found in DOM', () => {
useSelector.mockImplementation((selector) => selector({
courseHome: { courseId: 'course-v1:test+id+2024' },
models: {
live: {
'course-v1:test+id+2024': {
iframe: '<div>No iframe here</div>',
},
},
},
}));

expect(() => render(<LiveTab />)).not.toThrow();
const iframe = document.getElementById('lti-tab-embed');
expect(iframe).toBeNull();
});
});
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import React from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this import?

Copy link
Author

Choose a reason for hiding this comment

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

no, removed

import {
render, screen, waitFor,
} from '@testing-library/react';
import { render, screen, waitFor } from '@testing-library/react';
Copy link
Contributor

Choose a reason for hiding this comment

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

[question]: Is it possible to use function render from file setupTests https://github.com/openedx/frontend-app-learning/blob/master/src/setupTest.js#L222 in these tests?

Copy link
Author

Choose a reason for hiding this comment

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

yes, fixed, i used render from setupTest

import userEvent from '@testing-library/user-event';
import { useSelector } from 'react-redux';
import { IntlProvider } from 'react-intl';

import { fireEvent } from '@testing-library/dom';
import GradeSummaryHeader from './GradeSummaryHeader';
import { useModel } from '../../../../generic/model-store';
import messages from '../messages';
Expand All @@ -18,6 +17,10 @@ jest.mock('../../../../generic/model-store', () => ({
useModel: jest.fn(),
}));

jest.mock('../../../../data/hooks', () => ({
useContextId: () => 'test-course-id',
}));

describe('GradeSummaryHeader', () => {
beforeEach(() => {
useSelector.mockImplementation((selector) => selector({
Expand All @@ -30,25 +33,95 @@ describe('GradeSummaryHeader', () => {
render(
<IntlProvider locale="en" messages={messages}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this extra IntlProvider?

Copy link
Author

Choose a reason for hiding this comment

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

no, removed

<GradeSummaryHeader
intl={{ formatMessage: jest.fn((msg) => msg.defaultMessage) }}
allOfSomeAssignmentTypeIsLocked={false}
{...props}
/>
</IntlProvider>,
);
};

it('visible the tooltip when Escape is pressed', async () => {
it('shows tooltip on icon button click', async () => {
renderComponent();

const iconButton = screen.getByRole('button', {
name: messages.gradeSummaryTooltipAlt.defaultMessage,
});

await userEvent.click(iconButton);

await waitFor(() => {
expect(screen.getByText(messages.gradeSummaryTooltipBody.defaultMessage)).toBeInTheDocument();
});
});

it('hides tooltip on mouse out', async () => {
renderComponent();

const iconButton = screen.getByRole('button', {
name: messages.gradeSummaryTooltipAlt.defaultMessage,
});

userEvent.click(iconButton);
fireEvent.mouseOver(iconButton);

await waitFor(() => {
expect(screen.getByText(messages.gradeSummaryTooltipBody.defaultMessage)).toBeVisible();
});

fireEvent.mouseOut(iconButton);

await waitFor(() => {
expect(screen.queryByText(messages.gradeSummaryTooltipBody.defaultMessage)).toBeNull();
});
});

it('hides tooltip on blur', async () => {
renderComponent();

const iconButton = screen.getByRole('button', {
name: messages.gradeSummaryTooltipAlt.defaultMessage,
});

await userEvent.hover(iconButton);
await userEvent.click(iconButton);

await waitFor(() => {
expect(screen.getByText(messages.gradeSummaryTooltipBody.defaultMessage)).toBeInTheDocument();
});

const blurTarget = document.createElement('button');
blurTarget.textContent = 'Outside';
document.body.appendChild(blurTarget);
blurTarget.focus();

await userEvent.unhover(iconButton);

await waitFor(() => {
expect(screen.queryByText(messages.gradeSummaryTooltipBody.defaultMessage)).not.toBeInTheDocument();
});

document.body.removeChild(blurTarget);
});

it('hides tooltip when Escape is pressed (covers handleKeyDown)', async () => {
renderComponent();

const iconButton = screen.getByRole('button', {
name: messages.gradeSummaryTooltipAlt.defaultMessage,
});

await userEvent.hover(iconButton);
await userEvent.click(iconButton);

await waitFor(() => {
expect(screen.getByText(messages.gradeSummaryTooltipBody.defaultMessage)).toBeInTheDocument();
});

fireEvent.keyDown(iconButton, { key: 'Escape', code: 'Escape' });
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

fixed


await userEvent.unhover(iconButton);

await waitFor(() => {
expect(screen.queryByText(messages.gradeSummaryTooltipBody.defaultMessage)).not.toBeInTheDocument();
});
});
});