From cd653b6ddf5bd2d88d6e35e5539b3c3bdabc0f65 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 24 Jul 2023 14:41:40 -0700 Subject: [PATCH 01/60] Bump word-wrap from 1.2.3 to 1.2.5 in /component-lib (#7066) Bumps [word-wrap](https://github.com/jonschlinkert/word-wrap) from 1.2.3 to 1.2.5. - [Release notes](https://github.com/jonschlinkert/word-wrap/releases) - [Commits](https://github.com/jonschlinkert/word-wrap/compare/1.2.3...1.2.5) --- updated-dependencies: - dependency-name: word-wrap dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- component-lib/yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/component-lib/yarn.lock b/component-lib/yarn.lock index 74bde065fc7d..cb080aa01d0f 100644 --- a/component-lib/yarn.lock +++ b/component-lib/yarn.lock @@ -4103,9 +4103,9 @@ which@^2.0.1: isexe "^2.0.0" word-wrap@~1.2.3: - version "1.2.3" - resolved "https://registry.yarnpkg.com/word-wrap/-/word-wrap-1.2.3.tgz#610636f6b1f703891bd34771ccb17fb93b47079c" - integrity sha512-Hz/mrNwitNRh/HUAtM/VT/5VH+ygD6DV7mYKZAtHOrbs8U7lvPS6xf7EJKMF0uW1KJCl0H701g3ZGus+muE5vQ== + version "1.2.5" + resolved "https://registry.yarnpkg.com/word-wrap/-/word-wrap-1.2.5.tgz#d2c45c6dd4fbce621a66f136cbe328afd0410b34" + integrity sha512-BN22B5eaMMI9UMtjrGd5g5eCYPpCPDUy0FJXbYsaT5zYxjFOckS53SQDE3pWkVoWpHXVb3BrYcEN4Twa55B5cA== wordwrapjs@^4.0.0: version "4.0.1" From 0ae6c716a2b6bc770568eb34b907deb5f1ff1e25 Mon Sep 17 00:00:00 2001 From: Maya Barnes <63436329+mayagbarnes@users.noreply.github.com> Date: Mon, 24 Jul 2023 18:39:51 -0700 Subject: [PATCH 02/60] Add RTL Rules to eslint config (#6998) Adding the recommended rules/configuration 'plugin:testing-library/react' to our ESLint config to enforce recommended practices. Resolves all outstanding warnings/errors from these rules. --- frontend/.eslintrc.js | 2 + frontend/app/src/App.test.tsx | 14 +- .../src/components/AppView/AppView.test.tsx | 27 ++- .../src/components/MainMenu/MainMenu.test.tsx | 158 ++++++------- .../src/components/Sidebar/Sidebar.test.tsx | 28 +-- .../core/Block/ElementNodeRenderer.test.tsx | 65 +++--- .../core/Block/ElementNodeRenderer.tsx | 3 + .../elements/ChatMessage/ChatMessage.test.tsx | 54 +++-- .../elements/ChatMessage/ChatMessage.tsx | 18 +- .../elements/DataFrame/DataFrame.tsx | 3 + .../FullScreenWrapper.test.tsx | 18 +- .../InputInstructions.test.tsx | 42 ++-- .../StreamlitMarkdown.test.tsx | 99 ++++---- .../widgets/BaseWidget/WidgetLabel.tsx | 1 + .../__snapshots__/WidgetLabel.test.tsx.snap | 1 + .../widgets/ChatInput/ChatInput.test.tsx | 211 +++++++++--------- .../widgets/ChatInput/ChatInput.tsx | 4 +- .../widgets/DataFrame/Tooltip.test.tsx | 4 +- .../components/widgets/DataFrame/Tooltip.tsx | 1 + .../columns/cells/DateTimeCell.test.tsx | 45 ++-- .../widgets/DateInput/DateInput.test.tsx | 9 +- .../widgets/Form/FormSubmitButton.test.tsx | 10 +- .../widgets/TimeInput/TimeInput.test.tsx | 96 ++++---- .../widgets/TimeInput/TimeInput.tsx | 4 +- .../lib/src/hooks/useScrollAnimation.test.ts | 22 +- 25 files changed, 471 insertions(+), 468 deletions(-) diff --git a/frontend/.eslintrc.js b/frontend/.eslintrc.js index d72239b1c92a..b92bd54dbdb0 100644 --- a/frontend/.eslintrc.js +++ b/frontend/.eslintrc.js @@ -36,6 +36,8 @@ module.exports = { "plugin:prettier/recommended", // Recommended Jest configuration to enforce good testing practices "plugin:jest/recommended", + // Uses the recommended rules from React Testing Library: + "plugin:testing-library/react", ], // Specifies the ESLint parser parser: "@typescript-eslint/parser", diff --git a/frontend/app/src/App.test.tsx b/frontend/app/src/App.test.tsx index 7a1667dc8c34..f3ada8843673 100644 --- a/frontend/app/src/App.test.tsx +++ b/frontend/app/src/App.test.tsx @@ -16,7 +16,7 @@ import React from "react" import { ReactWrapper, ShallowWrapper } from "enzyme" -import { waitFor } from "@testing-library/dom" +import { waitFor } from "@testing-library/react" import cloneDeep from "lodash/cloneDeep" import { LocalStore, @@ -931,15 +931,14 @@ describe("App.onHistoryChange", () => { pushStateSpy.mockRestore() }) - it("does rerun when we are navigating to a different page and the last window history url contains an anchor", () => { + it("does rerun when we are navigating to a different page and the last window history url contains an anchor", async () => { const pushStateSpy = jest.spyOn(window.history, "pushState") - - jest.spyOn(instance, "onPageChange") + const pageChangeSpy = jest.spyOn(instance, "onPageChange") // navigate to current page with anchor window.history.pushState({}, "", "#foo_bar") instance.onHistoryChange() - expect(instance.onPageChange).not.toHaveBeenCalled() + expect(pageChangeSpy).not.toHaveBeenCalled() // navigate to new page instance.handleNewSession( @@ -947,8 +946,9 @@ describe("App.onHistoryChange", () => { ) window.history.back() - waitFor(() => { - expect(instance.onPageChange).toHaveBeenLastCalledWith("sub_hash") + // Check for rerun + await waitFor(() => { + expect(pageChangeSpy).toHaveBeenLastCalledWith("top_hash") }) pushStateSpy.mockRestore() diff --git a/frontend/app/src/components/AppView/AppView.test.tsx b/frontend/app/src/components/AppView/AppView.test.tsx index f19d5dd78f74..7e4ae4d4658f 100644 --- a/frontend/app/src/components/AppView/AppView.test.tsx +++ b/frontend/app/src/components/AppView/AppView.test.tsx @@ -15,8 +15,8 @@ */ import React from "react" -import { screen } from "@testing-library/react" import "@testing-library/jest-dom" +import { screen } from "@testing-library/react" import { AppContext, Props as AppContextProps, @@ -206,9 +206,11 @@ describe("AppView element", () => { const props = getProps({ elements: new AppRoot(new BlockNode([main, sidebar, event])), }) - const { getByTestId } = render() + render() - const style = window.getComputedStyle(getByTestId("block-container")) + const style = window.getComputedStyle( + screen.getByTestId("block-container") + ) expect(style.maxWidth).not.toEqual("initial") }) @@ -221,9 +223,10 @@ describe("AppView element", () => { return realUseContext(input) }) - const { getByTestId } = render() - const style = window.getComputedStyle(getByTestId("block-container")) - + render() + const style = window.getComputedStyle( + screen.getByTestId("block-container") + ) expect(style.maxWidth).toEqual("initial") }) @@ -244,10 +247,10 @@ describe("AppView element", () => { return realUseContext(input) }) - const { getByRole, getByTestId } = render() + render() - expect(getByTestId("AppViewBlockSpacer")).toBeInTheDocument() - expect(getByRole("contentinfo")).toBeInTheDocument() + expect(screen.getByTestId("AppViewBlockSpacer")).toBeInTheDocument() + expect(screen.getByRole("contentinfo")).toBeInTheDocument() }) it("does not render the Spacer and Footer when embedded", () => { @@ -260,10 +263,10 @@ describe("AppView element", () => { return realUseContext(input) }) - const { queryByRole, queryByTestId } = render() + render() - expect(queryByTestId("AppViewBlockSpacer")).not.toBeInTheDocument() - expect(queryByRole("contentinfo")).not.toBeInTheDocument() + expect(screen.queryByTestId("AppViewBlockSpacer")).not.toBeInTheDocument() + expect(screen.queryByRole("contentinfo")).not.toBeInTheDocument() }) describe("when window.location.hash changes", () => { diff --git a/frontend/app/src/components/MainMenu/MainMenu.test.tsx b/frontend/app/src/components/MainMenu/MainMenu.test.tsx index 1d47bab99106..41a94f8f2078 100644 --- a/frontend/app/src/components/MainMenu/MainMenu.test.tsx +++ b/frontend/app/src/components/MainMenu/MainMenu.test.tsx @@ -15,7 +15,14 @@ */ import React from "react" - +import "@testing-library/jest-dom" +import { + screen, + waitFor, + fireEvent, + RenderResult, + Screen, +} from "@testing-library/react" import { mount, render, @@ -35,8 +42,6 @@ import { import { SegmentMetricsManager } from "@streamlit/app/src/SegmentMetricsManager" import MainMenu, { Props } from "./MainMenu" -import { waitFor } from "@testing-library/dom" -import { fireEvent, RenderResult } from "@testing-library/react" const { GitStates } = GitInfo @@ -64,9 +69,11 @@ const getProps = (extend?: Partial): Props => ({ ...extend, }) -async function openMenu(wrapper: RenderResult): Promise { - fireEvent.click(wrapper.getByRole("button")) - await waitFor(() => expect(wrapper.findByRole("listbox")).toBeDefined()) +async function openMenu(screen: Screen): Promise { + fireEvent.click(screen.getByRole("button")) + // Each SubMenu is a listbox, so need to use findAllByRole (findByRole throws error if multiple matches) + const menu = await screen.findAllByRole("listbox") + expect(menu).toBeDefined() } function getMenuStructure( @@ -117,61 +124,60 @@ describe("MainMenu", () => { const props = getProps({ hostMenuItems: items, }) - const wrapper = render() - await openMenu(wrapper) + render() + await openMenu(screen) - await waitFor(() => - expect( - wrapper - .getAllByRole("option") - .map(item => item.querySelector("span:first-of-type")?.textContent) - ).toEqual([ - "Rerun", - "Settings", - "Print", - "Record a screencast", - "View app source", - "Report bug with app", - "About", - "Developer options", - "Clear cache", - ]) - ) + const menuOptions = await screen.findAllByRole("option") + + const expectedLabels = [ + "Rerun", + "Settings", + "Print", + "Record a screencast", + "View app source", + "Report bug with app", + "About", + "Developer options", + "Clear cache", + ] + + expectedLabels.forEach((label, index) => { + expect(menuOptions[index]).toHaveTextContent(label) + }) }) it("should render core set of menu elements", async () => { const props = getProps() - const wrapper = render() - await openMenu(wrapper) + render() + await openMenu(screen) - await waitFor(() => - expect( - wrapper - .getAllByRole("option") - .map(item => item.querySelector("span:first-of-type")?.textContent) - ).toEqual([ - "Rerun", - "Settings", - "Print", - "Record a screencast", - "About", - "Developer options", - "Clear cache", - "Deploy this app", - ]) - ) + const menuOptions = await screen.findAllByRole("option") + + const expectedLabels = [ + "Rerun", + "Settings", + "Print", + "Record a screencast", + "About", + "Developer options", + "Clear cache", + "Deploy this app", + ] + + expectedLabels.forEach((label, index) => { + expect(menuOptions[index]).toHaveTextContent(label) + }) }) it("should render deploy app menu item", async () => { const props = getProps({ gitInfo: {} }) - const wrapper = render() - await openMenu(wrapper) + render() + await openMenu(screen) - await waitFor(() => - expect( - wrapper.getByRole("option", { name: "Deploy this app" }) - ).toBeDefined() - ) + const menu = await screen.findByRole("option", { + name: "Deploy this app", + }) + expect(menu).toBeDefined() }) describe("Onclick deploy button", () => { @@ -190,7 +196,6 @@ describe("MainMenu", () => { const items: any = menuWrapper.find("StatefulMenu").at(1).prop("items") const deployOption = items.find( - // @ts-expect-error ({ label }) => label === "Deploy this app" ) @@ -274,7 +279,6 @@ describe("MainMenu", () => { .find("MenuStatefulContainer") .at(0) .prop("items") - // @ts-expect-error .map(item => item.label) expect(menuLabels).toEqual([ "Rerun", @@ -293,13 +297,11 @@ describe("MainMenu", () => { aboutSectionMd: "", } const props = getProps({ menuItems }) - const wrapper = render() - await openMenu(wrapper) + render() + await openMenu(screen) await waitFor(() => - expect( - wrapper.queryByRole("option", { name: "Report a bug" }) - ).toBeNull() + expect(screen.queryByRole("option", { name: "Report a bug" })).toBeNull() ) }) @@ -311,14 +313,13 @@ describe("MainMenu", () => { aboutSectionMd: "", } const props = getProps({ menuItems }) - const wrapper = render() - await openMenu(wrapper) + render() + await openMenu(screen) - await waitFor(() => - expect( - wrapper.getByRole("option", { name: "Report a bug" }) - ).toBeDefined() - ) + const reportOption = await screen.findByRole("option", { + name: "Report a bug", + }) + expect(reportOption).toBeDefined() }) it("should not render dev menu when developmentMode is false", () => { @@ -333,7 +334,6 @@ describe("MainMenu", () => { .find("MenuStatefulContainer") // make sure that we only have one menu otherwise prop will fail .prop("items") - // @ts-expect-error .map(item => item.label) expect(menuLabels).toEqual([ "Rerun", @@ -356,10 +356,10 @@ describe("MainMenu", () => { { label: "Host menu item", key: "host-item", type: "text" }, ], }) - const wrapper = render() - await openMenu(wrapper) + const view = render() + await openMenu(screen) - const menuStructure = getMenuStructure(wrapper) + const menuStructure = getMenuStructure(view) expect(menuStructure[0]).toContainEqual({ type: "option", label: "Host menu item", @@ -373,9 +373,9 @@ describe("MainMenu", () => { hostMenuItems: [], }) - const wrapper = render() + render() - expect(wrapper.queryByRole("button")).toBeNull() + expect(screen.queryByRole("button")).toBeNull() }) it("should skip divider from host menu items if it is at the beginning and end", async () => { @@ -390,10 +390,10 @@ describe("MainMenu", () => { { type: "separator" }, ], }) - const wrapper = render() - await openMenu(wrapper) + const view = render() + await openMenu(screen) - const menuStructure = getMenuStructure(wrapper) + const menuStructure = getMenuStructure(view) expect(menuStructure).toEqual([ [{ type: "option", label: "View all apps" }], ]) @@ -463,10 +463,10 @@ describe("MainMenu", () => { ), }) - const wrapper = render() - await openMenu(wrapper) + const view = render() + await openMenu(screen) - const menuStructure = getMenuStructure(wrapper) + const menuStructure = getMenuStructure(view) expect(menuStructure).toEqual([expectedMenuItems]) } ) @@ -488,10 +488,10 @@ describe("MainMenu", () => { aboutSectionMd: "# This is a header. This is an *extremely* cool app!", }, }) - const wrapper = render() - await openMenu(wrapper) + const view = render() + await openMenu(screen) - const menuStructure = getMenuStructure(wrapper) + const menuStructure = getMenuStructure(view) expect(menuStructure).toEqual([ [ { diff --git a/frontend/app/src/components/Sidebar/Sidebar.test.tsx b/frontend/app/src/components/Sidebar/Sidebar.test.tsx index 24f8797547b2..064085727207 100644 --- a/frontend/app/src/components/Sidebar/Sidebar.test.tsx +++ b/frontend/app/src/components/Sidebar/Sidebar.test.tsx @@ -30,7 +30,7 @@ import SidebarNav from "./SidebarNav" expect.extend(matchers) -function renderSideBar(props: Partial = {}): ReactWrapper { +function mountSidebar(props: Partial = {}): ReactWrapper { return mount( = {}): ReactWrapper { describe("Sidebar Component", () => { it("should render without crashing", () => { - const wrapper = renderSideBar({}) + const wrapper = mountSidebar({}) expect(wrapper.find("StyledSidebar").exists()).toBe(true) }) it("should render expanded", () => { - const wrapper = renderSideBar({ + const wrapper = mountSidebar({ initialSidebarState: PageConfig.SidebarState.EXPANDED, }) @@ -62,7 +62,7 @@ describe("Sidebar Component", () => { }) it("should render collapsed", () => { - const wrapper = renderSideBar({ + const wrapper = mountSidebar({ initialSidebarState: PageConfig.SidebarState.COLLAPSED, }) @@ -70,7 +70,7 @@ describe("Sidebar Component", () => { }) it("should collapse on toggle if expanded", () => { - const wrapper = renderSideBar({ + const wrapper = mountSidebar({ initialSidebarState: PageConfig.SidebarState.EXPANDED, }) @@ -79,7 +79,7 @@ describe("Sidebar Component", () => { }) it("should expand on toggle if collapsed", () => { - const wrapper = renderSideBar({ + const wrapper = mountSidebar({ initialSidebarState: PageConfig.SidebarState.COLLAPSED, }) @@ -91,7 +91,7 @@ describe("Sidebar Component", () => { }) it("chevron does not render if sidebar expanded", () => { - const wrapper = renderSideBar({ + const wrapper = mountSidebar({ initialSidebarState: PageConfig.SidebarState.EXPANDED, }) @@ -99,7 +99,7 @@ describe("Sidebar Component", () => { }) it("hides scrollbar when hideScrollbar is called", () => { - const wrapper = renderSideBar({}) + const wrapper = mountSidebar({}) expect(wrapper.find("StyledSidebarContent")).toHaveStyleRule( "overflow", @@ -116,7 +116,7 @@ describe("Sidebar Component", () => { }) it("has extra top and bottom padding if no SidebarNav is displayed", () => { - const wrapper = renderSideBar({ + const wrapper = mountSidebar({ appPages: [{ pageName: "streamlit_app", pageScriptHash: "page_hash" }], }) @@ -127,7 +127,7 @@ describe("Sidebar Component", () => { }) it("has less padding if the SidebarNav is displayed", () => { - const wrapper = renderSideBar({ + const wrapper = mountSidebar({ appPages: [ { pageName: "streamlit_app", pageScriptHash: "page_hash" }, { pageName: "streamlit_app2", pageScriptHash: "page_hash2" }, @@ -141,7 +141,7 @@ describe("Sidebar Component", () => { }) it("uses the default chevron spacing if chevronDownshift is zero", () => { - const wrapper = renderSideBar({ + const wrapper = mountSidebar({ chevronDownshift: 0, initialSidebarState: PageConfig.SidebarState.COLLAPSED, }) @@ -153,7 +153,7 @@ describe("Sidebar Component", () => { }) it("uses the given chevron spacing if chevronDownshift is nonzero", () => { - const wrapper = renderSideBar({ + const wrapper = mountSidebar({ chevronDownshift: 50, initialSidebarState: PageConfig.SidebarState.COLLAPSED, }) @@ -169,7 +169,7 @@ describe("Sidebar Component", () => { { pageName: "streamlit_app", pageScriptHash: "page_hash" }, { pageName: "streamlit_app2", pageScriptHash: "page_hash2" }, ] - const wrapper = renderSideBar({ appPages }) + const wrapper = mountSidebar({ appPages }) expect(wrapper.find(SidebarNav).exists()).toBe(true) expect(wrapper.find(SidebarNav).prop("appPages")).toEqual(appPages) @@ -186,7 +186,7 @@ describe("Sidebar Component", () => { { pageName: "streamlit_app", pageScriptHash: "page_hash" }, { pageName: "streamlit_app2", pageScriptHash: "page_hash2" }, ] - const wrapper = renderSideBar({ appPages, hideSidebarNav: true }) + const wrapper = mountSidebar({ appPages, hideSidebarNav: true }) expect(wrapper.find(SidebarNav).exists()).toBe(false) expect( diff --git a/frontend/lib/src/components/core/Block/ElementNodeRenderer.test.tsx b/frontend/lib/src/components/core/Block/ElementNodeRenderer.test.tsx index 5cc0d3381a32..0bdeae92ac8a 100644 --- a/frontend/lib/src/components/core/Block/ElementNodeRenderer.test.tsx +++ b/frontend/lib/src/components/core/Block/ElementNodeRenderer.test.tsx @@ -15,15 +15,17 @@ */ import React from "react" +import "@testing-library/jest-dom" +import { screen, waitFor } from "@testing-library/react" +import { render } from "@streamlit/lib/src/test_util" + import { Balloons as BalloonsProto, ForwardMsgMetadata, Snow as SnowProto, } from "@streamlit/lib/src/proto" -import { render } from "@streamlit/lib/src/test_util" import { ElementNode } from "@streamlit/lib/src/AppNode" import { ScriptRunState } from "@streamlit/lib/src/ScriptRunState" -import { waitFor } from "@testing-library/dom" import { createFormsData, WidgetStateManager, @@ -94,14 +96,13 @@ describe("ElementNodeRenderer Block Component", () => { node: createBalloonNode(scriptRunId), scriptRunId: "NEW_SCRIPT_ID", }) - const wrapper = render() + render() - await waitFor(() => - expect(wrapper.baseElement.textContent).not.toContain("Loading...") - ) - expect( - wrapper.baseElement.querySelectorAll(".element-container > *") - ).toHaveLength(0) + await waitFor(() => expect(screen.queryByText("Loading...")).toBeNull()) + const elementNodeRenderer = screen.getByTestId("element-container") + expect(elementNodeRenderer).toBeInTheDocument() + // eslint-disable-next-line testing-library/no-node-access + expect(elementNodeRenderer.children).toHaveLength(0) }) it("should render a fresh component", async () => { @@ -110,16 +111,15 @@ describe("ElementNodeRenderer Block Component", () => { node: createBalloonNode(scriptRunId), scriptRunId, }) + render() - const wrapper = render() - - await waitFor(() => - expect(wrapper.baseElement.textContent).not.toContain("Loading...") - ) - expect( - wrapper.baseElement.querySelectorAll(".element-container > *") - ).toHaveLength(1) - expect(wrapper.baseElement.querySelectorAll(".balloons")).toHaveLength(1) + await waitFor(() => expect(screen.queryByText("Loading...")).toBeNull()) + const elementNodeRenderer = screen.getByTestId("element-container") + expect(elementNodeRenderer).toBeInTheDocument() + // eslint-disable-next-line testing-library/no-node-access + const elementRendererChildren = elementNodeRenderer.children + expect(elementRendererChildren).toHaveLength(1) + expect(elementRendererChildren[0]).toHaveClass("balloons") }) }) @@ -130,14 +130,13 @@ describe("ElementNodeRenderer Block Component", () => { node: createSnowNode(scriptRunId), scriptRunId: "NEW_SCRIPT_ID", }) - const wrapper = render() + render() - await waitFor(() => - expect(wrapper.baseElement.textContent).not.toContain("Loading...") - ) - expect( - wrapper.baseElement.querySelectorAll(".element-container > *") - ).toHaveLength(0) + await waitFor(() => expect(screen.queryByText("Loading...")).toBeNull()) + const elementNodeRenderer = screen.getByTestId("element-container") + expect(elementNodeRenderer).toBeInTheDocument() + // eslint-disable-next-line testing-library/no-node-access + expect(elementNodeRenderer.children).toHaveLength(0) }) it("should render a fresh component", async () => { @@ -146,15 +145,15 @@ describe("ElementNodeRenderer Block Component", () => { node: createSnowNode(scriptRunId), scriptRunId, }) - const wrapper = render() + render() - await waitFor(() => - expect(wrapper.baseElement.textContent).not.toContain("Loading...") - ) - expect( - wrapper.baseElement.querySelectorAll(".element-container > *") - ).toHaveLength(1) - expect(wrapper.baseElement.querySelectorAll(".snow")).toHaveLength(1) + await waitFor(() => expect(screen.queryByText("Loading...")).toBeNull()) + const elementNodeRenderer = screen.getByTestId("element-container") + expect(elementNodeRenderer).toBeInTheDocument() + // eslint-disable-next-line testing-library/no-node-access + const elementRendererChildren = elementNodeRenderer.children + expect(elementRendererChildren).toHaveLength(1) + expect(elementRendererChildren[0]).toHaveClass("snow") }) }) }) diff --git a/frontend/lib/src/components/core/Block/ElementNodeRenderer.tsx b/frontend/lib/src/components/core/Block/ElementNodeRenderer.tsx index 737945241fd7..c7cb27f45a49 100644 --- a/frontend/lib/src/components/core/Block/ElementNodeRenderer.tsx +++ b/frontend/lib/src/components/core/Block/ElementNodeRenderer.tsx @@ -116,6 +116,8 @@ const BokehChart = React.lazy( () => import("@streamlit/lib/src/components/elements/BokehChart") ) +// RTL ESLint triggers a false positive on this render function +// eslint-disable-next-line testing-library/render-result-naming-convention const DebouncedBokehChart = debounceRender(BokehChart, 100) const DataFrame = React.lazy( @@ -761,6 +763,7 @@ const ElementNodeRenderer = ( isStale={isStale && !isFullScreen} width={width} className={"element-container"} + data-testid={"element-container"} elementType={elementType} > diff --git a/frontend/lib/src/components/elements/ChatMessage/ChatMessage.test.tsx b/frontend/lib/src/components/elements/ChatMessage/ChatMessage.test.tsx index 93dd4e5d4688..920a3afc9851 100644 --- a/frontend/lib/src/components/elements/ChatMessage/ChatMessage.test.tsx +++ b/frontend/lib/src/components/elements/ChatMessage/ChatMessage.test.tsx @@ -16,6 +16,7 @@ import React from "react" import "@testing-library/jest-dom" +import { screen } from "@testing-library/react" import { render } from "@streamlit/lib/src/test_util" import { Block as BlockProto } from "@streamlit/lib/src/proto" @@ -36,16 +37,16 @@ const getProps = ( describe("ChatMessage", () => { it("renders without crashing", () => { const props = getProps() - const rtlResults = render() - expect(rtlResults).toBeDefined() + render() + + const chatContent = screen.getByTestId("stChatMessageContent") + expect(chatContent).toBeInTheDocument() }) it("renders message children content", () => { const props = getProps() - const { getByLabelText } = render( - Hello, world! - ) - expect(getByLabelText("Chat message from user").textContent).toBe( + render(Hello, world!) + expect(screen.getByLabelText("Chat message from user").textContent).toBe( "Hello, world!" ) }) @@ -55,8 +56,8 @@ describe("ChatMessage", () => { avatar: "😃", avatarType: BlockProto.ChatMessage.AvatarType.EMOJI, }) - const rtlResults = render() - expect(rtlResults.getByText("😃")).toBeTruthy() + render() + expect(screen.getByText("😃")).toBeTruthy() }) it("renders with an image avatar", () => { @@ -64,10 +65,9 @@ describe("ChatMessage", () => { avatar: "http://example.com/avatar.jpg", avatarType: BlockProto.ChatMessage.AvatarType.IMAGE, }) - const { container } = render() - const images = container.getElementsByTagName("img") - expect(images.length).toEqual(1) - expect(images[0].src).toBe("http://example.com/avatar.jpg") + render() + const chatAvatar = screen.getByAltText("user avatar") + expect(chatAvatar).toHaveAttribute("src", "http://example.com/avatar.jpg") }) it("renders with a name label character as fallback", () => { @@ -76,8 +76,8 @@ describe("ChatMessage", () => { avatarType: undefined, name: "test", }) - const { getByText } = render() - expect(getByText("T")).toBeTruthy() + render() + expect(screen.getByText("T")).toBeTruthy() }) it("renders with a 'user' icon avatar", () => { @@ -86,10 +86,10 @@ describe("ChatMessage", () => { avatarType: BlockProto.ChatMessage.AvatarType.ICON, name: "foo", }) - const { container } = render() + render() - const svgs = container.getElementsByTagName("svg") - expect(svgs.length).toEqual(1) + const userAvatarIcon = screen.getByTestId("chatAvatarIcon-user") + expect(userAvatarIcon).toBeInTheDocument() }) it("renders with a 'assistant' icon avatar", () => { @@ -98,28 +98,26 @@ describe("ChatMessage", () => { avatarType: BlockProto.ChatMessage.AvatarType.ICON, name: "foo", }) - const { container } = render() + render() - const svgs = container.getElementsByTagName("svg") - expect(svgs.length).toEqual(1) + const assistantAvatarIcon = screen.getByTestId("chatAvatarIcon-assistant") + expect(assistantAvatarIcon).toBeInTheDocument() }) it("renders with a grey background when name is 'user'", () => { - const props = getProps({ - name: "user", - }) - const { container } = render() - const messageContainer = container.firstChild - expect(messageContainer).toHaveStyle( + const props = getProps() + render() + const chatMessage = screen.getByTestId("stChatMessage") + expect(chatMessage).toHaveStyle( "background-color: rgba(240, 242, 246, 0.5)" ) }) it("sets an aria label on the chat message", () => { const props = getProps() - const { getByTestId } = render() + render() - const chatMessageContent = getByTestId("stChatMessageContent") + const chatMessageContent = screen.getByTestId("stChatMessageContent") expect(chatMessageContent.getAttribute("aria-label")).toEqual( "Chat message from user" ) diff --git a/frontend/lib/src/components/elements/ChatMessage/ChatMessage.tsx b/frontend/lib/src/components/elements/ChatMessage/ChatMessage.tsx index d50cbd7b2907..c1296d6d3e62 100644 --- a/frontend/lib/src/components/elements/ChatMessage/ChatMessage.tsx +++ b/frontend/lib/src/components/elements/ChatMessage/ChatMessage.tsx @@ -49,13 +49,19 @@ function ChatMessageAvatar(props: ChatMessageAvatarProps): ReactElement { case BlockProto.ChatMessage.AvatarType.ICON: if (avatar === "user") { return ( - + ) } else if (avatar === "assistant") { return ( - + ) @@ -84,14 +90,10 @@ const ChatMessage: React.FC = ({ return ( - + { hideFullScreenButtons: true, } as Partial - const { queryByTestId } = customRenderLibContext( - , - providerProps - ) - expect(queryByTestId("StyledFullScreenButton")).toBeNull() + customRenderLibContext(, providerProps) + // queryBy returns null vs. error + expect(screen.queryByTestId("StyledFullScreenButton")).toBeNull() // eslint-disable-line testing-library/prefer-presence-queries }) it("can find StyledFullScreenButton with default LibContext", () => { const props = getProps() - const { queryByTestId } = customRenderLibContext( - , - {} - ) - expect(queryByTestId("StyledFullScreenButton")).not.toBeNull() + customRenderLibContext(, {}) + // queryBy returns null vs. error + expect(screen.queryByTestId("StyledFullScreenButton")).not.toBeNull() // eslint-disable-line testing-library/prefer-presence-queries }) }) diff --git a/frontend/lib/src/components/shared/InputInstructions/InputInstructions.test.tsx b/frontend/lib/src/components/shared/InputInstructions/InputInstructions.test.tsx index 6c7100f6dfaa..8a6a906c8075 100644 --- a/frontend/lib/src/components/shared/InputInstructions/InputInstructions.test.tsx +++ b/frontend/lib/src/components/shared/InputInstructions/InputInstructions.test.tsx @@ -15,6 +15,8 @@ */ import React from "react" +import "@testing-library/jest-dom" +import { screen } from "@testing-library/react" import { render } from "@streamlit/lib/src/test_util" import InputInstructions, { Props } from "./InputInstructions" @@ -30,15 +32,15 @@ describe("InputInstructions", () => { const props = getProps() it("renders without crashing", () => { - const { getByTestId } = render() + render() - expect(getByTestId("InputInstructions").textContent).toBeDefined() + expect(screen.getByTestId("InputInstructions").textContent).toBeDefined() }) it("should show Enter instructions", () => { - const { getByTestId } = render() + render() - expect(getByTestId("InputInstructions").textContent).toBe( + expect(screen.getByTestId("InputInstructions").textContent).toBe( "Press Enter to apply" ) }) @@ -49,8 +51,8 @@ describe("InputInstructions", () => { }) it("should show Ctrl+Enter instructions", () => { - const { getByTestId } = render() - expect(getByTestId("InputInstructions").textContent).toBe( + render() + expect(screen.getByTestId("InputInstructions").textContent).toBe( "Press Ctrl+Enter to apply" ) }) @@ -64,9 +66,9 @@ describe("InputInstructions", () => { const props = getProps({ type: "multiline", }) - const { getByTestId } = render() + render() - expect(getByTestId("InputInstructions").textContent).toBe( + expect(screen.getByTestId("InputInstructions").textContent).toBe( "Press ⌘+Enter to apply" ) }) @@ -76,9 +78,9 @@ describe("InputInstructions", () => { type: "multiline", maxLength: 3, }) - const { getByTestId } = render() + render() - expect(getByTestId("InputInstructions").textContent).toBe( + expect(screen.getByTestId("InputInstructions").textContent).toBe( "Press ⌘+Enter to apply3/3" ) }) @@ -88,9 +90,9 @@ describe("InputInstructions", () => { const props = getProps({ maxLength: 3, }) - const { getByTestId } = render() + render() - expect(getByTestId("InputInstructions").textContent).toBe( + expect(screen.getByTestId("InputInstructions").textContent).toBe( "Press Enter to apply3/3" ) }) @@ -101,8 +103,8 @@ describe("InputInstructions", () => { }) it("should not show instructions", () => { - const { getByTestId } = render() - expect(getByTestId("InputInstructions").textContent).toBe("") + render() + expect(screen.getByTestId("InputInstructions").textContent).toBe("") }) it("should show instructions for max length", () => { @@ -110,9 +112,9 @@ describe("InputInstructions", () => { type: "chat", maxLength: 3, }) - const { getByTestId } = render() + render() - expect(getByTestId("InputInstructions").textContent).toBe("3/3") + expect(screen.getByTestId("InputInstructions").textContent).toBe("3/3") }) }) @@ -122,9 +124,9 @@ describe("InputInstructions", () => { inForm: true, type: "single", }) - const { getByTestId } = render() + render() - expect(getByTestId("InputInstructions").textContent).toBe( + expect(screen.getByTestId("InputInstructions").textContent).toBe( "Press Enter to submit form" ) }) @@ -135,9 +137,9 @@ describe("InputInstructions", () => { inForm: true, type: "multiline", }) - const { getByTestId } = render() + render() - expect(getByTestId("InputInstructions").textContent).toBe( + expect(screen.getByTestId("InputInstructions").textContent).toBe( "Press ⌘+Enter to submit form" ) }) diff --git a/frontend/lib/src/components/shared/StreamlitMarkdown/StreamlitMarkdown.test.tsx b/frontend/lib/src/components/shared/StreamlitMarkdown/StreamlitMarkdown.test.tsx index 425673421830..5a8e318afb90 100644 --- a/frontend/lib/src/components/shared/StreamlitMarkdown/StreamlitMarkdown.test.tsx +++ b/frontend/lib/src/components/shared/StreamlitMarkdown/StreamlitMarkdown.test.tsx @@ -175,13 +175,12 @@ describe("StreamlitMarkdown", () => { test.each(validCases)( "renders valid markdown when isLabel is true - $tag", ({ input, tag, expected }) => { - const wrapper = render( - - ) - const container = wrapper.getByTestId("stMarkdownContainer") - const expectedTag = container.querySelector(tag) - expect(expectedTag).not.toBeNull() - expect(expectedTag).toHaveTextContent(expected) + render() + const markdownText = screen.getByText(expected) + expect(markdownText).toBeInTheDocument() + + const expectedTag = markdownText.nodeName.toLowerCase() + expect(expectedTag).toEqual(tag) // Removes rendered StreamlitMarkdown component before next case run cleanup() @@ -202,12 +201,6 @@ describe("StreamlitMarkdown", () => { ` const invalidCases = [ - { - input: - "![Image Text](https://dictionary.cambridge.org/us/images/thumb/corgi_noun_002_08554.jpg?version=5.0.297)", - tag: "img", - expected: "", - }, { input: table, tag: "table", expected: tableText }, { input: table, tag: "thead", expected: tableText }, { input: table, tag: "tbody", expected: tableText }, @@ -233,29 +226,36 @@ describe("StreamlitMarkdown", () => { test.each(invalidCases)( "does NOT render invalid markdown when isLabel is true - $tag", ({ input, tag, expected }) => { - const wrapper = render( - - ) - const container = wrapper.getByTestId("stMarkdownContainer") - const invalidTag = container.querySelector(tag) - expect(invalidTag).toBeNull() - expect(container).toHaveTextContent(expected) + render() + const markdownText = screen.getByText(expected) + expect(markdownText).toBeInTheDocument() + + const expectedTag = markdownText.nodeName.toLowerCase() + expect(expectedTag).not.toEqual(tag) // Removes rendered StreamlitMarkdown component before next case run cleanup() } ) + it("doesn't render images when isLabel is true", () => { + const source = + "![Image Text](https://dictionary.cambridge.org/us/images/thumb/corgi_noun_002_08554.jpg?version=5.0.297)" + + render() + const image = screen.queryByAltText("Image Text") + expect(image).not.toBeInTheDocument() + }) + it("doesn't render links when isButton is true", () => { // Valid markdown further restricted with buttons to eliminate links - const source = "Link: [text](www.example.com)" - const wrapper = render( + const source = "[text](www.example.com)" + render( ) - const container = wrapper.getByTestId("stMarkdownContainer") - const invalidTag = container.querySelector("a") - expect(invalidTag).toBeNull() - expect(container).toHaveTextContent("Link: ") + const markdown = screen.getByText("text") + const tagName = markdown.nodeName.toLowerCase() + expect(tagName).not.toBe("a") }) it("renders smaller text sizing when isToast is true", () => { @@ -277,14 +277,11 @@ describe("StreamlitMarkdown", () => { colorMapping.forEach(function (style, color) { const source = `:${color}[text]` - const wrapper = render( - - ) - - const container = wrapper.getByTestId("stMarkdownContainer") - const span = container.querySelector("span") - - expect(span).toHaveStyle(`color: ${style}`) + render() + const markdown = screen.getByText("text") + const tagName = markdown.nodeName.toLowerCase() + expect(tagName).toBe("span") + expect(markdown).toHaveStyle(`color: ${style}`) // Removes rendered StreamlitMarkdown component before next case run cleanup() @@ -308,40 +305,42 @@ st.write("Hello") describe("CustomCodeTag Element", () => { it("should render without crashing", () => { const props = getCustomCodeTagProps() - const { baseElement } = render() + render() + + const codeTag = screen.getByText(`st.write("Hello")`) + const tagName = codeTag.nodeName.toLowerCase() - expect(baseElement.querySelectorAll("pre code")).toHaveLength(1) + expect(codeTag).toBeInTheDocument() + expect(tagName).toBe("code") }) it("should render as plaintext", () => { const props = getCustomCodeTagProps({ className: "language-plaintext" }) - const { baseElement } = render() + render() - expect(baseElement.querySelector("pre code")?.outerHTML).toBe( - 'import streamlit as st\n' + - "\n" + - 'st.write("Hello")' - ) + const codeTag = screen.getByText(`st.write("Hello")`) + expect(codeTag).toHaveClass("language-plaintext") }) it("should render copy button when code block has content", () => { const props = getCustomCodeTagProps({ children: ["i am not empty"], }) - const { baseElement } = render() - expect( - baseElement.querySelectorAll('[title="Copy to clipboard"]') - ).toHaveLength(1) + render() + const copyButton = screen.getByTitle("Copy to clipboard") + + expect(copyButton).not.toBeNull() }) it("should not render copy button when code block is empty", () => { const props = getCustomCodeTagProps({ children: [""], }) - const { baseElement } = render() - expect( - baseElement.querySelectorAll('[title="Copy to clipboard"]') - ).toHaveLength(0) + render() + // queryBy returns null vs. error + const copyButton = screen.queryByRole("button") // eslint-disable-line testing-library/prefer-presence-queries + + expect(copyButton).toBeNull() }) it("should render inline", () => { diff --git a/frontend/lib/src/components/widgets/BaseWidget/WidgetLabel.tsx b/frontend/lib/src/components/widgets/BaseWidget/WidgetLabel.tsx index e65bb9075638..83d9aea6ce52 100644 --- a/frontend/lib/src/components/widgets/BaseWidget/WidgetLabel.tsx +++ b/frontend/lib/src/components/widgets/BaseWidget/WidgetLabel.tsx @@ -48,6 +48,7 @@ export function WidgetLabel({ // we use aria-hidden to disable ARIA for StyleWidgetLabel, because each // widget should have its own aria-label and/or implement accessibility.