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

QA mini code review #23

Open
bobbysebolao opened this issue Jun 15, 2020 · 0 comments
Open

QA mini code review #23

bobbysebolao opened this issue Jun 15, 2020 · 0 comments

Comments

@bobbysebolao
Copy link

MAD RESPECT WIT' DA FRESH TO DEATH TESTS! Your project looks like it's coming along really nicely, and it's great that you have several passing tests and code coverage reporting 🙌 🎉 💥 really really well done y'all!

Here is some constructive feedback from me:

1. Wrap related tests inside a describe() block

It’s a good idea to use Jest’s describe() block to group related tests. I think the main benefit of doing this in your test files would be to keep the test code nice and readable. When a file contains lots of tests, describe blocks can help you to quickly find the ones you’re looking for:

describe('the Nav component', () => {
	test('should render the Contact link', () => {
		render(
			<App>
				<Nav />
			</App>,
		);
		screen.getByText('Contact');
	});

	test('should render the Home link', () => {
		render(
			<App>
				<Nav />
			</App>,
		);
		screen.getAllByText('Home');
	});

	// ...more tests

});

2. Import react-router-dom into all test files where you are testing routing logic

I noticed your commented out test in buttons.test.js. You are very close to having a working test here! 🚀

Here are a couple of things you might want to have a look at:

  • The createMemoryHistory method being invoked on line 24 isn’t being imported from history
  • The fireEvent method being invoked on line 29 isn’t being imported from @testing-library/react

I think this test also fails because you are rendering <BackButton to={"nowhere"}/> (line 27, buttons.test.js) outside of a <Router><Router /> component. <BackButton /> internally uses the useHistory() method from react-router-dom, so when you render it without wrapping it a top level <Router><Router /> component, your code doesn’t recognise the react-router-dom method. Here’s a StackOverflow thread about the same issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant