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

Frontend Tests, hooks and utils #437

Merged
merged 67 commits into from
Nov 5, 2023

Conversation

austin-bagwell
Copy link
Contributor

Using Vitest as testing framework, this sets up some basic tests for custom hooks and utility functions. More tests to come!

src/__tests__/hooks/useKeyPress.test.js Outdated Show resolved Hide resolved
src/__tests__/hooks/useOnClickOutside.test.js Show resolved Hide resolved
src/__tests__/hooks/useWindowSize.test.js Outdated Show resolved Hide resolved
src/__tests__/util/errors.test.js Outdated Show resolved Hide resolved
src/__tests__/util/isChrome.test.js Outdated Show resolved Hide resolved
src/__tests__/hooks/useDebounce.test.js Outdated Show resolved Hide resolved
src/__tests__/hooks/useDebounce.test.js Outdated Show resolved Hide resolved
Comment on lines 53 to 54
const insideElement = document.createElement('div');
ref.current.appendChild(insideElement);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use the outsideElement code here instead as the handler would only be called if the click was on an element outside the one hosting the hook.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed through new changes, but I'm not 100% sure that my test is doing the right thing.

I changed this so that the event is dispatched to outerElement after unmounting the ref element hosting the hook, but for some reason it only works if I don't use act(). Keeping the both unmount() and outsideElement.dispatchEvent(event) inside the act() scope caused the test to fail, removing act results in a failed test with the same assertion.

I read the docs and this related post, but I'm still not 100% sure what is going on.

Am I actually testing the correct behavior this way?

@robert-w-gries

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AB - use RTL debug to check that all DOM elements are present. Can also get more specific with assertions to double check that correct behavior is being tested


outsideElement.dispatchEvent(event);
expect(handler).toHaveBeenCalledTimes(1);
expect(handler).not.toHaveBeenCalledTimes(2);
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: if you've already checked it was called once, the second assert is redundant

@robert-w-gries robert-w-gries marked this pull request as ready for review November 5, 2023 18:52
@robert-w-gries robert-w-gries merged commit dae0d3c into deardurham:master Nov 5, 2023
1 check passed
@robert-w-gries robert-w-gries linked an issue Nov 5, 2023 that may be closed by this pull request
12 tasks
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

Successfully merging this pull request may close these issues.

Front End Tests
2 participants