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

chore: switch existing tests to new next-test-helper library #629

Closed
wants to merge 1 commit into from

Conversation

bcoe
Copy link
Contributor

@bcoe bcoe commented Jan 2, 2017

switches next.js itself to using the new test helping library next-test-helper that we've been working on.

follows on from: #461

beforeAll(async () => {
app = await setup(dir, next)
})
afterAll(async () => await teardown())
Copy link
Member

Choose a reason for hiding this comment

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

afterAll(teardown) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 wasn't sure if Jest was smart enough to detect that the teardown was async/await I'll give this a try.

beforeAll(() => app.prepare())

afterAll(() => app.close())
beforeAll(async () => {
Copy link
Member

Choose a reason for hiding this comment

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

beforeAll(() => setup(dir, next)) ?

@bcoe bcoe mentioned this pull request Jan 3, 2017
import pkg from '../package.json'
import next from '../dist/server/next'
import {expectElement, setup, render, teardown} from 'next-test-helper'
Copy link
Contributor

Choose a reason for hiding this comment

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

I hope we need to expose some low level helpers rather these generic helpers.
For an example, here are few cases:

  • Get the programmatic API
  • Run an app by giving the path
  • Get a random available port

There are many ways, we could write tests. It's depend on test case to case.
Specially with Next.js integration tests, it gets so crazy. Sometimes, we may need to do npm install.

@arunoda
Copy link
Contributor

arunoda commented Jan 4, 2017

Why do we create another repo for this?
We've a babel preset sits inside next/babel.

Just like that, I think we could expose this test helper.

@rauchg
Copy link
Member

rauchg commented Jan 10, 2017

@arunoda is this superseded by the other PR ?

@arunoda
Copy link
Contributor

arunoda commented Jan 10, 2017

This introduce a set of helpers. They seems like testing meta helpers like setup, render. But I expect to have more low level APIs like rendering a page via HTTP, API or via webdriver feels much better than this.

So, we could use them easily with any testing service and no information is hidden. I have introduced a set of helpers like that in the other PR.

@rauchg
Copy link
Member

rauchg commented Jan 10, 2017

So, for next.js core, the higher level the tests (HTTP), the better. I'll close this for now.

@rauchg rauchg closed this Jan 10, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants