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

Testing with usePageLoadingContext #1596

Open
jvanbaarsen opened this issue Dec 29, 2020 · 7 comments
Open

Testing with usePageLoadingContext #1596

jvanbaarsen opened this issue Dec 29, 2020 · 7 comments

Comments

@jvanbaarsen
Copy link
Contributor

jvanbaarsen commented Dec 29, 2020

I was wondering if it makes sense to create some form of helper or something to easily mock out the usePageLoadingContext. I'm now doing it this way:

import { render, screen } from '@redwoodjs/testing'
import { usePageLoadingContext } from '@redwoodjs/router'
import PageLoader from './PageLoader'

jest.mock('@redwoodjs/router')

describe('PageLoader', () => {
  it('renders a loading block when usePageLoadingContext returns true', () => {
    usePageLoadingContext.mockReturnValueOnce({ loading: true })
    render(<PageLoader />)
    expect(screen.getByText('Loading')).toBeInTheDocument()
  })

  it('doesnt render anything when usePageLoadingContext returns false', () => {
    usePageLoadingContext.mockReturnValueOnce({ loading: false })
    render(<PageLoader />)
    expect(screen.queryByText('Loading')).not.toBeInTheDocument()
  })
})

But it feels super tedious having to do this in every test for every component that uses the PageLoader component.

There is a good chance that I'm not doing things the way that Jest/React Testing Library thinks it should be done

@thedavidprice
Copy link
Contributor

Hi @jvanbaarsen Sorry this one's gone without a reply for so long!

We've been focusing on this kind of test + mock at the Cell level -- have you seen the boilerplate test and mock files that come with a generated Cell component?

If that doesn't fit your use case, let's definitely explore this further.

@jvanbaarsen
Copy link
Contributor Author

@thedavidprice I'll check that! I'm not sure if something on a Cell level would be good enough, since I'm using the usePageLoading in a layout. I was under the impression that cells were meant for loading GraphQL data. Or am I missing something?

@thedavidprice
Copy link
Contributor

...since I'm using the usePageLoading in a layout. I was under the impression that cells were meant for loading GraphQL data. Or am I missing something?

^^ No, you're not missing anything. Correct and Cells == data. Wasn't sure about your use case.

@dac09 has quite a bit of experience handling loading UX for his app Tape.sh. Any thoughts/suggestion about this, Danny?

@dac09
Copy link
Collaborator

dac09 commented Jan 13, 2021

I've not done this specifically, but a good idea might be to use beforeEach and mock it there to be true, that way you don't have to repeat yourself in every test :)

Also, is there a reason to test for the page loader every time?

@jvanbaarsen
Copy link
Contributor Author

@dac09 No, I don't really wanna test it every time, I'd be fine with testing it just once in the test for the actual Page component. But since React Testing Library (RTL) does a full render of all the components in the tree the layout gets rendered in every test, even the ones that are not about the Layout. So all components that are wrapped inside the page. That way RTL throws an error every time I'm not mocking the context.

@jtoar
Copy link
Contributor

jtoar commented May 28, 2021

Also, is there a reason to test for the page loader every time?

@jvanbaarsen I'm also wondering this; it feels like testing usePageLoadingContext should be on us as a framework not you. This looks like a test that we should have:

describe('PageLoader', () => {
  it('renders a loading block when usePageLoadingContext returns true', () => {
    // ...

@jvanbaarsen
Copy link
Contributor Author

@jtoar Yeah I'm fine with considering this an "implementation detail". But that still means I currently would need to mock it out in every test that comes by the context.

It would be great if out of the box Redwood would mock this call (A bit the same as we do with mockCurrentUser)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

No branches or pull requests

4 participants