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

(jest) improves performance and mem use reduction; adds compatibility with 3rd party tools; fwds all args to jest command #4096

Merged
merged 31 commits into from
Feb 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
2484a84
Setup backwards compatible settings with jest-preset
dac09 Jan 10, 2022
0a651ef
Make db push work from both rw and jest cli
dac09 Jan 10, 2022
d4c5163
Remove duplicate filter flags from test cli
dac09 Jan 10, 2022
140c43f
Remove redundant jest.config.js
dac09 Jan 10, 2022
c27bd21
Merge branch 'main' into fix/standardize-jest-config
dac09 Jan 11, 2022
2ad12d7
Remove prisma reset command from cli test tests
dac09 Jan 11, 2022
f2ca919
Merge branch 'fix/standardize-jest-config' of github.com:dac09/redwoo…
dac09 Jan 11, 2022
3b5d3de
Merge branch 'main' of github.com:redwoodjs/redwood into fix/standard…
dac09 Jan 17, 2022
87c82c9
Merge conflicts, update test
dac09 Jan 17, 2022
1ca53fc
Merge branch 'main' into fix/standardize-jest-config
dac09 Jan 17, 2022
3ad243e
Update jest config to export from a variable, to make it easier to cu…
dac09 Jan 17, 2022
9a3b107
Merge branch 'fix/standardize-jest-config' of github.com:dac09/redwoo…
dac09 Jan 17, 2022
7046d8c
Merge branch 'main' into fix/standardize-jest-config
dac09 Jan 17, 2022
c5f5060
Typo in web config
dac09 Jan 17, 2022
00bc594
Merge branch 'fix/standardize-jest-config' of github.com:dac09/redwoo…
dac09 Jan 17, 2022
40cec50
Merge branch 'main' into fix/standardize-jest-config
dac09 Jan 18, 2022
b4f87b6
Merge branch 'main' into fix/standardize-jest-config
dac09 Jan 20, 2022
bbdb4a4
Also add coverage directory to web preset
dac09 Jan 24, 2022
7eb5d54
Merge branch 'main' into fix/standardize-jest-config
dac09 Jan 24, 2022
8462d2d
Merge branch 'main' into fix/standardize-jest-config
dac09 Jan 27, 2022
7ccda3e
Improve memory usage for api side tests
dac09 Feb 1, 2022
302cb3e
Merge branch 'main' of github.com:redwoodjs/redwood into fix/standard…
dac09 Feb 1, 2022
1525389
Comments from PR suggestions
dac09 Feb 1, 2022
360e4df
Handle duplicate flags the way jest expects
dac09 Feb 2, 2022
8f899da
Merge branch 'main' of github.com:redwoodjs/redwood into fix/standard…
dac09 Feb 2, 2022
d70e8ce
Merge branch 'main' into fix/standardize-jest-config
dac09 Feb 2, 2022
e511644
Merge branch 'main' into fix/standardize-jest-config
dac09 Feb 2, 2022
af48e1d
Update links to docs
dac09 Feb 2, 2022
b04a032
Merge branch 'fix/standardize-jest-config' of github.com:dac09/redwoo…
dac09 Feb 2, 2022
2dcfdd9
Merge branch 'main' into fix/standardize-jest-config
thedavidprice Feb 2, 2022
fd2ff97
Merge branch 'main' into fix/standardize-jest-config
thedavidprice Feb 2, 2022
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
64 changes: 17 additions & 47 deletions packages/cli/src/commands/__tests__/test.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,31 +23,12 @@ afterEach(() => {
jest.clearAllMocks()
})

test('Creates/resets a test db when side has api, before calling jest', async () => {
await handler({
filter: ['api'],
})

expect(execa.mock.results[0].value).toEqual({
cmd: 'yarn rw',
params: ['prisma db push', '--force-reset', '--accept-data-loss'],
})

expect(execa.mock.results[1].value.cmd).toBe('yarn jest')
})
dac09 marked this conversation as resolved.
Show resolved Hide resolved

test('Runs tests for all available sides if no filter passed', async () => {
await handler({})

// Api side runs db reset first
expect(execa.mock.results[0].value).toEqual({
cmd: 'yarn rw',
params: ['prisma db push', '--force-reset', '--accept-data-loss'],
})

expect(execa.mock.results[1].value.cmd).toBe('yarn jest')
expect(execa.mock.results[1].value.params).toContain('web')
expect(execa.mock.results[1].value.params).toContain('api')
expect(execa.mock.results[0].value.cmd).toBe('yarn jest')
expect(execa.mock.results[0].value.params).toContain('web')
expect(execa.mock.results[0].value.params).toContain('api')
})

test('Syncs or creates test database when the flag --db-push is set to true', async () => {
Expand All @@ -56,12 +37,9 @@ test('Syncs or creates test database when the flag --db-push is set to true', as
dbPush: true,
})

expect(execa.mock.results[0].value).toEqual({
cmd: 'yarn rw',
params: ['prisma db push', '--force-reset', '--accept-data-loss'],
})
expect(execa.mock.results[0].value.cmd).toBe('yarn jest')

expect(execa.mock.results[1].value.cmd).toBe('yarn jest')
expect(execa.mock.results[0].value.params).toContain('--projects', 'api')
})

test('Skips test database sync/creation when the flag --db-push is set to false', async () => {
Expand All @@ -78,24 +56,17 @@ test('Runs tests for all available sides if no side filter passed', async () =>
filter: ['bazinga'],
})

// Api side runs db reset first
expect(execa.mock.results[0].value).toEqual({
cmd: 'yarn rw',
params: ['prisma db push', '--force-reset', '--accept-data-loss'],
})

expect(execa.mock.results[1].value.cmd).toBe('yarn jest')
expect(execa.mock.results[1].value.params).toContain('bazinga')
expect(execa.mock.results[1].value.params).toContain('web')
expect(execa.mock.results[1].value.params).toContain('api')
expect(execa.mock.results[0].value.cmd).toBe('yarn jest')
expect(execa.mock.results[0].value.params).toContain('bazinga')
expect(execa.mock.results[0].value.params).toContain('web')
expect(execa.mock.results[0].value.params).toContain('api')
})

test('Runs tests specified side if even with additional filters', async () => {
await handler({
filter: ['web', 'bazinga'],
})

// Api side would have run prisma reset
expect(execa.mock.results[0].value.cmd).not.toBe('yarn rw')
expect(execa.mock.results[0].value.params).not.toContain('api')

Expand Down Expand Up @@ -129,12 +100,11 @@ test('Passes other flags to jest', async () => {
collectCoverage: true,
})

// Second command because api side runs
expect(execa.mock.results[1].value.cmd).toBe('yarn jest')
expect(execa.mock.results[1].value.params).toContain('-u')
expect(execa.mock.results[1].value.params).toContain('--debug')
expect(execa.mock.results[1].value.params).toContain('--json')
expect(execa.mock.results[1].value.params).toContain('--collectCoverage')
expect(execa.mock.results[0].value.cmd).toBe('yarn jest')
expect(execa.mock.results[0].value.params).toContain('-u')
expect(execa.mock.results[0].value.params).toContain('--debug')
expect(execa.mock.results[0].value.params).toContain('--json')
expect(execa.mock.results[0].value.params).toContain('--collectCoverage')
})

test('Passes values of other flags to jest', async () => {
Expand All @@ -144,15 +114,15 @@ test('Passes values of other flags to jest', async () => {
})

// Second command because api side runs
expect(execa.mock.results[1].value.cmd).toBe('yarn jest')
expect(execa.mock.results[0].value.cmd).toBe('yarn jest')

// Note that these below tests aren't the best, since they don't check for order
// But I'm making sure only 2 extra params get passed
expect(execa.mock.results[1].value.params).toEqual(
expect(execa.mock.results[0].value.params).toEqual(
expect.arrayContaining(['--bazinga', false])
)

expect(execa.mock.results[1].value.params).toEqual(
expect(execa.mock.results[0].value.params).toEqual(
expect.arrayContaining(['--hello', 'world'])
)
})
47 changes: 23 additions & 24 deletions packages/cli/src/commands/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { getProject } from '@redwoodjs/structure'
import { errorTelemetry } from '@redwoodjs/telemetry'

import { getPaths } from '../lib'
import c from '../lib/colors'

// https://github.com/facebook/create-react-app/blob/cbad256a4aacfc3084be7ccf91aad87899c63564/packages/react-scripts/scripts/test.js#L39
function isInGitRepository() {
Expand Down Expand Up @@ -56,10 +57,12 @@ export const builder = (yargs) => {
default: true,
})
.epilogue(
`Also see the ${terminalLink(
`For all available flags, run jest cli directly ${c.green(
'yarn jest --help'
)}\n\nAlso see the ${terminalLink(
'Redwood CLI Reference',
'https://redwoodjs.com/reference/command-line-interface#test'
)}`
)}\n`
)
}

Expand All @@ -79,10 +82,20 @@ export const handler = async ({
return []
} else {
// and forward on the other flags
return [
flagName.length > 1 ? `--${flagName}` : `-${flagName}`,
others[flagName],
]
const flagValue = others[flagName]

if (Array.isArray(flagValue)) {
// jest does not collapse flags e.g. --coverageReporters=html --coverageReporters=text
// so we pass it on. Yargs collapses these flags into an array of values
return flagValue.flatMap((val) => {
return [flagName.length > 1 ? `--${flagName}` : `-${flagName}`, val]
})
} else {
return [
flagName.length > 1 ? `--${flagName}` : `-${flagName}`,
flagValue,
]
}
}
})

Expand All @@ -103,7 +116,6 @@ export const handler = async ({
...forwardJestFlags,
collectCoverage ? '--collectCoverage' : null,
'--passWithNoTests',
...jestFilterArgs,
].filter((flagOrValue) => flagOrValue !== null) // Filter out nulls, not booleans because user may have passed a --something false flag

// If the user wants to watch, set the proper watch flag based on what kind of repo this is
Expand All @@ -118,11 +130,6 @@ export const handler = async ({
getProject().sides.forEach((side) => sides.push(side))
}

jestArgs.push(
'--config',
`"${require.resolve('@redwoodjs/testing/config/jest/jest.config.js')}"`
)

if (sides.length > 0) {
jestArgs.push('--projects', ...sides)
}
Expand All @@ -133,18 +140,10 @@ export const handler = async ({
)}/test.db`
const DATABASE_URL = process.env.TEST_DATABASE_URL || cacheDirDb

if (sides.includes('api') && dbPush) {
// Sync||create test database
await execa(
`yarn rw`,
['prisma db push', '--force-reset', '--accept-data-loss'],
{
cwd: rwjsPaths.api.base,
stdio: 'inherit',
shell: true,
env: { DATABASE_URL },
}
)
if (sides.includes('api') && !dbPush) {
// @NOTE
// DB push code now lives in packages/testing/config/jest/api/jest-preset.js
process.env.SKIP_DB_PUSH = '1'
}

// **NOTE** There is no official way to run Jest programmatically,
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/middleware/checkForBabelConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ const isUsingBabelRc = () => {
}).length > 0
)
}
const BABEL_SETTINGS_LINK = c.warning('https://redwoodjs.com/docs/babel')
const BABEL_SETTINGS_LINK = c.warning(
'https://redwoodjs.com/docs/project-configuration-dev-test-build'
)

const checkForBabelConfig = () => {
if (isUsingBabelRc()) {
Expand Down
9 changes: 8 additions & 1 deletion packages/create-redwood-app/template/api/jest.config.js
Original file line number Diff line number Diff line change
@@ -1 +1,8 @@
module.exports = require('@redwoodjs/testing/config/jest/api')
// More info at https://redwoodjs.com/docs/project-configuration-dev-test-build

const config = {
thedavidprice marked this conversation as resolved.
Show resolved Hide resolved
rootDir: '../',
preset: '@redwoodjs/testing/config/jest/api',
}

module.exports = config
8 changes: 8 additions & 0 deletions packages/create-redwood-app/template/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// This the Redwood root jest config
// Each side, e.g. ./web/ and ./api/ has specific config that references this root
// More info at https://redwoodjs.com/docs/project-configuration-dev-test-build

module.exports = {
rootDir: '.',
projects: ['<rootDir>/{*,!(node_modules)/**/}/jest.config.js'],
}
9 changes: 8 additions & 1 deletion packages/create-redwood-app/template/web/jest.config.js
Original file line number Diff line number Diff line change
@@ -1 +1,8 @@
module.exports = require('@redwoodjs/testing/config/jest/web')
// More info at https://redwoodjs.com/docs/project-configuration-dev-test-build

const config = {
dac09 marked this conversation as resolved.
Show resolved Hide resolved
rootDir: '../',
preset: '@redwoodjs/testing/config/jest/web',
}

module.exports = config
44 changes: 2 additions & 42 deletions packages/testing/config/jest/api/index.js
Original file line number Diff line number Diff line change
@@ -1,42 +1,2 @@
const path = require('path')

const {
getPaths,
getApiSideDefaultBabelConfig,
} = require('@redwoodjs/internal')

const rwjsPaths = getPaths()
const NODE_MODULES_PATH = path.join(rwjsPaths.base, 'node_modules')
const { babelrc } = getApiSideDefaultBabelConfig()

module.exports = {
roots: ['<rootDir>/src/'],
runner: path.join(__dirname, '../jest-serial-runner.js'),
testEnvironment: path.join(__dirname, './RedwoodApiJestEnv.js'),
displayName: {
color: 'redBright',
name: 'api',
},
setupFilesAfterEnv: [path.join(__dirname, './jest.setup.js')],
moduleNameMapper: {
// @NOTE: Import @redwoodjs/testing in api tests, and it automatically remaps to the api side only
// This is to prevent web stuff leaking into api, and vice versa
'^@redwoodjs/testing$': path.join(
NODE_MODULES_PATH,
'@redwoodjs/testing/api'
),
},
transform: {
'\\.[jt]sx?$': [
'babel-jest',
// When jest runs tests in parallel, it serializes the config before passing down options to babel
// that's why these must be serializable. So ideally, we should just pass reference to a
// configFile or "extends" a config. But we need a few other option only at root level, so we'll pass
// here and remove those keys inside "extend"ed config.
{
babelrc, // babelrc can not reside inside "extend"ed config, that's why we have it here
configFile: path.resolve(__dirname, './apiBabelConfig.js'),
},
],
},
}
// This is for backwards compatibility
module.exports = require('./jest-preset')
83 changes: 83 additions & 0 deletions packages/testing/config/jest/api/jest-preset.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
const path = require('path')

const {
getPaths,
getApiSideDefaultBabelConfig,
} = require('@redwoodjs/internal')

const rwjsPaths = getPaths()
const NODE_MODULES_PATH = path.join(rwjsPaths.base, 'node_modules')
const { babelrc } = getApiSideDefaultBabelConfig()

// @NOTE: is there a better way we could implmenet this?
if (process.env.SKIP_DB_PUSH !== '1') {
const process = require('process')
const path = require('path')
// Load dotenvs
require('dotenv-defaults/config')

const cacheDirDb = `file:${path.join(__dirname, '.redwood', 'test.db')}`
process.env.DATABASE_URL = process.env.TEST_DATABASE_URL || cacheDirDb

const execa = require('execa')
execa.sync(
`yarn rw`,
['prisma db push', '--force-reset', '--accept-data-loss'],
{
cwd: rwjsPaths.api.base,
stdio: 'inherit',
shell: true,
env: {
DATABASE_URL: process.env.DATABASE_URL,
},
}
)

// If its been reset once, we don't need to re-run it for every test
process.env.SKIP_DB_PUSH = '1'
}

module.exports = {
// To make sure other config option which depends on rootDir use
// correct path, for example, coverageDirectory
rootDir: rwjsPaths.base,
roots: [path.join(rwjsPaths.api.src)],
runner: path.join(__dirname, '../jest-serial-runner.js'),
testEnvironment: path.join(__dirname, './RedwoodApiJestEnv.js'),
displayName: {
color: 'redBright',
name: 'api',
},
collectCoverageFrom: [
'**/*.{js,jsx,ts,tsx}',
'!**/node_modules/**',
'!**/dist/**',
],
coverageDirectory: path.join(rwjsPaths.base, 'coverage'),
dac09 marked this conversation as resolved.
Show resolved Hide resolved
watchPlugins: [
'jest-watch-typeahead/filename',
'jest-watch-typeahead/testname',
],
dac09 marked this conversation as resolved.
Show resolved Hide resolved
setupFilesAfterEnv: [path.join(__dirname, './jest.setup.js')],
moduleNameMapper: {
// @NOTE: Import @redwoodjs/testing in api tests, and it automatically remaps to the api side only
// This is to prevent web stuff leaking into api, and vice versa
'^@redwoodjs/testing$': path.join(
NODE_MODULES_PATH,
'@redwoodjs/testing/api'
),
},
transform: {
'\\.[jt]sx?$': [
'babel-jest',
// When jest runs tests in parallel, it serializes the config before passing down options to babel
// that's why these must be serializable. So ideally, we should just pass reference to a
// configFile or "extends" a config. But we need a few other option only at root level, so we'll pass
// here and remove those keys inside "extend"ed config.
{
babelrc, // babelrc can not reside inside "extend"ed config, that's why we have it here
configFile: path.resolve(__dirname, './apiBabelConfig.js'),
},
],
},
}
Loading