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

[UI v2] exercise: Adds test for variables hook that focuses on react query and query caching and invalidation #16158

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

devinvillarosa
Copy link
Contributor

@devinvillarosa devinvillarosa commented Dec 1, 2024

⚠️ This PR is an exercise and has some lessons learned when going doing this. Additionally, this PR may be used as a good reference if we need an example on how to write tests for our react query modules

This PR goes over the exercise of what it may look like adding tests that focuses on our react query modules.
This test covers the integration between how data is fetched, stored, mutated, and fetched again when using @tanstack/query for server data management.
The main test is to ensure our query keys are being properly invalidated and re-fetching data from the server. This should help build confidence when building query modules for different data sources without being dependent on building and testing with UI

A good blog on how to write tests for react query: https://tkdodo.eu/blog/testing-react-query

  1. Adds variables.test.tsx (which may get confusing with our other file of variables.test.tx that tests the route and UX level). These tests our query hooks and its mutations.

💡 Future considerations when going through this exercise:

  1. When calling msw, there is a overlap with https://github.com/PrefectHQ/prefect/blob/main/ui-v2/tests/mocks/node.ts. Maybe we should create an alias, or centralize some type of test utils folder for this. It seems odd to call the server using ../../test/mocks/node.

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • If this pull request adds new functionality, it includes unit tests that cover the changes
  • If this pull request removes docs files, it includes redirect settings in mint.json.
  • If this pull request adds functions or classes, it includes helpful docstrings.

Related to #15512

@devinvillarosa devinvillarosa force-pushed the add-variables-hook-test branch from 03a60ea to 475a68f Compare December 1, 2024 03:13
@devinvillarosa devinvillarosa marked this pull request as ready for review December 1, 2024 03:16
@devinvillarosa devinvillarosa marked this pull request as draft December 1, 2024 03:26
@devinvillarosa devinvillarosa force-pushed the add-variables-hook-test branch 4 times, most recently from b7fb258 to 3d0d6b8 Compare December 1, 2024 17:22
Copy link
Contributor Author

Choose a reason for hiding this comment

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

exporting these queries so we can access the cache data via queryKey programmatically

@devinvillarosa devinvillarosa marked this pull request as ready for review December 1, 2024 22:15
@devinvillarosa devinvillarosa marked this pull request as draft December 1, 2024 23:05
@devinvillarosa devinvillarosa marked this pull request as ready for review December 2, 2024 05:51
@evan-liu
Copy link
Contributor

evan-liu commented Dec 2, 2024

Hi, team, great discussions around the data fetch/cache layer. I followed some of the discussions and am inspired to create a tool for generating some of the code discussed:

  • The TypeScript types that are organized for use with ReactQuery instead of only a mapping of the OpenAPI schema.
  • Factory methods for ReactQuery query options which use a query service talking to the API.
  • MSW handlers to return successful "happy path" responses.

The goal is to have an easy-to-use API and write minimal boilerplate code. Would the team be interested in using it?

BTW,

It seems odd to call the server using ../../test/mocks/node.

Would the below work better?

  • import { server } from "@tests/mocks/node.ts";
  • import { server } from "@mocks/node.ts";

By adding to ui-v2/tsconfig.app.json:

"paths": {
  "@/*": ["./src/*"],
  "@tests/*": ["./tests/*"] // Or "@mocks/*": ["./tests/mocks/*"]
},

@devinvillarosa
Copy link
Contributor Author

Hi, team, great discussions around the data fetch/cache layer. I followed some of the discussions and am inspired to create a tool for generating some of the code discussed:

  • The TypeScript types that are organized for use with ReactQuery instead of only a mapping of the OpenAPI schema.
  • Factory methods for ReactQuery query options which use a query service talking to the API.
  • MSW handlers to return successful "happy path" responses.

The goal is to have an easy-to-use API and write minimal boilerplate code. Would the team be interested in using it?

BTW,

It seems odd to call the server using ../../test/mocks/node.

Would the below work better?

  • import { server } from "@tests/mocks/node.ts";
  • import { server } from "@mocks/node.ts";

By adding to ui-v2/tsconfig.app.json:

"paths": {
  "@/*": ["./src/*"],
  "@tests/*": ["./tests/*"] // Or "@mocks/*": ["./tests/mocks/*"]
},

I think from a dev-ex perspective, as a developer, I just want to import the necessary utilities to help expedite my test creation.

So I think something like:

// Whatever re-usable mocks or util functions I need to start writing
import {  server, createProviderWrapper } from '@test/utils' 
import { useListData } from './some-data-query-hook'
describe("my new data layer hooks", () => {
	it("useListData() is able to fetch from initial cache", async () => {
		const queryClient = new QueryClient();

		// ------------ Mock API requests when cache is empty
                 server.use()

		// ------------ Initialize hooks to test
		const { result } = renderHook(useListData, {
			wrapper: createProviderWrapper({ queryClient }),
		});

		// ------------ Assert
		await waitFor(() => expect(result.current.isSuccess).toBe(true));
		expect(result.current.data).toEqual( /* something */ )
	});


@devinvillarosa devinvillarosa force-pushed the add-variables-hook-test branch from 3d0d6b8 to e232e62 Compare December 2, 2024 23:23
@devinvillarosa devinvillarosa force-pushed the add-variables-hook-test branch from e232e62 to 377fcc5 Compare December 3, 2024 06:30
@devinvillarosa devinvillarosa merged commit b68b4b3 into main Dec 3, 2024
7 checks passed
@devinvillarosa devinvillarosa deleted the add-variables-hook-test branch December 3, 2024 15:26
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.

3 participants