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

refactor: test suggestions review, linter #2

Open
wants to merge 5 commits into
base: master
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
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ To run tests please run `npm test`

2. UX improvements:

a) Table is not very mobile friendly, but great for general data display. For this task I decided to leave it as is, but ux-wise it could work better.
a) Table is not very mobile friendly, but great for general data display. For this task I decided to leave it as is, but ux-wise it could work better.

b) Implementing on press enter for search input.
b) Implementing on press enter for search input.

3. File structure:
This is very specific for every project and there are countless wars around that topic.
This is very specific for every project and there are countless wars around that topic.

4. Linting files on pre-commit.

Expand Down
8 changes: 0 additions & 8 deletions codegen.yml

This file was deleted.

15 changes: 8 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
"build": "react-scripts build",
"test": "react-scripts test",
"eject": "react-scripts eject",
"format": "prettier --write ."
"format": "prettier --write .",
"lint": "eslint src/**/*.{ts,tsx}"
},
"eslintConfig": {
"extends": [
Expand All @@ -49,6 +50,7 @@
]
},
"devDependencies": {
"eslint": "^8.56.0",
"prettier": "^3.1.1"
}
}
14 changes: 9 additions & 5 deletions src/Components/RepositoryTable/RepositoryTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ import './RepositoryTable.scss';

const RepositoryTable = React.memo(() => {
const repositories = useReactiveVar(repositoriesVar);
const { handleLoadMore } = useFetchData();
const { loading, error } = useFetchData();

const { handleLoadMore, loading, error } = useFetchData();
if (loading) {
return <p data-testid="tableMessageLoading">Loading...</p>;
}
Expand Down Expand Up @@ -51,13 +49,19 @@ const RepositoryTable = React.memo(() => {
{repositories.search.edges.map(({ node }, i: number) => (
<TableRow
key={i}
data-testid="repoTableRow"
sx={{
'&:last-child td, &:last-child th': { border: 0 },
typography: 'subtitle2',
}}
>
<TableCell component="th" scope="row">
<a href={node.url} target="_blank" className="tableLink">
<TableCell component="th" scope="row" data-testid="repoTableRowName">
<a
href={node.url}
target="_blank"
rel="noreferrer"
className="tableLink"
>
{node.name}
</a>
</TableCell>
Expand Down
217 changes: 152 additions & 65 deletions src/Components/__tests__/RepositoryTable.test.tsx
Original file line number Diff line number Diff line change
@@ -1,107 +1,194 @@
import { render, screen, waitFor } from '@testing-library/react';
import * as useFetchDataModule from '../../hooks/useFetchData';
import { render, screen } from '@testing-library/react';
import RepositoryTable from '../RepositoryTable/RepositoryTable';
import { FetchDataHandlers } from '../../hooks/useFetchData';

jest.mock('../../hooks/useFetchData', () => ({
useFetchData: jest.fn(),
}));
import { POPULAR_REPOS_QUERY } from '../../graphql/queries/repositories';
import { MockedProvider } from '@apollo/client/testing';

const mockRepositoryData = {
search: {
edges: [
{
node: {
forkCount: 20,
forkCount: 40,
name: 'Repo 1',
url: 'https://repo1.com',
stargazerCount: 10,
stargazerCount: 25,
},
},
{
node: {
forkCount: 30,
forkCount: 40,
name: 'Repo 2',
url: 'https://repo2.com',
stargazerCount: 70,
stargazerCount: 25,
},
},
{
node: {
forkCount: 40,
name: 'Repo 3',
url: 'https://repo3.com',
stargazerCount: 25,
},
},
{
node: {
forkCount: 15,
name: 'Repo 4',
url: 'https://repo4.com',
stargazerCount: 50,
},
},
{
node: {
forkCount: 22,
name: 'Repo 5',
url: 'https://repo5.com',
stargazerCount: 15,
},
},
{
node: {
forkCount: 18,
name: 'Repo 6',
url: 'https://repo6.com',
stargazerCount: 40,
},
},
{
node: {
forkCount: 33,
name: 'Repo 7',
url: 'https://repo7.com',
stargazerCount: 30,
},
},
{
node: {
forkCount: 25,
name: 'Repo 8',
url: 'https://repo8.com',
stargazerCount: 20,
},
},
{
node: {
forkCount: 28,
name: 'Repo 9',
url: 'https://repo9.com',
stargazerCount: 18,
},
},
{
node: {
forkCount: 15,
name: 'Repo 10',
url: 'https://repo10.com',
stargazerCount: 22,
},
},
],
pageInfo: {
endCursor: 'cursor2',
hasNextPage: false,
hasNextPage: true,
},
},
};

jest.mock('@apollo/client', () => ({
...jest.requireActual('@apollo/client'),
gql: jest.fn((strings) => strings[0]),
makeVar: () => {},
useReactiveVar: () => mockRepositoryData,
}));
const mocks = [
{
request: {
query: POPULAR_REPOS_QUERY,
variables: {
first: 10,
query: 'language:javascript topic:react ',
},
},
result: {
data: mockRepositoryData,
},
},
];

describe('RepositoryTable component', () => {
beforeEach(() => {
jest.spyOn(useFetchDataModule, 'useFetchData').mockImplementation(
() =>
({
loading: false,
error: null,
handleLoadMore: jest.fn(),
}) as unknown as FetchDataHandlers,
it('renders RepositoryTable component with 10 rows', async () => {
render(
<MockedProvider mocks={mocks} addTypename={false}>
<RepositoryTable />
</MockedProvider>,
);
const rows = await screen.findAllByTestId('repoTableRow');
expect(rows).toHaveLength(10);
});

test('renders RepositoryTable component with Repo 2', async () => {
render(<RepositoryTable />);
await waitFor(() => screen.getByText('Repo 2'));
const repositoryName = screen.getByText('Repo 2');
expect(repositoryName).toBeInTheDocument();
});
it('renders button correctly', async () => {
render(
<MockedProvider mocks={mocks} addTypename={false}>
<RepositoryTable />
</MockedProvider>,
);
const buttonLoadMore = await screen.findByTestId('buttonLoadMore');

test('renders button correctly', async () => {
render(<RepositoryTable />);
const buttonLoadMore = screen.getByTestId('buttonLoadMore');
expect(buttonLoadMore).toBeDisabled();
expect(buttonLoadMore).toBeInTheDocument();
});

test('renders repo names as links', async () => {
render(<RepositoryTable />);
await waitFor(() => screen.getByText('Repo 1'));
const repo1Link = screen.getByRole('link', { name: 'Repo 1' });
const repo2Link = screen.getByRole('link', { name: 'Repo 2' });
expect(repo1Link).toBeInTheDocument();
expect(repo1Link).toHaveAttribute('href', 'https://repo1.com');
expect(repo1Link).toHaveAttribute('target', '_blank');

expect(repo2Link).toBeInTheDocument();
expect(repo2Link).toHaveAttribute('href', 'https://repo2.com');
expect(repo2Link).toHaveAttribute('target', '_blank');
it('renders repo names as links', async () => {
render(
<MockedProvider mocks={mocks} addTypename={false}>
<RepositoryTable />
</MockedProvider>,
);
await screen.findByTestId('repoTableComponent');
await screen.findByTestId('repoTableComponent');
const repoTableRowNames = screen.getAllByTestId('repoTableRowName');
repoTableRowNames.forEach((repoTableRowName) => {
const nestedAnchorElement = repoTableRowName.querySelector('a');
expect(nestedAnchorElement).toBeInTheDocument();
});
});

test('renders message if loading', async () => {
jest.spyOn(useFetchDataModule, 'useFetchData').mockImplementation(
() =>
({
loading: true,
}) as unknown as FetchDataHandlers,
it('renders message if loading', async () => {
const loadingMocks = [
{
delay: 30,
request: {
query: POPULAR_REPOS_QUERY,
variables: {
first: 10,
query: 'language:javascript topic:react ',
},
},
result: {
data: mockRepositoryData,
},
},
];
render(
<MockedProvider mocks={loadingMocks} addTypename={false}>
<RepositoryTable />
</MockedProvider>,
);
render(<RepositoryTable />);
const loadingMessage = screen.getByTestId('tableMessageLoading');
expect(loadingMessage).toBeInTheDocument();
});

test('renders message if error', async () => {
jest.spyOn(useFetchDataModule, 'useFetchData').mockImplementation(
() =>
({
loading: false,
error: true,
}) as unknown as FetchDataHandlers,
it('renders message if error', async () => {
const errorMocks = [
{
request: {
query: POPULAR_REPOS_QUERY,
variables: {
first: 10,
query: 'language:javascript topic:react ',
},
},
error: new Error('An error occurred'),
},
];
render(
<MockedProvider mocks={errorMocks} addTypename={false}>
<RepositoryTable />
</MockedProvider>,
);
render(<RepositoryTable />);
const errorMessage = screen.getByTestId('tableMessageError');
const errorMessage = await screen.findByTestId('tableMessageError');
expect(errorMessage).toBeInTheDocument();
});
});
Loading