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

[legacy-framework] Fix typescript jest issue in forgotPassword.test.ts #2110

Merged
merged 10 commits into from
Mar 17, 2021
14 changes: 7 additions & 7 deletions examples/auth/app/auth/mutations/forgotPassword.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {hash256, Ctx} from "blitz"
import { hash256, Ctx } from "blitz"
import forgotPassword from "./forgotPassword"
import db from "db"
import previewEmail from "preview-email"
Expand All @@ -8,15 +8,15 @@ beforeEach(async () => {
})

const generatedToken = "plain-token"
jest.mock("@blitzjs/core/server", () => ({
...jest.requireActual("@blitzjs/core/server")!,
jest.mock("blitz", () => ({
...jest.requireActual<object>("blitz")!,
Copy link
Member

Choose a reason for hiding this comment

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

Thanks @roesh!

You have to actually revert this change as it breaks example tests here in the monorepo. Maybe a way to fix it, but needs to be @blitzjs/core/server for now

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, got it. Let me revert that

generateToken: () => generatedToken,
}))
jest.mock("preview-email", () => jest.fn())

describe("forgotPassword mutation", () => {
it("does not throw error if user doesn't exist", async () => {
await expect(forgotPassword({email: "[email protected]"}, {} as Ctx)).resolves.not.toThrow()
await expect(forgotPassword({ email: "[email protected]" }, {} as Ctx)).resolves.not.toThrow()
})

it("works correctly", async () => {
Expand All @@ -34,13 +34,13 @@ describe("forgotPassword mutation", () => {
},
},
},
include: {tokens: true},
include: { tokens: true },
})

// Invoke the mutation
await forgotPassword({email: user.email}, {} as Ctx)
await forgotPassword({ email: user.email }, {} as Ctx)

const tokens = await db.token.findMany({where: {userId: user.id}})
const tokens = await db.token.findMany({ where: { userId: user.id } })
const token = tokens[0]

// delete's existing tokens
Expand Down
30 changes: 14 additions & 16 deletions examples/auth/app/pages/index.test.tsx
Original file line number Diff line number Diff line change
@@ -1,27 +1,25 @@
import {render} from "test/utils"
import { render } from "test/utils"

import Home from "./index"
import { useCurrentUser } from "app/core/hooks/useCurrentUser"

jest.mock("@blitzjs/core", () => ({
...jest.requireActual("@blitzjs/core")!,
useQuery: () => [
{
id: 1,
name: "User",
email: "[email protected]",
role: "user",
},
],
}))
jest.mock("app/core/hooks/useCurrentUser")
const mockUseCurrentUser = useCurrentUser as jest.MockedFunction<typeof useCurrentUser>

test("renders blitz documentation link", () => {
// This is an example of how to ensure a specific item is in the document
// But it's disabled by default (by test.skip) so the test doesn't fail
// when you remove the the default content from the page

// This is an example on how to mock api hooks when testing
mockUseCurrentUser.mockReturnValue({
id: 1,
name: "User",
email: "[email protected]",
role: "user",
})

const {getByText} = render(<Home />)
const element = getByText(/powered by blitz/i)
// @ts-ignore
expect(element).toBeInTheDocument()
const { getByText } = render(<Home />)
const linkElement = getByText(/Documentation/i)
expect(linkElement).toBeInTheDocument()
})
Copy link
Member

Choose a reason for hiding this comment

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

So some change in this file is still causing tests to fail.

Not sure if jest.mock needs to be before the import?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah I see. Should the test syncing be a separate PR then? I can commit just the single change to satisfy typescript and we can update the auth tests later?
Also I'm actually not sure how to trigger the CI/CD tests, is there a way to retrigger them and check if the changes I made are OK in the monorepo?

Copy link
Member

Choose a reason for hiding this comment

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

it's up to you. If we get tests passing, then fine to keep the change here.

CI auto runs on each git push, but you can run these failing tests locally by running yarn jest inside examples/auth

Copy link
Collaborator Author

@roshan-sama roshan-sama Mar 17, 2021

Choose a reason for hiding this comment

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

Ah, I see. I reverted back to the original tests, with the only change being the <object> before their jest.requireActual reference. I tried to troubleshoot the index.test.tsx file but I'm actually ran into issues with the forgotpassword mutation as well, so I reverted that back to its original version too. I made sure to run yarn jest this time and all tests passed.
EDIT: Lint failed, I have to make sure I run that too, let me to that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, that was on me, I forgot to push the changes I'd made after resolving a merge conflict, I reran the yarn jest command and eslint --fix and lint, and those passed and waiting for the results of the CI tests

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ beforeEach(async () => {

const generatedToken = "plain-token"
jest.mock("blitz", () => ({
...jest.requireActual("blitz")!,
...jest.requireActual<object>("blitz")!,
generateToken: () => generatedToken,
}))
jest.mock("preview-email", () => jest.fn())
Expand Down