-
Notifications
You must be signed in to change notification settings - Fork 108
Unit test for browsing page #2851
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,7 @@ | |
Notice the use of %PUBLIC_URL% in the tags above. | ||
It will be replaced with the URL of the `public` folder during the build. | ||
Only files inside the `public` folder can be referenced from the HTML. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You've got some superfluous blank characters, here. |
||
Unlike "/favicon.ico" or "favicon.ico", "%PUBLIC_URL%/favicon.ico" will | ||
work correctly both with client-side routing and a non-root public URL. | ||
Learn how to configure a non-root public URL by running `npm run build`. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ import MainLayout from 'modules/containers/MainLayout'; | |
function App() { | ||
useEffect(() => { | ||
const faviconLogo = document.getElementById("favicon"); | ||
faviconLogo.setAttribute("href", favicon); | ||
faviconLogo?.setAttribute("href", favicon); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change doesn't seem like it should be in a unit testing commit/PR. |
||
}, []); | ||
|
||
return ( | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import { Provider } from "react-redux"; | ||
import store from "store/store"; | ||
import App from "../../../App"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this context, do we have a "root" configured, so that we don't have to use relative import paths? (I.e., can we just import this |
||
const { render, screen, fireEvent } = require("@testing-library/react"); | ||
const AppWrapper = () => { | ||
return ( | ||
<Provider store={store}> | ||
<App /> | ||
</Provider> | ||
); | ||
}; | ||
test("Alert message is displayed on initial load", () => { | ||
render(<AppWrapper />); | ||
const alert = screen.getByText('Want to see your own data?'); | ||
expect(alert).toBeInTheDocument(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there any other attributes that we should be checking? |
||
}); | ||
|
||
test("Alert message is closed on clicking close button", () => { | ||
render(<AppWrapper />); | ||
const alert = screen.getByText('Want to see your own data?'); | ||
const closeButton = screen.getByRole("button", { | ||
name: /close info alert/i, | ||
}); | ||
fireEvent.click(closeButton); | ||
expect(alert).not.toBeVisible(); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
import { Provider } from "react-redux"; | ||
import store from "store/store"; | ||
import { MOCK_DATA } from "utils/mockData"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is specifically a mock for |
||
import App from "../../../App"; | ||
const { render, screen, fireEvent } = require("@testing-library/react"); | ||
const AppWrapper = () => { | ||
return ( | ||
<Provider store={store}> | ||
<App /> | ||
</Provider> | ||
); | ||
}; | ||
Comment on lines
+6
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be refactored into a common module and reused in all the tests? |
||
jest.mock("utils/api", () => { | ||
return { | ||
get: () => ({ | ||
data: MOCK_DATA, | ||
status:200 | ||
}), | ||
}; | ||
}); | ||
test("data is filtered based on date range selected from date picker", async () => { | ||
render(<AppWrapper />); | ||
await screen.findByText("pbench_user_benchmark1"); | ||
const datePickerInput = screen.getAllByPlaceholderText('YYYY-MM-DD'); | ||
fireEvent.change(datePickerInput[0], { target: { value: "2022-02-16" } }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we be checking that the UI has updated from Also, can the code here look at the values in the React-store? I think those would be good things to check, as well. |
||
fireEvent.change(datePickerInput[1], { target: { value: "2022-02-20" } }); | ||
const updateBtn = screen.getByRole("button", { name: 'Update'}); | ||
fireEvent.click(updateBtn); | ||
const datasetNameOne = screen.queryByText("pbench_user_benchmark1"); | ||
const datasetNameTwo = screen.queryByText("pbench_user_benchmark2"); | ||
const datasetNameThree = screen.queryByText("pbench_user_benchmark3"); | ||
const datasetNameFour = screen.queryByText("pbench_user_benchmark4"); | ||
const datasetNameFive = screen.queryByText("pbench_user_benchmark5"); | ||
expect(datasetNameOne).toBeInTheDocument(); | ||
expect(datasetNameTwo).toBeInTheDocument(); | ||
expect(datasetNameThree).toBeInTheDocument(); | ||
expect(datasetNameFour).not.toBeInTheDocument(); | ||
expect(datasetNameFive).not.toBeInTheDocument(); | ||
Comment on lines
+29
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks to me like these assertions will fit on one line, so there's no need to use local variables for them (in-lining the expressions will cut the number of new lines of code in half...).
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A similar comment applies to |
||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
import { Provider } from "react-redux"; | ||
import store from "store/store"; | ||
import { MOCK_DATA } from "utils/mockData"; | ||
import App from "../../../App"; | ||
const { render, screen, fireEvent } = require("@testing-library/react"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Teach me something: why do we use |
||
const AppWrapper = () => { | ||
return ( | ||
<Provider store={store}> | ||
<App /> | ||
</Provider> | ||
); | ||
}; | ||
jest.mock("utils/api", () => { | ||
return { | ||
get: () => ({ | ||
data: MOCK_DATA, | ||
status: 200, | ||
}), | ||
}; | ||
}); | ||
test("Page heading is displayed on initial load", async () => { | ||
render(<AppWrapper />); | ||
await screen.findByText("pbench_user_benchmark1"); | ||
const heading = screen.getByRole("heading", { name: 'Results' }); | ||
expect(heading).toBeInTheDocument(); | ||
}); | ||
test("data from API is displayed on initial load", async () => { | ||
render(<AppWrapper />); | ||
await screen.findByText("pbench_user_benchmark1"); | ||
const datasetNameOne = screen.queryByText("pbench_user_benchmark1"); | ||
const datasetNameTwo = screen.queryByText("pbench_user_benchmark2"); | ||
const datasetNameThree = screen.queryByText("pbench_user_benchmark3"); | ||
const datasetNameFour = screen.queryByText("pbench_user_benchmark4"); | ||
const datasetNameFive = screen.queryByText("pbench_user_benchmark5"); | ||
expect(datasetNameOne).toBeInTheDocument(); | ||
expect(datasetNameTwo).toBeInTheDocument(); | ||
expect(datasetNameThree).toBeInTheDocument(); | ||
expect(datasetNameFour).toBeInTheDocument(); | ||
expect(datasetNameFive).toBeInTheDocument(); | ||
}); | ||
|
||
test("row is favorited after clicking on favorite icon", async () => { | ||
render(<AppWrapper />); | ||
await screen.findByText("pbench_user_benchmark1"); | ||
const starBtn = screen.getAllByRole("button", { | ||
name: 'Not starred', | ||
}); | ||
fireEvent.click(starBtn[0]); | ||
fireEvent.click(starBtn[1]); | ||
const favoriteBtn = screen.getByRole("button", { | ||
name: 'see favorites button', | ||
}); | ||
fireEvent.click(favoriteBtn); | ||
const datasetNameOne = screen.queryByText("pbench_user_benchmark1"); | ||
const datasetNameTwo = screen.queryByText("pbench_user_benchmark2"); | ||
const datasetNameThree = screen.queryByText("pbench_user_benchmark3"); | ||
const datasetNameFour = screen.queryByText("pbench_user_benchmark4"); | ||
const datasetNameFive = screen.queryByText("pbench_user_benchmark5"); | ||
expect(datasetNameOne).toBeInTheDocument(); | ||
expect(datasetNameTwo).toBeInTheDocument(); | ||
expect(datasetNameThree).not.toBeInTheDocument(); | ||
expect(datasetNameFour).not.toBeInTheDocument(); | ||
expect(datasetNameFive).not.toBeInTheDocument(); | ||
}); | ||
test("data is filtered based on value in search box", async () => { | ||
render(<AppWrapper />); | ||
await screen.findByText("pbench_user_benchmark1"); | ||
const searchBox = screen.getByPlaceholderText('Search Dataset'); | ||
fireEvent.change(searchBox, { target: { value: "pbench_user_benchmark2" } }); | ||
webbnh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const searchBtn = screen.getByRole("button", { | ||
name: 'searchButton', | ||
}); | ||
fireEvent.click(searchBtn); | ||
const datasetNameTwo = screen.queryByText("pbench_user_benchmark2"); | ||
const datasetNameThree = screen.queryByText("pbench_user_benchmark3"); | ||
expect(datasetNameTwo).toBeInTheDocument(); | ||
expect(datasetNameThree).not.toBeInTheDocument(); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,11 +26,11 @@ import DatePickerWidget from "../DatePickerComponent"; | |
import PathBreadCrumb from "../BreadCrumbComponent"; | ||
import { LoginHint, Heading, EmptyTable, SearchBox } from "./common-components"; | ||
import { getTodayMidnightUTCDate } from "utils/dateFunctions"; | ||
import { bumpToDate } from "utils/dateFunctions"; | ||
|
||
let startDate = new Date(Date.UTC(1990, 10, 4)); | ||
let endDate = getTodayMidnightUTCDate(); | ||
let endDate = bumpToDate(getTodayMidnightUTCDate(),1); | ||
let datasetName = ""; | ||
let dataArray = []; | ||
|
||
const TableWithFavorite = () => { | ||
const columnNames = { | ||
|
@@ -51,7 +51,7 @@ const TableWithFavorite = () => { | |
dispatch(getFavoritedDatasets()); | ||
}, [dispatch]); | ||
|
||
const { publicData, favoriteRepoNames } = useSelector( | ||
const { publicData, favoriteRepoNames,tableData } = useSelector( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like you need to run Prettier again. 🙃 It would be good to develop a habit of running Prettier before each commit. |
||
(state) => state.datasetlist | ||
); | ||
const setPublicData = (data) => { | ||
|
@@ -141,20 +141,17 @@ const TableWithFavorite = () => { | |
|
||
<PageSection variant={PageSectionVariants.light}> | ||
<PathBreadCrumb pathList={datasetBreadcrumb} /> | ||
<Heading | ||
containerClass="publicDataPageTitle" | ||
headingTitle="Results" | ||
/> | ||
<Heading containerClass="publicDataPageTitle" headingTitle="Results" /> | ||
<div className="filterContainer"> | ||
<SearchBox | ||
dataArray={dataArray} | ||
dataArray={tableData} | ||
setPublicData={setPublicData} | ||
startDate={startDate} | ||
endDate={endDate} | ||
setDatasetName={setDatasetName} | ||
/> | ||
<DatePickerWidget | ||
dataArray={dataArray} | ||
dataArray={tableData} | ||
setPublicData={setPublicData} | ||
datasetName={datasetName} | ||
setDateRange={setDateRange} | ||
|
@@ -167,13 +164,15 @@ const TableWithFavorite = () => { | |
isSelected={isSelected === "datasetListButton"} | ||
onChange={handleButtonClick} | ||
className="datasetListButton" | ||
aria-label="see dataset button" | ||
/> | ||
<ToggleGroupItem | ||
text={`Favorites(${favoriteRepoNames?.length})`} | ||
buttonId="favoriteListButton" | ||
isSelected={isSelected === "favoriteListButton"} | ||
onChange={handleButtonClick} | ||
className="favoriteListButton" | ||
aria-label="see favorites button" | ||
/> | ||
</ToggleGroup> | ||
<TableComposable aria-label="Favoritable table" variant="compact"> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
export const MOCK_DATA = [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm still a bit concerned about the generic name There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
|
||
{ | ||
name: "pbench_user_benchmark1", | ||
metadata: { | ||
"dataset.created": "2022-02-16T13:21:29+00:00", | ||
}, | ||
}, | ||
{ | ||
name: "pbench_user_benchmark2", | ||
metadata: { | ||
"dataset.created": "2022-02-18T13:21:29+00:00", | ||
}, | ||
}, | ||
{ | ||
name: "pbench_user_benchmark3", | ||
metadata: { | ||
"dataset.created": "2022-02-20T13:21:29+00:00", | ||
}, | ||
}, | ||
{ | ||
name: "pbench_user_benchmark4", | ||
metadata: { | ||
"dataset.created": "2022-02-25T13:21:29+00:00", | ||
}, | ||
}, | ||
{ | ||
name: "pbench_user_benchmark5", | ||
metadata: { | ||
"dataset.created": "2022-03-08T13:21:29+00:00", | ||
}, | ||
}, | ||
]; |
Uh oh!
There was an error while loading. Please reload this page.