Skip to content

chore(site): update and refactor all custom hook tests that rely on React Router #12219

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.

Sign up for GitHub

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

Merged
merged 20 commits into from
Mar 8, 2024
Merged

chore(site): update and refactor all custom hook tests that rely on React Router #12219

Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
0b186bf
chore: rename useTab to useSearchParamsKey and add test
Parkreiner Feb 19, 2024
a7a0944
chore: mark old renderHookWithAuth as deprecated (temp)
Parkreiner Feb 19, 2024
ec3ba4f
fix: update imports for useResourcesNav
Parkreiner Feb 19, 2024
6d7ef9f
Merge branch 'main' into mes/hook-test-revamps-4
Parkreiner Mar 3, 2024
afffb26
refactor: change API for useSearchParamsKey
Parkreiner Mar 3, 2024
590f02b
chore: let user pass in their own URLSearchParams value
Parkreiner Mar 3, 2024
9e00ea6
refactor: clean up comments for clarity
Parkreiner Mar 3, 2024
d7110c5
fix: update import
Parkreiner Mar 3, 2024
2cc63d7
wip: commit progress on useWorkspaceDuplication revamp
Parkreiner Mar 3, 2024
26089e3
chore: migrate duplication test to new helper
Parkreiner Mar 3, 2024
8057397
refactor: update code for clarity
Parkreiner Mar 3, 2024
f8f87a3
refactor: reorder test cases for clarity
Parkreiner Mar 3, 2024
dcc89dc
refactor: split off hook helper into separate file
Parkreiner Mar 3, 2024
1ab6f03
refactor: remove reliance on internal React Router state property
Parkreiner Mar 4, 2024
b32603f
refactor: move variables around for more clarity
Parkreiner Mar 4, 2024
b0fabde
refactor: more updates for clarity
Parkreiner Mar 4, 2024
b94decc
refactor: reorganize test cases for clarity
Parkreiner Mar 4, 2024
3e41877
refactor: clean up test cases for useWorkspaceDupe
Parkreiner Mar 8, 2024
1fcf9e2
refactor: clean up test cases for useWorkspaceDupe
Parkreiner Mar 8, 2024
13f5e53
Merge branch 'main' into mes/hook-test-revamps-4
Parkreiner Mar 8, 2024
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
Prev Previous commit
Next Next commit
refactor: update code for clarity
  • Loading branch information
@Parkreiner
Parkreiner committed Mar 3, 2024
commit 8057397d7855ca996f3f17f25ac9f7f2423ef9cc
32 changes: 14 additions & 18 deletions site/src/pages/CreateWorkspacePage/useWorkspaceDuplication.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ type RenderHookResult = RenderResult["result"];

async function performNavigation(
result: RenderHookResult,
getSnapshot: GetLocationSnapshot,
getLocationSnapshot: GetLocationSnapshot,
) {
await waitFor(() => expect(result.current.isDuplicationReady).toBe(true));
void act(() => result.current.duplicateWorkspace());
Copy link
Member

Choose a reason for hiding this comment

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

another void/await?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup – will fix


const templateName = MockWorkspace.template_name;
return waitFor(() => {
const { pathname } = getSnapshot();
const { pathname } = getLocationSnapshot();
expect(pathname).toEqual(`/templates/${templateName}/workspace`);
});
}
Expand Down Expand Up @@ -66,12 +66,6 @@ describe(`${useWorkspaceDuplication.name}`, () => {
});

test("Navigating populates the URL search params with the workspace's build params", async () => {
const { result, getLocationSnapshot } = await render(MockWorkspace);
await performNavigation(result, getLocationSnapshot);

const { search } = getLocationSnapshot();
const parsedParams = new URLSearchParams(search);

const mockBuildParams = [
M.MockWorkspaceBuildParameter1,
M.MockWorkspaceBuildParameter2,
Expand All @@ -80,27 +74,29 @@ describe(`${useWorkspaceDuplication.name}`, () => {
M.MockWorkspaceBuildParameter5,
];

const { result, getLocationSnapshot } = await render(MockWorkspace);
await performNavigation(result, getLocationSnapshot);

const { search } = getLocationSnapshot();
for (const { name, value } of mockBuildParams) {
const key = `param.${name}`;
expect(parsedParams.get(key)).toEqual(value);
expect(search.get(key)).toEqual(value);
}
});

test("Navigating appends other necessary metadata to the search params", async () => {
const { result, getLocationSnapshot } = await render(MockWorkspace);
await performNavigation(result, getLocationSnapshot);

const { search } = getLocationSnapshot();
const parsedParams = new URLSearchParams(search);

const extraMetadataEntries = [
const extraMetadataEntries: readonly [string, string][] = [
Copy link
Member

Choose a reason for hiding this comment

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

why the switch from as const?

Copy link
Member Author

@Parkreiner Parkreiner Mar 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I felt like it would surface any accidental errors in a better way, and that it does a better job of communicating intent

With the previous code, I could do something like this:

const extraMetadataEntries = [
  ["mode", "duplicate"],
  ["name", `${MockWorkspace.name}-copy`],
  ["version", MockWorkspace.template_active_version_id],

  function obviouslyWrong() {},
  null,
  ["forgotToAddTheSecondTupleElement"]
] as const;

And this would be valid still – the underlying type would get broadened. So while the type mismatch would get caught down the line, redefining the type to be an array of string tuples tells you what types should go in, and is more defensive against typos

["mode", "duplicate"],
["name", `${MockWorkspace.name}-copy`],
["version", MockWorkspace.template_active_version_id],
] as const;
];

const { result, getLocationSnapshot } = await render(MockWorkspace);
await performNavigation(result, getLocationSnapshot);

const { search } = getLocationSnapshot();
for (const [key, value] of extraMetadataEntries) {
expect(parsedParams.get(key)).toBe(value);
expect(search.get(key)).toBe(value);
}
});
});