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

Renu - Workshop Feedback #8

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
38 changes: 30 additions & 8 deletions src/challenges/challengeOne/1.Accordion/Accordion.test.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,30 @@
// import React from "react";
// import { render, screen } from "@testing-library/react";
// import userEvent from "@testing-library/user-event";
// import { Accordion } from "./Accordion";

// describe("Accordion", () => {
// it("renders...", () => {});
// });
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { Accordion, AccordionProps } from "./Accordion";

const renderComponent = (additionalProps?: Partial<AccordionProps>) =>{
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd always put the renderComponet function inside the describe block. Makes it easier to understand what's going on when it's together.

const defaultProps = {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm surprised you aren't getting a type error when you try and pass this object into the Accordion component.

But I just checked and nothing comes up. I thought you had to do:

const defaultProps: AccordionProps =

title: "My title",
children: <h1>Accordion content</h1>
}

render(<Accordion {...defaultProps} {...additionalProps}/>)
}

describe("Accordion", () => {
it("renders the button correctly", () => {
renderComponent();
expect(screen.getByRole("button", {name: "My title"})).toBeInTheDocument();
});

it("renders the children correctly", async () => {
renderComponent();
userEvent.click(screen.getByRole("button", {name: "My title"}))
expect(await screen.findByRole("heading", {name: "Accordion content"})).toBeInTheDocument();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This works, but I would have personally gone findByText on this one. Usually, findByRole is the better option but you're more concerned the text/content is right, not the element itself.

Still, keep it as is. It's good work.

});

it("does not render the children unless the button in clicked", async () => {
renderComponent();
expect(screen.queryByRole("heading", {name: "Accordion content"})).not.toBeInTheDocument();
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great test file, the only test case I would add would be to test the content is hidden after a second click.

});
39 changes: 31 additions & 8 deletions src/challenges/challengeOne/2.Dropdown/Dropdown.test.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,31 @@
// import React from "react";
// import { render, screen } from "@testing-library/react";
// import userEvent from "@testing-library/user-event";
// import { Dropdown } from "./Dropdown";

// describe("Dropdown", () => {
// it("renders...", () => {});
// });
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { Dropdown, DropdownProps } from "./Dropdown";

const renderComponent = (additionalProps?: Partial<DropdownProps>) => {
const defaultProps = {
options: ["Option 1", "Option 2", "Option 3"],
label: "Label",
id: "id",
};

render(<Dropdown {...defaultProps} {...additionalProps} />);
};

describe("Dropdown", () => {
it("renders the label correctly", () => {
renderComponent();
expect(screen.getByText("Label")).toBeInTheDocument();
});

it("selects the first option in the list by default", () => {
renderComponent();
expect(screen.getByRole("menu")).toHaveValue("Option 1");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test is failing with:

TestingLibraryElementError: Unable to find an accessible element with the role "menu"

Don't you want combobox?

  it("selects the first option by default", () => {
    renderComponent();
    expect(screen.getByRole("combobox")).toHaveValue("Option 1");
  });

This passes

});

it("selects the options correctly", () => {
renderComponent();
userEvent.click(screen.getByRole("menuitem", { name: "Option 2" }));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same issue as above.

Since this is a native select, you needed to use userEvent.selectOptions to change the dropdown.

  it("allows a user to select an option", async () => {
    renderComponent();

    expect(screen.getByRole("combobox")).toHaveValue("Option 1");

    await userEvent.selectOptions(screen.getByRole("combobox"), "Option 2");

    expect(screen.getByRole("combobox")).toHaveValue("Option 2");
  });

expect(screen.getByRole("menu")).toHaveValue("Option 2");
});
});
46 changes: 38 additions & 8 deletions src/challenges/challengeOne/3.Table/Table.test.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,38 @@
// import React from "react";
// import { render, screen } from "@testing-library/react";
// import userEvent from "@testing-library/user-event";
// import { Table } from "./Table";

// describe("Table", () => {
// it("renders...", () => {});
// });
import { render, screen } from "@testing-library/react";
import { Table, TableProps } from "./Table";

const renderComponent = (additionalProps?: Partial<TableProps>) => {
const defaultProps = {
columns: [
{ property: "name", label: "Name" },
{ property: "email", label: "Email" },
],
data: [
{ id: 1, name: "John Doe", email: "[email protected]" },
{ id: 2, name: "Jane Doe", email: "[email protected]" },
]
};

render(<Table {...defaultProps} {...additionalProps} />);
};

describe("Table", () => {
it("renders the table headings correctly", () => {
renderComponent();
expect(screen.getByRole("columnheader", { name: "Name" })).toBeInTheDocument();
expect(screen.getByRole("columnheader", { name: "Email" })).toBeInTheDocument();
});

it("renders the names correctly", () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Your tests pass, but the tests are very related to your mocks. This is a reusable component so the tests should reflect that.

A better second test would have simply been:

it("renders the data correctly", () => ...)

renderComponent();
expect(screen.getByRole("cell", { name: "John Doe" })).toBeInTheDocument();
expect(screen.getByRole("cell", { name: "Jane Doe" })).toBeInTheDocument();
});

it("renders the emails correctly", () => {
renderComponent();
expect(screen.getByRole("cell", { name: "[email protected]" })).toBeInTheDocument();
expect(screen.getByRole("cell", { name: "[email protected]" })).toBeInTheDocument();
});
});

71 changes: 63 additions & 8 deletions src/challenges/challengeOne/4.Form/Form.test.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,63 @@
// import React from "react";
// import { render, screen } from "@testing-library/react";
// import userEvent from "@testing-library/user-event";
// import { Form } from "./Form";

// describe("Form", () => {
// it("renders...", () => {});
// });
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { Form, FormProps } from "./Form";

const handleFormSubmitMock = jest.fn();

const renderComponent = (additionalProps?: Partial<FormProps>) => {
const defaultProps = {
fields: [
{
label: "Name",
type: "text",
name: "name",
placeholder: "Enter your name",
value: "",
id: "name",
},
{
label: "Email",
type: "email",
name: "email",
placeholder: "Enter your email",
value: "",
id: "email",
},
],
onSubmit: handleFormSubmitMock
};

render(<Form {...defaultProps} {...additionalProps} />);
};

describe("Form", () => {
it("renders the input label correctly", () => {
renderComponent();
expect(screen.getByText("Name")).toBeInTheDocument();
});

it("renders the placeholder correctly", () => {
renderComponent();
expect(screen.getByPlaceholderText("Enter your email")).toBeInTheDocument();
});

it("updates the input box correctly", async () => {
renderComponent();
userEvent.type(screen.getByRole("textbox", {name: "Name"}), "Renu")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test is failing because you aren't awaiting the userEvent.type to actually type into your input.

All you needed to do is this:

Suggested change
userEvent.type(screen.getByRole("textbox", {name: "Name"}), "Renu")
await userEvent.type(screen.getByRole("textbox", {name: "Name"}), "Renu")

expect(await screen.findByRole("textbox", {name: "Name"})).toHaveValue("Renu");
});

it("renders the submit button correctly", () => {
renderComponent();
expect(screen.getByRole("button", {name: "Submit"})).toBeInTheDocument();
});

it("calls the on submit function when the button is clicked", () => {
renderComponent();
userEvent.type(screen.getByRole("textbox", {name: "Name"}), "Renu")
userEvent.type(screen.getByRole("textbox", {name: "Name"}), "[email protected]")
userEvent.click(screen.getByRole("button", {name: "Submit"}));
expect(handleFormSubmitMock).toHaveBeenCalledWith({name: "[email protected]"})
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test was a real shame, as it was clearly rushed. Really look at your test Renu. Take the time to read and absorb the consequences of going too fast. Read every line before looking below.

When you rush, you make mistakes. In this test, you made a number of mistakes.

Compare your test to mine below

  it("calls the onSubmit function with the correct data when the form is filled in and the submit button is clicked", async () => {
    renderComponent();
    await userEvent.type(screen.getByRole("textbox", { name: "Name" }), "Renu");
    await userEvent.type(
      screen.getByRole("textbox", { name: "Email" }),
      "[email protected]",
    );
    await userEvent.click(screen.getByRole("button", { name: "Submit" }));
    expect(handleFormSubmitMock).toHaveBeenCalledWith({
      name: "Renu",
      email: "[email protected]",
    });
  });

});
});

58 changes: 50 additions & 8 deletions src/challenges/challengeOne/5.Modal/Modal.test.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,50 @@
// import React from "react";
// import { render, screen } from "@testing-library/react";
// import userEvent from "@testing-library/user-event";
// import { Modal } from "./Modal";

// describe("Modal", () => {
// it("renders...", () => {});
// });
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { Modal, ModalProps } from "./Modal";

const handleCloseMock = jest.fn();
const handleConfirmMock = jest.fn();

const renderComponent = (additionalProps?: Partial<ModalProps>) => {
const defaultProps = {
isOpen: true,
onClose: handleCloseMock,
headerText: "Form Modal",
onConfirm: handleConfirmMock,
showActions: false,
children: <h1>Hello world</h1>
};

render(<Modal {...defaultProps} {...additionalProps} />);
};

describe("Modal", () => {
it("opens the modal correctly", () => {
renderComponent();
expect(screen.getByRole("heading", {name: "Form Modal"})).toBeInTheDocument();
});

it("hides the modal correctly", () => {
renderComponent({isOpen: false});
expect(screen.queryByRole("heading", {name: "Form Modal"})).not.toBeInTheDocument();
});

it("calls the on close function correctly", () => {
renderComponent();
userEvent.click(screen.getByRole("button"));
expect(handleCloseMock).toHaveBeenCalled()
});

it("renders the cancel button correctly", () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're not testing it renders, you're testing the onClose is called when clicked.

renderComponent({showActions: true});
userEvent.click(screen.getByRole("button", {name: "Cancel"}));
expect(handleCloseMock).toHaveBeenCalled()
});

it("renders the confirm button correctly", () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Again you're testing the handleConfirm is being called.

renderComponent({showActions: true});
userEvent.click(screen.getByRole("button", {name: "Confirm"}));
expect(handleConfirmMock).toHaveBeenCalled()
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These last three tests are all failing without async await

});

2 changes: 1 addition & 1 deletion src/challenges/challengeOne/5.Modal/Modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export function Modal({
<div className="modal w-[300px] rounded-md bg-white p-6">
<div className="flex items-center justify-between pb-6">
<h2 className="text-lg font-semibold">{headerText}</h2>
<button onClick={onClose} type="button">
<button onClick={onClose} type="button" title="close-modal-button">
<svg
xmlns="http://www.w3.org/2000/svg"
className="h-6 w-6"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,54 @@
// import React from "react";
// import { render, screen } from "@testing-library/react";
// import userEvent from "@testing-library/user-event";
// import { IntegrationComponent } from "./IntegrationComponent";

// describe("IntegrationComponent", () => {
// it("renders...", () => {});
// });
import { render, screen, waitForElementToBeRemoved } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import axios from "axios";
import { IntegrationComponent } from "./IntegrationComponent";

const WEATHER_DATA_MOCK = {
temperature: "30C",
weather: "Cloudy"
};

jest.mock("axios");

const renderComponent = () => {
render(<IntegrationComponent />);
};

describe("Modal", () => {
beforeEach(() => {
const mockedAxios = axios as jest.Mocked<typeof axios>;
mockedAxios.get.mockResolvedValueOnce({data: WEATHER_DATA_MOCK});
});

it("render the 'Open Modal' button correctly", async () => {
renderComponent();
expect(await screen.findByRole("button", {name: "Open Form"})).toBeInTheDocument();
});

it("opens the modal correctly", async () => {
renderComponent();
userEvent.click(await screen.findByRole("button", {name: "Open Form"}))
expect(await screen.findByRole("heading", {name: "Form Modal"})).toBeInTheDocument();
});

it("closes the modal when the cancel icon is clicked", async () => {
renderComponent();
userEvent.click(await screen.findByRole("button", {name: "Open Form"}))
userEvent.click(await screen.findByTitle("close-modal-button"));
await waitForElementToBeRemoved(() => screen.queryByRole("heading", {name: "Form Modal"}))
});

it("displays the toast message when the form is submitted", async () => {
renderComponent();
userEvent.click(await screen.findByRole("button", {name: "Open Form"}))
userEvent.type(await screen.findByRole("textbox", {name: "Name"}), "Renu")
userEvent.type(await screen.findByRole("textbox", {name: "Email"}), "[email protected]")
userEvent.click(screen.getByRole("button", {name: "Submit"}));
expect(await screen.findByText("Hello Renu, your email [email protected] has been submitted!"))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test is failing as you're not waiting for the userEvent.type to input your text. You're also using findBy when you should be using getBy.

When you aren't waiting for data to come into your component for it to render, it will always be available, it's only when you are waiting for something async to happen so the content changes that you need findBy

  it("displays the toast message when the form is submitted", async () => {
    renderComponent();
    await userEvent.click(screen.getByRole("button", { name: "Open Form" }));

    await userEvent.type(screen.getByRole("textbox", { name: "Name" }), "Renu");
    await userEvent.type(
      screen.getByRole("textbox", { name: "Email" }),
      "[email protected]",
    );

    await userEvent.click(screen.getByRole("button", { name: "Submit" }));

    expect(
      await screen.findByText(
        "Hello Renu, your email [email protected] has been submitted!",
      ),
    );
  });

});

it("displays the weather data correctly", async () => {
renderComponent();
expect(await screen.findByText("Temperature: 30C")).toBeInTheDocument();
});
});
28 changes: 14 additions & 14 deletions src/pages/index.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
// import React, { useState } from "react";
import React, { useState } from "react";
import Head from "next/head";

// import { Accordion } from "@/challenges/challengeOne/1.Accordion/Accordion";
// import { Dropdown } from "@/challenges/challengeOne/2.Dropdown/Dropdown";
// import { Table } from "@/challenges/challengeOne/3.Table/Table";
// import { Form } from "@/challenges/challengeOne/4.Form/Form";
// import { Modal } from "@/challenges/challengeOne/5.Modal/Modal";
// import { IntegrationComponent } from "@/challenges/challengeTwo/IntegrationComponent/IntegrationComponent";
import { Accordion } from "@/challenges/challengeOne/1.Accordion/Accordion";
import { Dropdown } from "@/challenges/challengeOne/2.Dropdown/Dropdown";
import { Table } from "@/challenges/challengeOne/3.Table/Table";
import { Form } from "@/challenges/challengeOne/4.Form/Form";
import { Modal } from "@/challenges/challengeOne/5.Modal/Modal";
import { IntegrationComponent } from "@/challenges/challengeTwo/IntegrationComponent/IntegrationComponent";

export default function Home() {
// const [isOpen, setIsOpen] = useState(true);
const [isOpen, setIsOpen] = useState(true);

// const handleModalClose = () => {
// setIsOpen(false);
// };
const handleModalClose = () => {
setIsOpen(false);
};

return (
<>
Expand All @@ -26,7 +26,7 @@ export default function Home() {
<main>
<div className="mx-auto flex h-full w-full flex-col items-center gap-3 p-40">
<h1 className="text-4xl font-bold">Testing Workshop</h1>
{/* <Accordion title="Accordion title">
<Accordion title="Accordion title">
<p>Accordion content</p>
</Accordion>

Expand Down Expand Up @@ -76,8 +76,8 @@ export default function Home() {
{ id: 1, name: "John Doe", email: "[email protected]" },
{ id: 2, name: "Jane Doe", email: "[email protected]" },
]}
/> */}
{/* <IntegrationComponent /> */}
/>
<IntegrationComponent />
</div>
</main>
</>
Expand Down