1
0
mirror of synced 2025-12-19 18:10:59 -05:00

Clean up jest (#50177)

This commit is contained in:
Peter Bengtsson
2024-04-16 15:38:46 -04:00
committed by GitHub
parent a863eab83f
commit 227e4e2bbf
24 changed files with 55 additions and 3071 deletions

View File

@@ -1,6 +1,6 @@
name: Test changed content
# **What it does**: Runs the jest tests for changed and deleted content files.
# **What it does**: Runs the vitest tests for changed and deleted content files.
# **Why we have it**: Use GitHub Actions to run tests on changed content files.
# **Who does it impact**: Docs engineering, open-source engineering contributors.

View File

@@ -3,7 +3,6 @@ src/bookmarklets/
src/open-source/scripts/add-pr-links.js
/.next/
# jest --coverage option
/.coverage
/coverage

2897
package-lock.json generated

File diff suppressed because it is too large Load Diff

View File

@@ -35,7 +35,7 @@
"index-test-fixtures": "npm run index-elasticsearch -- -l en -l ja -V ghec -V dotcom --index-prefix tests -- src/search/tests/fixtures/search-indexes",
"lint": "eslint '**/*.{js,mjs,ts,tsx}'",
"lint-content": "node src/content-linter/scripts/lint-content.js",
"lint-translation": "cross-env NODE_OPTIONS=--experimental-vm-modules vitest src/content-linter/tests/lint-files.js",
"lint-translation": "vitest src/content-linter/tests/lint-files.js",
"generate-code-scanning-query-list": "tsx src/code-scanning/scripts/generate-code-scanning-query-list.ts",
"generate-content-linter-docs": "tsx src/content-linter/scripts/generate-docs.ts",
"move-content": "node src/content-render/scripts/move-content.js",
@@ -63,7 +63,7 @@
"sync-search-indices": "node src/search/scripts/sync-search-indices.js",
"sync-search-server": "cross-env NODE_ENV=production PORT=4002 MINIMAL_RENDER=true CHANGELOG_DISABLED=true tsx src/frame/server.ts",
"sync-webhooks": "src/rest/scripts/update-files.js -o webhooks",
"test": "cross-env NODE_OPTIONS='--max_old_space_size=4096 --experimental-vm-modules' vitest",
"test": "vitest",
"test-local-dev": "node src/workflows/test-local-dev.js",
"test-moved-content": "tsx src/content-render/scripts/test-moved-content.ts",
"tsc": "tsc --noEmit",
@@ -283,7 +283,6 @@
"@github/markdownlint-github": "^0.4.1",
"@graphql-inspector/core": "^5.0.0",
"@graphql-tools/load": "^8.0.0",
"@jest/globals": "29.7.0",
"@octokit/rest": "^20.0.2",
"@playwright/test": "1.43.0",
"@types/imurmurhash": "^0.1.4",
@@ -312,10 +311,6 @@
"graphql": "^16.8.1",
"http-status-code": "^2.1.0",
"husky": "^9.0.8",
"jest": "29.7.0",
"jest-expect-message": "1.1.3",
"jest-fail-on-console": "^3.1.1",
"jest-slow-test-reporter": "^1.0.0",
"json-schema-merge-allof": "^0.8.1",
"kill-port": "2.0.1",
"lint-staged": "^15.0.1",
@@ -335,18 +330,13 @@
"robots-parser": "^3.0.0",
"sass": "^1.52.3",
"start-server-and-test": "^2.0.3",
"ts-jest": "29.1.2",
"typescript": "^5.4.4",
"unist-util-remove": "^4.0.0",
"unist-util-visit-parents": "6.0.1",
"vitest": "1.5.0",
"website-scraper": "^5.3.1"
},
"overrides": {
"ts-jest@29.1.1": {
"@babel/core": "7.23.3"
}
},
"overrides": {},
"optionalDependencies": {
"esm": "^3.2.25"
},

View File

@@ -231,9 +231,7 @@ if (diffFiles.length > 0) {
}
if (ymlToLint.length === 0) {
// With this in place, at least one `test()` is called and you don't
// get the `Your test suite must contain at least one test.` error
// from `jest`.
// This is to make sure the file has at least once `describe`.
describe('deliberately do nothing', () => {
test('void', () => {})
})

View File

@@ -22,7 +22,7 @@ describe('annotate', () => {
// but in this case its a short and concise example
// that won't change regularly.
// If it fails, study the output and make sure it's correct.
// If it is indeed correct, run `jest --updateSnapshot` to update it.
// If it is indeed correct, run `vitest --updateSnapshot` to update it.
expect(await renderContent(example)).toMatchSnapshot()
})

View File

@@ -67,12 +67,12 @@ vi.setConfig({ testTimeout: 60 * 1000 })
describe('changed-content', () => {
const changedContentFiles = getChangedContentFiles()
// `jest.each` will throw if the array is empty, so we need to add a dummy
// `test.each` will throw if the array is empty, so we need to add a dummy
// when there are no changed files in the environment.
if (!changedContentFiles.length) changedContentFiles.push(EMPTY)
test.each(changedContentFiles)('changed-content: %s', async (file) => {
// Necessary because `jest.each` will throw if the array is empty
// Necessary because `test.each` will throw if the array is empty
if (file === EMPTY) return
const page = pageList.find((p) => {
@@ -87,7 +87,7 @@ describe('changed-content', () => {
if (!res.ok) {
let msg = `This error happened when rendering from: ${file}\n`
msg +=
'To see the full error from jest re-run the test with DEBUG_MIDDLEWARE_TESTS=true set\n'
'To see the full error from vitest re-run the test with DEBUG_MIDDLEWARE_TESTS=true set\n'
msg += `Or, to view it locally start the server (npm run dev) and visit http://localhost:4000${href}`
console.log(msg)
throw new Error(`Rendering ${href} failed with status ${res.statusCode}`)
@@ -99,12 +99,12 @@ describe('changed-content', () => {
describe('deleted-content', () => {
const deletedContentFiles = getDeletedContentFiles()
// `jest.each` will throw if the array is empty, so we need to add a dummy
// `test.each` will throw if the array is empty, so we need to add a dummy
// when there are no deleted files in the environment.
if (!deletedContentFiles.length) deletedContentFiles.push(EMPTY)
test.each(deletedContentFiles)('deleted-content: %s', async (file) => {
// Necessary because `jest.each` will throw if the array is empty
// Necessary because `test.each` will throw if the array is empty
if (file === EMPTY) return
const page = pageList.find((p) => {

View File

@@ -221,7 +221,7 @@ front: >'matter
describe('get-data on corrupt translations', () => {
let dd
const enDirBefore = languages.en.dir
// Only `en` is available in jest tests, so pretend we also have Japanese
// Only `en` is available in vitest tests, so pretend we also have Japanese
languages.ja = Object.assign({}, languages.en, {})
beforeAll(() => {

View File

@@ -44,18 +44,18 @@ to your assertions.
### What to test
Beyond some basic happy path tests, **only test what `jest` can't test**.
Beyond some basic happy path tests, **only test what `vitest` can't test**.
In particular this means client-side JavaScript interactions. For example,
`jest` can fetch the HTML over HTTP and assert against the `cheerio` parsed
`vitest` can fetch the HTML over HTTP and assert against the `cheerio` parsed
HTML, but it can't test what happens when you click a client-side routing
link that triggers some sort of user agent interaction.
`jest` is always faster. Playwright tests can test things like displaying
`vitest` is always faster. Playwright tests can test things like displaying
different things depending on cookies or `localStorage`. Playwright tests
can test the visual presence of something. For example, if something
like `<div style="display:none">Text here</div>` is in the DOM only
Playwright can understand that it's not actually present in the page since
`jest` and Cheerio can't understand CSS.
`vitest` and Cheerio can't understand CSS.
Think of your headless tests as "What would a human QA person do?"
The imaginary QA person can be you. If there's something you find yourself

View File

@@ -33,7 +33,7 @@ Deeper than the product level, the names and directories can be whatever
you want it to be.
Once you've found a place to put some fixture content, before writing
a `jest` test, you can preview your changes using:
a `vitest` test, you can preview your changes using:
```shell
npm run fixture-dev
@@ -53,7 +53,7 @@ about end-to-end testing a new custom Liquid tag called
To run the tests use:
```shell
ROOT=src/fixtures/fixtures jest src/fixtures/tests
ROOT=src/fixtures/fixtures vitest src/fixtures/tests
```
### Exceptions
@@ -86,6 +86,6 @@ It's less likely now that your tests break because of some other change.
Similar to unit testing strategies, try to keep things in small units that
worries about *one thing at a time*.
Don't be afraid to write a `jest` test that is very specific about what it
Don't be afraid to write a `vitest` test that is very specific about what it
tests. It might seem strange when someone is only reading the tests directly.
But the fixtures are part of the tests. It's just in different files.

View File

@@ -38,7 +38,7 @@ describe('FailBot', () => {
const backendPromises = FailBot.report(err, { foo: 'bar' })
// Note! You don't need to await the promises it returns to be
// able to use `FailBot.report()`. It will send.
// But here in the context of jest, we need to await *now*
// But here in the context of vitest, we need to await *now*
// so we can assert that it did make the relevant post requests.
// Once we've done this, we can immediate check what it did.
await Promise.all(await backendPromises)

View File

@@ -177,7 +177,7 @@ async function getPageInfoFromCache(page, pathname) {
// followed by a CDN purge).
// In development (local preview), the performance doesn't really matter.
// In CI, we use the caching because the CI runs
// `npm run precompute-pageinfo` right before it runs jest tests.
// `npm run precompute-pageinfo` right before it runs vitest tests.
}
info.cacheInfo = cacheInfo
return info

View File

@@ -35,7 +35,7 @@ Object.values(allVersions).forEach((info) => {
function getIndexPrefix() {
// This logic is mirrored in the scripts we use before running tests
// In particular, see the `index-test-fixtures` npm script.
// That's expected to be run before CI and local jest testing.
// That's expected to be run before CI and local vitest testing.
// The reason we have a deliberately different index name (by prefix)
// for testing compared to regular operation is to make it convenient
// for engineers working on local manual testing *and* automated

View File

@@ -7,7 +7,7 @@
* ELASTICSEARCH_URL=http://localhost:9200 npm run index-test-fixtures
*
* This will replace any "real" Elasticsearch indexes you might have so
* once you're done working on jest tests you need to index real
* once you're done working on vitest tests you need to index real
* content again.
*/

View File

@@ -7,7 +7,7 @@
* ELASTICSEARCH_URL=http://localhost:9200 npm run index-test-fixtures
*
* This will replace any "real" Elasticsearch indexes you might have so
* once you're done working on jest tests you need to index real
* once you're done working on vitest tests you need to index real
* content again.
*/

View File

@@ -114,7 +114,7 @@ describe('rate limiting', () => {
const newRemaining = parseInt(res.headers['ratelimit-remaining'])
expect(newLimit).toBe(limit)
// Can't rely on `newRemaining == remaining - 1` because of
// concurrency of jest-running.
// concurrency of test-running.
expect(newRemaining).toBeLessThan(remaining)
}
})

View File

@@ -5,10 +5,9 @@ always open a pull request and rely on the CI service to run tests for you,
but it's helpful to run tests locally before pushing your changes to
GitHub.
Tests are written using [jest](https://ghub.io/jest), a framework maintained
by Facebook and used by many teams at GitHub.
Jest provides everything: a test runner, an assertion library, code coverage analysis,
custom reporters for different types of test output, etc.
Tests are written using [vitest](https://vitest.dev/).
`vitest` runs tests and handles assertions.
### Install optional dependencies
@@ -48,7 +47,7 @@ You can run specific tests in two ways:
# The TEST_NAME can be a filename, partial filename, or path to a file or directory
npm test -- <TEST_NAME>
NODE_OPTIONS=--experimental-vm-modules npx jest tests/unit
vitest path/to/tests/directory
```
### Failed Local Tests
@@ -74,8 +73,8 @@ npm run lint
### Keeping the server running
When you run `jest` tests that depend on making real HTTP requests
to `localhost:4000`, the `jest` tests have a hook that starts the
When you run `vitest` tests that depend on making real HTTP requests
to `localhost:4000`, the `vitest` tests have a hook that starts the
server before running all/any tests and stops the server when done.
You can disable this, which might make it easier when debugging tests
@@ -90,13 +89,13 @@ NODE_ENV=test PORT=4000 tsx src/frame/server.ts
In another terminal, type:
```shell
START_JEST_SERVER=false jest tests/rendering/foo/bar.js
START_VITEST_SERVER=false vitests src/versions/tests
```
Or whatever the testing command you use is.
The `START_JEST_SERVER` environment variable needs to be set to `false`, or else `jest` will try to start
a server on `:4000` too.
The `START_VITEST_SERVER` environment variable needs to be set to `false`,
or else `vitest` will try to start a server on `:4000` too.
### Debugging middleware errors
@@ -109,7 +108,7 @@ error is happening, set `$DEBUG_MIDDLEWARE_TESTS` to `true`. For example:
```shell
export DEBUG_MIDDLEWARE_TESTS=true
jest tests/rendering/ -b
vitest src/shielding/tests -b
```
### Fixture based testing

View File

@@ -1,50 +0,0 @@
import { jest } from '@jest/globals'
export function coreMock() {
return {
info: jest.fn(),
warn: jest.fn(),
error: jest.fn(console.error),
setOutput: jest.fn(),
}
}
export function octokitMock({ requestMock, listForRepoMock } = {}) {
return {
request: jest.fn(requestMock),
rest: {
issues: {
listForRepo: jest.fn(listForRepoMock),
createComment: jest.fn(),
update: jest.fn(),
},
},
}
}
export function cheerioMock(argToValueMap) {
return {
load: jest.fn(async () => {
return (arg) => {
return argToValueMap[arg]
}
}),
}
}
export function gotMock({ status } = {}) {
return jest.fn(async () => {
if (status < 200 || status >= 400) {
throw new Error({
status,
})
}
return new Error({
status,
})
})
}
export function uploadArtifactMock() {
return jest.fn()
}

View File

@@ -25,7 +25,7 @@ function stripLiquid(text) {
// Given a URI that does not start with a specific language,
// return undefined if it can found as a known page.
// Otherwise, return an object with information that is used to
// print a useful jest error message in the assertion.
// print a useful test error message in the assertion.
export function checkURL(uri, index, redirectsContext) {
const url = `/en${stripLiquid(uri).split('#')[0]}`
if (!(url in redirectsContext.pages)) {

View File

@@ -1,12 +0,0 @@
import failOnConsole from 'jest-fail-on-console'
// Jest tests don't fail in some cases where we would see an error in DevTools
// Console when running locally and we would also see the error in the test
// output. This includes the React `Each child in a list should have a unique
// "key" prop` error example.
//
// To catch this and fail tests in cases like this, we use `jest-fail-on-console`
// to fail on calls to `console.error()`.
failOnConsole({
shouldFailOnWarn: false,
})

View File

@@ -1,13 +0,0 @@
#!/usr/bin/env node
import { START_JEST_SERVER, isServerHealthy, killServer } from './server-for-jest.js'
export default async () => {
if (START_JEST_SERVER) {
global.__SERVER__.close()
if (await isServerHealthy()) {
killServer()
}
}
}

View File

@@ -1,35 +0,0 @@
import kill from 'kill-port'
import portUsed from 'port-used'
import got, { RequestError } from 'got'
export const PORT = 4000
// By default it's on
export const START_JEST_SERVER = Boolean(JSON.parse(process.env.START_JEST_SERVER || 1))
export async function isServerHealthy() {
try {
const res = await got.head(`http://localhost:${PORT}/healthz`, { retry: { limit: 0 } })
return res.statusCode === 200
} catch (err) {
// This exception is thrown if you can't even connect.
if (err instanceof RequestError) {
return false
}
throw err
}
}
export function killServer() {
kill(PORT, 'tcp')
.then(() => {
console.log(`Killed what was on :${PORT}`)
})
.catch((error) => {
console.log(`Unable to kill whatever was on :${PORT}:`, error)
})
}
export async function isPortRunning() {
return await portUsed.check(PORT)
}

View File

@@ -1,31 +0,0 @@
#!/usr/bin/env node
import { main } from '#src/frame/start-server.js'
import { PORT, START_JEST_SERVER, isServerHealthy, isPortRunning } from './server-for-jest.js'
export default async () => {
if (START_JEST_SERVER) {
console.log(`Starting a server for jest on port :${PORT}.`)
process.env.NODE_ENV = 'test'
// Has to be this because that's what the end-to-end tests expect
process.env.PORT = `${PORT}`
if (await isPortRunning()) {
console.error(`Something's already running on :${PORT}`)
console.log(
'If you intend to run jest tests with an existing server, set env var START_JEST_SERVER=false',
)
process.exit(1)
}
// So it can be accessed from the script that
// is set up by the jest config: `globalTeardown`
global.__SERVER__ = await main()
console.assert(await isServerHealthy())
} else {
console.warn(`jest is NOT automatically starting a server on port :${PORT}`)
}
}

View File

@@ -12,7 +12,7 @@ import got from 'got'
* We use this in CI to make sure the local development server works.
* There are certain things that only work and happen when in
* local dev, that don't make sense to test in regular end-to-end tests
* such as `jest` rendering.
* such as `vitest` rendering.
*
* For engineers to test this locally do the following:
*