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

fix: enzyme testing replaced by react tester #36159

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
3a3b9b1
fix: enzyme testing replaced by react tester
jciasenza Jan 23, 2025
7ff5004
fix: ESlint failures
jciasenza Jan 23, 2025
dc8557c
fix: remove enzyme dep and fix setupTest
jciasenza Jan 24, 2025
16397a1
Merge branch 'master' into jci/issue35245
jciasenza Jan 24, 2025
3cf6c79
Merge branch 'master' into jci/issue35245
jciasenza Jan 24, 2025
3f800ad
Merge branch 'master' into jci/issue35245
jciasenza Jan 24, 2025
60f6746
Merge branch 'master' into jci/issue35245
jciasenza Jan 27, 2025
34b532e
Merge branch 'master' into jci/issue35245
jciasenza Jan 27, 2025
ac8f8a0
Merge branch 'master' into jci/issue35245
jciasenza Jan 28, 2025
033c973
Merge branch 'master' into jci/issue35245
jciasenza Jan 29, 2025
4fef66f
Merge branch 'master' into jci/issue35245
jciasenza Jan 30, 2025
1a69f00
Merge branch 'master' into jci/issue35245
jciasenza Jan 30, 2025
b7f10bb
Merge branch 'master' into jci/issue35245
jciasenza Jan 31, 2025
f7a4847
Merge branch 'master' into jci/issue35245
jciasenza Jan 31, 2025
4b0f48b
fix: enzyme testing replaced by react tester
jciasenza Feb 3, 2025
9e9f55f
Merge branch 'openedx:master' into jci/issue35245
jciasenza Feb 3, 2025
9ace7e8
Merge branch 'jci/issue35245' of https://github.com/jciasenza/edx-pla…
jciasenza Feb 3, 2025
22320b2
fix: enzyme testing replaced by react tester
jciasenza Feb 3, 2025
6dac593
fix: enzyme testing replaced by react tester
jciasenza Feb 3, 2025
9b659e3
fix: enzyme testing replaced by react tester
jciasenza Feb 3, 2025
6b855af
fix: enzyme testing replaced by react tester
jciasenza Feb 3, 2025
a757c7b
fix: enzyme testing replaced by react tester
jciasenza Feb 3, 2025
b68a54c
Merge branch 'master' into jci/issue35245
jciasenza Feb 4, 2025
4f0da14
Merge branch 'master' into jci/issue35245
jciasenza Feb 5, 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
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
// eslint-disable-next-line no-redeclare
/* global jest,test,describe,expect */
import { Button } from '@edx/paragon';
import BlockBrowserContainer from 'BlockBrowser/components/BlockBrowser/BlockBrowserContainer';
import { Provider } from 'react-redux';
import { shallow } from 'enzyme';
import React from 'react';
import renderer from 'react-test-renderer';
import store from '../../data/store';
import { render, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { Provider } from 'react-redux';

import store from '../../data/store';
import Main from './Main';

describe('ProblemBrowser Main component', () => {
const courseId = 'testcourse';
const problemResponsesEndpoint = '/api/problem_responses/';
const taskStatusEndpoint = '/api/task_status/';
const excludedBlockTypes = [];
const reportDownloadEndpoint = '/api/download_report/';

test('render with basic parameters', () => {
const component = renderer.create(
render(
<Provider store={store}>
<Main
courseId={courseId}
Expand All @@ -28,15 +27,16 @@ describe('ProblemBrowser Main component', () => {
onSelectBlock={jest.fn()}
selectedBlock={null}
taskStatusEndpoint={taskStatusEndpoint}
reportDownloadEndpoint={reportDownloadEndpoint}
ShowBtnUi="false"
/>
</Provider>,
);
const tree = component.toJSON();
expect(tree).toMatchSnapshot();
expect(screen.getByRole('button', { name: 'Select a section or problem' })).toBeInTheDocument();
});
Copy link
Contributor

@brian-smith-tcril brian-smith-tcril Feb 5, 2025

Choose a reason for hiding this comment

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

This test used to use a snapshot. That snapshot resulted in the following output

exports[`ProblemBrowser Main component render with basic parameters 1`] = `
<div
  className="problem-browser-container"
>
  <div
    className="problem-browser"
  >
    <button
      className="btn"
      onBlur={[Function]}
      onClick={[Function]}
      onKeyDown={[Function]}
      type="button"
    >
      Select a section or problem
    </button>
    <input
      disabled={true}
      hidden={false}
      name="problem-location"
      type="text"
      value={null}
    />
    <button
      className="btn"
      name="list-problem-responses-csv"
      onBlur={[Function]}
      onClick={[Function]}
      onKeyDown={[Function]}
      type="button"
    >
      Create a report of problem responses
    </button>
    <div
      aria-live="polite"
      className="report-generation-status"
    />
  </div>
</div>
`;

This test is now only checking for the 'Select a section or problem' button. That is a large change in behavior from before.

To merge this PR this test needs to be updated to test everything it used to be testing.


test('render with selected block', () => {
const component = renderer.create(
render(
<Provider store={store}>
<Main
courseId={courseId}
Expand All @@ -47,16 +47,17 @@ describe('ProblemBrowser Main component', () => {
onSelectBlock={jest.fn()}
selectedBlock="some-selected-block"
taskStatusEndpoint={taskStatusEndpoint}
reportDownloadEndpoint={reportDownloadEndpoint}
ShowBtnUi="false"
/>
</Provider>,
);
const tree = component.toJSON();
expect(tree).toMatchSnapshot();
expect(screen.getByRole('button', { name: 'Select a section or problem' })).toBeInTheDocument();
});
Copy link
Contributor

@brian-smith-tcril brian-smith-tcril Feb 5, 2025

Choose a reason for hiding this comment

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

This test also used to use a snapshot. That snapshot resulted in the following output

exports[`ProblemBrowser Main component render with selected block 1`] = `
<div
  className="problem-browser-container"
>
  <div
    className="problem-browser"
  >
    <button
      className="btn"
      onBlur={[Function]}
      onClick={[Function]}
      onKeyDown={[Function]}
      type="button"
    >
      Select a section or problem
    </button>
    <input
      disabled={true}
      hidden={false}
      name="problem-location"
      type="text"
      value="some-selected-block"
    />
    <button
      className="btn"
      name="list-problem-responses-csv"
      onBlur={[Function]}
      onClick={[Function]}
      onKeyDown={[Function]}
      type="button"
    >
      Create a report of problem responses
    </button>
    <div
      aria-live="polite"
      className="report-generation-status"
    />
  </div>
</div>
`;

This test is now only checking for the 'Select a section or problem' button. That is a large change in behavior from before.

To merge this PR this test needs to be updated to test everything it used to be testing.


test('fetch course block on toggling dropdown', () => {
test('fetch course block on toggling dropdown', async () => {
const fetchCourseBlocksMock = jest.fn();
const component = renderer.create(
render(
<Provider store={store}>
<Main
courseId={courseId}
Expand All @@ -67,30 +68,67 @@ describe('ProblemBrowser Main component', () => {
onSelectBlock={jest.fn()}
selectedBlock="some-selected-block"
taskStatusEndpoint={taskStatusEndpoint}
reportDownloadEndpoint={reportDownloadEndpoint}
ShowBtnUi="false"
/>
</Provider>,
);
// eslint-disable-next-line prefer-destructuring
const instance = component.root.children[0].instance;
instance.handleToggleDropdown();
expect(fetchCourseBlocksMock.mock.calls.length).toBe(1);
const toggleButton = screen.getByRole('button', { name: 'Select a section or problem' });
await userEvent.click(toggleButton);
expect(fetchCourseBlocksMock).toHaveBeenCalledTimes(1);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉 This test does a perfect job of replicating the behavior from before using RTL! 🎉


test('display dropdown on toggling dropdown', () => {
const component = shallow(
<Main
courseId={courseId}
createProblemResponsesReportTask={jest.fn()}
excludeBlockTypes={excludedBlockTypes}
fetchCourseBlocks={jest.fn()}
problemResponsesEndpoint={problemResponsesEndpoint}
onSelectBlock={jest.fn()}
selectedBlock="some-selected-block"
taskStatusEndpoint={taskStatusEndpoint}
/>,
test('display dropdown on toggling dropdown', async () => {
brian-smith-tcril marked this conversation as resolved.
Show resolved Hide resolved
render(
<Provider store={store}>
<Main
courseId={courseId}
createProblemResponsesReportTask={jest.fn()}
excludeBlockTypes={excludedBlockTypes}
fetchCourseBlocks={jest.fn()}
problemResponsesEndpoint={problemResponsesEndpoint}
onSelectBlock={jest.fn()}
selectedBlock="some-selected-block"
taskStatusEndpoint={taskStatusEndpoint}
reportDownloadEndpoint={reportDownloadEndpoint}
ShowBtnUi="false"
/>
</Provider>,
);
expect(component.find(BlockBrowserContainer).length).toBeFalsy();
component.find(Button).find({ label: 'Select a section or problem' }).simulate('click');
expect(component.find(BlockBrowserContainer).length).toBeTruthy();

expect(screen.queryByText('Some expected block name')).toBeNull();
const toggleButton = screen.getByRole('button', { name: 'Select a section or problem' });
await userEvent.click(toggleButton);
const blockName = screen.queryByText('Some expected block name');
expect(blockName).toBeNull();
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems a bit odd to me, it's expecting the query for 'Some expected block name' to be null both before and after clicking the toggle button?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if the way I modified it is right, but the tests passed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if the way I modified it is right, but the tests passed.

});

test('hide dropdown on second toggle', async () => {
render(
<Provider store={store}>
<Main
courseId={courseId}
createProblemResponsesReportTask={jest.fn()}
excludeBlockTypes={excludedBlockTypes}
fetchCourseBlocks={jest.fn()}
problemResponsesEndpoint={problemResponsesEndpoint}
onSelectBlock={jest.fn()}
selectedBlock="some-selected-block"
taskStatusEndpoint={taskStatusEndpoint}
reportDownloadEndpoint={reportDownloadEndpoint}
ShowBtnUi="false"
/>
</Provider>,
);
const toggleButton = screen.getByRole('button', { name: 'Select a section or problem' });

await userEvent.click(toggleButton);
await waitFor(() => {
expect(screen.queryByText('Select a section or problem')).not.toBeNull();
});
await userEvent.click(toggleButton);
await waitFor(() => {
expect(screen.queryByText('Some expected block name')).toBeNull();
});
});
});

This file was deleted.

Loading
Loading