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

test: Resolve warnings and errors in Jest tests #9784

Merged
merged 7 commits into from
Oct 17, 2023

Conversation

jdamore-linode
Copy link
Contributor

Description 📝

This PR aims to clean up our Jest tests a bit by resolving some of the errors and warnings that get logged to the console when running Jest. Specifically, this aims to fix issues within the tests themselves -- warnings and errors that need to be solved by modifying the source files themselves are out of scope here.

I plan to circle back tomorrow with a self review and comparison showing how the changes impact the Jest output.

How to test 🧪

We can rely on GitHub Actions for this, but you can also run the tests locally:

yarn test

@jdamore-linode jdamore-linode requested a review from a team as a code owner October 11, 2023 20:35
@jdamore-linode jdamore-linode requested review from coliu-akamai and tyler-akamai and removed request for a team October 11, 2023 20:35
@jdamore-linode jdamore-linode self-assigned this Oct 11, 2023
Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

Nice! Lots of warnings/errors are gone compared to develop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A toast notification appears when an attempt by the user to download their kubeconfig fails for any reason. This failure always occurs from within this Jest test since we're not mocking the download request, and we can't make real Linode API requests via Jest.

Since the app attempts to show a toast notification, Jest logs an error because Notistack is not actually set up and available from within the context of the Jest test. This change resolves the Jest error by mocking Notistack's useSnackbar export. However, it may be a better solution to mock the HTTP request to prevent the toast notification in the first place.

Before After
ClusterActionMenu BEFORE ClusterActionMenu AFTER

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The AddonsPanel component makes a bunch of requests to fetch Images, but the requests aren't being mocked by MSW. This causes Jest to log ECONNREFUSED errors for each failed HTTP request, as well as warnings from MSW relating to the unhandled requests. The fix here is to mock those GET requests to the Image endpoints.

Before After
AddonsPanel BEFORE 1
AddonsPanel BEFORE 2 AddonsPanel AFTER

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even though testRow is async, it indirectly calls act() which cannot be executed concurrently. This change fixes the problem by awaiting each call to testRow() to ensure that act() calls do not overlap.

Before After
LongviewPlans BEFORE LongviewPlans AFTER

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case, the errors logged by Jest are expected since we're deliberately testing an error condition. This change suppresses these errors to reduce log noise.

Before After
BucketLanding BEFORE BucketLanding AFTER

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar to BucketLanding.test.tsx, we're testing an error condition so the Jest error is expected, and we can safely suppress it to reduce log noise. However, there is also a Formik warning being logged; I think resolving that will require a change in the form itself.

Before After
CreateBucketDrawer BEFORE CreateBucketDrawer AFTER

Copy link
Contributor Author

@jdamore-linode jdamore-linode Oct 16, 2023

Choose a reason for hiding this comment

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

Similar to AddonsPanel.test.tsx, this component makes requests to fetch Linode types; these are unhandled by the test, which causes ECONNREFUSED errors and MSW warnings to be logged. Mocking the requests resolves them.

Before After
SearchLanding BEFORE 1SearchLanding BEFORE 2SearchLanding BEFORE 3 SearchLanding AFTER

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're getting a DOM validation error here because we're not wrapping the plan selection table rows in a table. Using the wrapWithTableBody helper resolves the errors. This is a rare case where the DOM validation error is caused by the test; other DOM validation errors that get logged by React are actual issues with our component markup.

Before After
PlanSelection BEFORE PlanSelection AFTER

Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

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

Thanks Joe! Confirmed that the errors you commented on the specific files are gone! Also confirming that there are a lot fewer errors from yarn test 🎉

@coliu-akamai coliu-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Oct 16, 2023
@tyler-akamai
Copy link
Contributor

Confirmed the tests now pass, great job!

Screenshot 2023-10-16 at 5 33 06 PM

also thank you for the self review!

@jdamore-linode jdamore-linode merged commit 8dee619 into linode:develop Oct 17, 2023
11 checks passed
abailly-akamai pushed a commit to abailly-akamai/manager that referenced this pull request Oct 17, 2023
* Clean up warnings by mocking API requests

* Fix longview test overlapping act() warnings

* Fix PlanSelection test warning by wrapping table row components in a table body

* Mock snackbar hook to resolve warning about updating state on unmounted component

* Suppress React Query errors in tests where errors are expected

* Added changeset: Clean up Jest warnings and errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants