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

feat: allow use of javascript in config file #18061

Merged
merged 68 commits into from
Sep 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
ff3baa1
fix: yarn build
elevatebart Sep 10, 2021
5955046
feat: allow use of javascript in config file
elevatebart Sep 10, 2021
b851c3b
fix: syntax changes
elevatebart Sep 10, 2021
4421239
fix: make changes to desktop gui
elevatebart Sep 10, 2021
3bb303a
fix: cache cypress.config.js powered projects
elevatebart Sep 10, 2021
6c3b0bc
fix: watch dependencies of cypress.config.js
elevatebart Sep 10, 2021
cae9227
fix: allow watcher to watch more than one change
elevatebart Sep 10, 2021
8ad1b4b
Revert "fix: yarn build"
elevatebart Sep 10, 2021
7b06e1b
tests: fix server unit tests
elevatebart Sep 10, 2021
079b666
tests: fix watcher tests
elevatebart Sep 11, 2021
01a1c6c
tests: fix gui files test
elevatebart Sep 11, 2021
5a606cf
fix: better error management of child proc
elevatebart Sep 11, 2021
d82aa70
Merge branch 'develop' into feat/allow-eval-config
Sep 12, 2021
dac75eb
tests: make integration tests pass
elevatebart Sep 13, 2021
81a9a04
Merge branch 'develop' into feat/allow-eval-config
Sep 13, 2021
93b94be
fix: keep merging test type in settings.js
elevatebart Sep 13, 2021
a092d67
build: only one version of css-loader
elevatebart Sep 13, 2021
4144b54
Merge branch 'develop' into feat/allow-eval-config
Sep 13, 2021
416988c
rollback changes to cypress vue examples
elevatebart Sep 13, 2021
bc90f4b
fix: typo in defaults for settings in javascript
Sep 14, 2021
083c05e
fix: reword of language conflict error
Sep 14, 2021
fe42eab
fix: simplify code for settings
Sep 14, 2021
13b603d
use a named export in require_async
elevatebart Sep 14, 2021
089660c
Merge branch 'develop' into feat/allow-eval-config
Sep 14, 2021
8115c02
minore changes per lachlans comments
elevatebart Sep 14, 2021
da13373
fix linting error
elevatebart Sep 14, 2021
0bf4083
fix: make sure that default files are not custom
elevatebart Sep 14, 2021
a1c4007
feat: bubble up the details of read errors on config
elevatebart Sep 14, 2021
af4608e
fix: json failure should return proper error
elevatebart Sep 15, 2021
db85633
Merge branch 'develop' into feat/allow-eval-config
Sep 16, 2021
b52d096
allow errors to happen before project is open
elevatebart Sep 16, 2021
f7cacb3
allow closing projects that have not yet been open
elevatebart Sep 16, 2021
d9ad213
fix errors wording (lang conflict)
Sep 16, 2021
2e562ed
fix errors wording (deprecation warning)
Sep 16, 2021
ac8ca36
in global mode, remove the spinner when error in list mode
elevatebart Sep 16, 2021
85d78a2
allow quotes and doublequotes in errors
elevatebart Sep 16, 2021
36bd9ae
Merge branch 'develop' into feat/allow-eval-config
Sep 17, 2021
1640b02
fix: avoid blocking the process when rebuilding
elevatebart Sep 17, 2021
2534208
Merge branch 'develop' into feat/allow-eval-config
Sep 17, 2021
6b070a7
tests: is server open should be open
elevatebart Sep 17, 2021
d8901fc
Merge branch 'develop' into feat/allow-eval-config
Sep 18, 2021
1e5541d
test: fix one small test
elevatebart Sep 20, 2021
8ea6b92
Merge branch 'develop' into feat/allow-eval-config
Sep 20, 2021
47d36f6
read JSON using a simpler lib
elevatebart Sep 20, 2021
94c6f04
tests: add a test for language conflict
elevatebart Sep 20, 2021
f33fc7f
chore: update vue-template-compiler
elevatebart Sep 21, 2021
7bad158
Merge branch 'develop' into feat/allow-eval-config
Sep 21, 2021
30085ca
fix: make getting configFile async
elevatebart Sep 21, 2021
5ee1b2c
fix loading project
elevatebart Sep 21, 2021
bd8ee94
refactor: rename configFile function
elevatebart Sep 21, 2021
62470d0
refactor: resolve default config file only once
elevatebart Sep 21, 2021
0b7f13f
tests: fix settings unit tests
elevatebart Sep 21, 2021
90fb7c4
fix: run mode initialization to be done too
elevatebart Sep 21, 2021
55951cc
fix: stubbing of run mode readdir
elevatebart Sep 21, 2021
9e461aa
fix: mock readdir
elevatebart Sep 21, 2021
19587ba
scaffold
elevatebart Sep 21, 2021
aac1402
Merge branch 'develop' into feat/allow-eval-config
jennifer-shehane Sep 22, 2021
5499d36
remove all changes to desktop-gui
elevatebart Sep 22, 2021
a1d4727
remove all changes to tests
elevatebart Sep 22, 2021
8c574c8
only reep enabling the use of js in custom files
elevatebart Sep 22, 2021
5453ab5
add unit tests
elevatebart Sep 22, 2021
b4a2455
fix project unit tests
elevatebart Sep 22, 2021
8a59ad0
test: e2e test use of JS
elevatebart Sep 22, 2021
515ecdb
Merge branch 'develop' into feat/allow-eval-config
Sep 22, 2021
e84968e
remove unused projects
elevatebart Sep 22, 2021
c139381
test: fix one test out of the 2 cypress that fail
elevatebart Sep 22, 2021
4fbd0f9
test: fix the other test
elevatebart Sep 22, 2021
9c6b625
Merge branch 'develop' into feat/allow-eval-config
Sep 22, 2021
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
7 changes: 6 additions & 1 deletion packages/server/lib/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ const getMsgByType = function (type, arg1 = {}, arg2, arg3) {
${chalk.yellow('Assign a different port with the \'--port <port>\' argument or shut down the other running process.')}`
case 'ERROR_READING_FILE':
filePath = `\`${arg1}\``
err = `\`${arg2}\``
err = `\`${arg2.type || arg2.code || arg2.name}: ${arg2.message}\``

return stripIndent`\
Error reading from: ${chalk.blue(filePath)}
Expand Down Expand Up @@ -1049,6 +1049,11 @@ const clone = function (err, options = {}) {

if (options.html) {
obj.message = ansi_up.ansi_to_html(err.message)
// revert back the distorted characters
// in case there is an error in a child_process
// that contains quotes
.replace(/\&\#x27;/g, '\'')
.replace(/\&quot\;/g, '"')
} else {
obj.message = err.message
}
Expand Down
13 changes: 11 additions & 2 deletions packages/server/lib/project-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export class ProjectBase<TServer extends ServerE2E | ServerCt> extends EE {
protected _server?: TServer
protected _automation?: Automation
private _recordTests?: any = null
private _isServerOpen: boolean = false

public browser: any
public options: OpenProjectLaunchOptions
Expand Down Expand Up @@ -220,6 +221,8 @@ export class ProjectBase<TServer extends ServerE2E | ServerCt> extends EE {
specsStore,
})

this._isServerOpen = true

// if we didnt have a cfg.port
// then get the port once we
// open the server
Expand Down Expand Up @@ -340,6 +343,10 @@ export class ProjectBase<TServer extends ServerE2E | ServerCt> extends EE {
this.spec = null
this.browser = null

if (!this._isServerOpen) {
return
}

const closePreprocessor = (this.testingType === 'e2e' && preprocessor.close) ?? undefined

await Promise.all([
Expand All @@ -348,6 +355,8 @@ export class ProjectBase<TServer extends ServerE2E | ServerCt> extends EE {
closePreprocessor?.(),
])

this._isServerOpen = false

process.chdir(localCwd)

const config = this.getConfig()
Expand Down Expand Up @@ -534,7 +543,7 @@ export class ProjectBase<TServer extends ServerE2E | ServerCt> extends EE {
}

if (configFile !== false) {
this.watchers.watch(settings.pathToConfigFile(projectRoot, { configFile }), obj)
this.watchers.watchTree(settings.pathToConfigFile(projectRoot, { configFile }), obj)
}

return this.watchers.watch(settings.pathToCypressEnvJson(projectRoot), obj)
Expand All @@ -552,7 +561,7 @@ export class ProjectBase<TServer extends ServerE2E | ServerCt> extends EE {

try {
Reporter.loadReporter(reporter, projectRoot)
} catch (err) {
} catch (err: any) {
const paths = Reporter.getSearchPathsForReporter(reporter, projectRoot)

// only include the message if this is the standard MODULE_NOT_FOUND
Expand Down
93 changes: 93 additions & 0 deletions packages/server/lib/util/require_async.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import _ from 'lodash'
import * as path from 'path'
import * as cp from 'child_process'
import * as inspector from 'inspector'
import * as util from '../plugins/util'
import * as errors from '../errors'
import { fs } from '../util/fs'
import Debug from 'debug'

const debug = Debug('cypress:server:require_async')

let requireProcess: cp.ChildProcess | null

interface RequireAsyncOptions{
projectRoot: string
loadErrorCode: string
/**
* members of the object returned that are functions and will need to be wrapped
*/
functionNames: string[]
}

interface ChildOptions{
stdio: 'inherit'
execArgv?: string[]
}

const killChildProcess = () => {
requireProcess && requireProcess.kill()
requireProcess = null
}

export async function requireAsync (filePath: string, options: RequireAsyncOptions): Promise<any> {
return new Promise((resolve, reject) => {
if (requireProcess) {
debug('kill existing config process')
killChildProcess()
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to return here or is it fine to continue execution?

Copy link
Contributor Author

@elevatebart elevatebart Sep 14, 2021

Choose a reason for hiding this comment

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

These statements are here to avoid leaving 2 processes running at the same time for reading the config.

If the previous one has not responded, we kill it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should not return

}

if (/\.json$/.test(filePath)) {
fs.readJson(path.resolve(options.projectRoot, filePath)).then((result) => resolve(result)).catch(reject)
}

const childOptions: ChildOptions = {
stdio: 'inherit',
}

if (inspector.url()) {
childOptions.execArgv = _.chain(process.execArgv.slice(0))
.remove('--inspect-brk')
.push(`--inspect=${process.debugPort + 1}`)
.value()
}

const childArguments = ['--projectRoot', options.projectRoot, '--file', filePath]

debug('fork child process', path.join(__dirname, 'require_async_child.js'), childArguments, childOptions)
requireProcess = cp.fork(path.join(__dirname, 'require_async_child.js'), childArguments, childOptions)
const ipc = util.wrapIpc(requireProcess)

if (requireProcess.stdout && requireProcess.stderr) {
// manually pipe plugin stdout and stderr for dashboard capture
// @see https://github.com/cypress-io/cypress/issues/7434
requireProcess.stdout.on('data', (data) => process.stdout.write(data))
requireProcess.stderr.on('data', (data) => process.stderr.write(data))
}

ipc.on('loaded', (result) => {
debug('resolving with result %o', result)
resolve(result)
})

ipc.on('load:error', (type, ...args) => {
debug('load:error %s, rejecting', type)
killChildProcess()

const err = errors.get(type, ...args)

// if it's a non-cypress error, restore the initial error
if (!(err.message?.length)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we able to do something like if (!err.isCypressErr)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since I do not want to change the tests drastically, I would prefer not yet.
When we will remove completely the availability of cypress.json I can safely remove all the tests for json and do the work for error management.

I doubt that user code will ever send a Cypress error though. But I might be mistaken.

err.isCypressErr = false
err.message = args[1]
err.code = type
err.name = type
}

reject(err)
})

debug('trigger the load of the file')
ipc.send('load')
})
}
71 changes: 71 additions & 0 deletions packages/server/lib/util/require_async_child.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
require('graceful-fs').gracefulify(require('fs'))
const stripAnsi = require('strip-ansi')
const debug = require('debug')('cypress:server:require_async:child')
const util = require('../plugins/util')
const ipc = util.wrapIpc(process)

require('./suppress_warnings').suppress()

const { file, projectRoot } = require('minimist')(process.argv.slice(2))

run(ipc, file, projectRoot)

/**
* runs and returns the passed `requiredFile` file in the ipc `load` event
* @param {*} ipc Inter Process Comunication protocol
* @param {*} requiredFile the file we are trying to load
* @param {*} projectRoot the root of the typescript project (useful mainly for tsnode)
* @returns
*/
function run (ipc, requiredFile, projectRoot) {
debug('requiredFile:', requiredFile)
debug('projectRoot:', projectRoot)
if (!projectRoot) {
throw new Error('Unexpected: projectRoot should be a string')
}

process.on('uncaughtException', (err) => {
debug('uncaught exception:', util.serializeError(err))
ipc.send('error', util.serializeError(err))

return false
})

process.on('unhandledRejection', (event) => {
const err = (event && event.reason) || event

debug('unhandled rejection:', util.serializeError(err))
ipc.send('error', util.serializeError(err))

return false
})

ipc.on('load', () => {
try {
debug('try loading', requiredFile)
const exp = require(requiredFile)

const result = exp.default || exp

ipc.send('loaded', result)

debug('config %o', result)
} catch (err) {
if (err.name === 'TSError') {
// beause of this https://github.com/TypeStrong/ts-node/issues/1418
// we have to do this https://stackoverflow.com/questions/25245716/remove-all-ansi-colors-styles-from-strings/29497680
const cleanMessage = stripAnsi(err.message)
// replace the first line with better text (remove potentially misleading word TypeScript for example)
.replace(/^.*\n/g, 'Error compiling file\n')

ipc.send('load:error', err.name, requiredFile, cleanMessage)
} else {
const realErrorCode = err.code || err.name

debug('failed to load file:%s\n%s: %s', requiredFile, realErrorCode, err.message)

ipc.send('load:error', realErrorCode, requiredFile, err.message)
}
}
})
}
81 changes: 62 additions & 19 deletions packages/server/lib/util/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,19 @@ const path = require('path')
const errors = require('../errors')
const log = require('../log')
const { fs } = require('../util/fs')
const { requireAsync } = require('./require_async')
const debug = require('debug')('cypress:server:settings')

function jsCode (obj) {
const objJSON = obj && !_.isEmpty(obj)
? JSON.stringify(_.omit(obj, 'configFile'), null, 2)
: `{

}`

return `module.exports = ${objJSON}
`
}

// TODO:
// think about adding another PSemaphore
Expand Down Expand Up @@ -78,7 +91,19 @@ module.exports = {
},

_write (file, obj = {}) {
return fs.outputJsonAsync(file, obj, { spaces: 2 })
if (/\.json$/.test(file)) {
debug('writing json file')

return fs.outputJsonAsync(file, obj, { spaces: 2 })
.return(obj)
.catch((err) => {
return this._logWriteErr(file, err)
})
}

debug('writing javascript file')

return fs.writeFileAsync(file, jsCode(obj))
.return(obj)
.catch((err) => {
return this._logWriteErr(file, err)
Expand Down Expand Up @@ -110,7 +135,13 @@ module.exports = {
return null
})
},

/**
* Ensures the project at this root has a config file
* that is readable and writable by the node process
* @param {string} projectRoot root of the project
* @param {object} options
* @returns
*/
exists (projectRoot, options = {}) {
const file = this.pathToConfigFile(projectRoot, options)

Expand All @@ -121,7 +152,7 @@ module.exports = {
// directory is writable
return fs.accessAsync(projectRoot, fs.W_OK)
}).catch({ code: 'ENOENT' }, () => {
// cypress.json does not exist, we missing project
// cypress.json does not exist, completely new project
log('cannot find file %s', file)

return this._err('CONFIG_FILE_NOT_FOUND', this.configFile(options), projectRoot)
Expand All @@ -144,29 +175,45 @@ module.exports = {

const file = this.pathToConfigFile(projectRoot, options)

return fs.readJsonAsync(file)
.catch({ code: 'ENOENT' }, () => {
return this._write(file, {})
}).then((json = {}) => {
if (this.isComponentTesting(options) && 'component' in json) {
json = { ...json, ...json.component }
return requireAsync(file,
{
projectRoot,
loadErrorCode: 'CONFIG_FILE_ERROR',
})
.catch((err) => {
if (err.type === 'MODULE_NOT_FOUND' || err.code === 'ENOENT') {
debug('file not found', file)

return this._write(file, {})
}

return Promise.reject(err)
})
.then((configObject = {}) => {
if (this.isComponentTesting(options) && 'component' in configObject) {
configObject = { ...configObject, ...configObject.component }
}

if (!this.isComponentTesting(options) && 'e2e' in json) {
json = { ...json, ...json.e2e }
if (!this.isComponentTesting(options) && 'e2e' in configObject) {
configObject = { ...configObject, ...configObject.e2e }
}

const changed = this._applyRewriteRules(json)
debug('resolved configObject', configObject)
const changed = this._applyRewriteRules(configObject)

// if our object is unchanged
// then just return it
if (_.isEqual(json, changed)) {
return json
if (_.isEqual(configObject, changed)) {
return configObject
}

// else write the new reduced obj
return this._write(file, changed)
.then((config) => {
return config
})
}).catch((err) => {
debug('an error occured when reading config', err)
if (errors.isCypressErr(err)) {
throw err
}
Expand Down Expand Up @@ -196,7 +243,7 @@ module.exports = {
return Promise.resolve({})
}

return this.read(projectRoot)
return this.read(projectRoot, options)
.then((settings) => {
_.extend(settings, obj)

Expand All @@ -206,10 +253,6 @@ module.exports = {
})
},

remove (projectRoot, options = {}) {
return fs.unlinkSync(this.pathToConfigFile(projectRoot, options))
},

pathToConfigFile (projectRoot, options = {}) {
const configFile = this.configFile(options)

Expand Down
2 changes: 1 addition & 1 deletion packages/server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"chai": "1.10.0",
"chalk": "2.4.2",
"check-more-types": "2.24.0",
"chokidar": "3.2.2",
"chokidar": "3.5.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to bump chokidar? Last time we did, it introduced some unexpected flake to CI. Unless we need too, I think deps should be bumped in their own PR. If we need 3.5.1, though, that's different.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found that cypress was only watching the first set of changes.
Closing the project and re-opening it was not restarting the watchers properly.
There was a fix done specifically fo that in v3.3 but it broke a lot more than it fixed.
Updating chokidar to 3.5.1 seemed to fix all the problems I had.

https://github.com/paulmillr/chokidar#changelog

"chrome-remote-interface": "0.28.2",
"cli-table3": "0.5.1",
"coffeescript": "1.12.7",
Expand Down
Loading