From a136bb32c9888ccabdb03cfe8923f3d1cf3065ad Mon Sep 17 00:00:00 2001 From: Zachary Williams Date: Wed, 16 Mar 2022 10:17:41 -0500 Subject: [PATCH 1/5] fix: remove --config-file false references and update types --- cli/__snapshots__/cli_spec.js | 5 ++-- cli/__snapshots__/errors_spec.js | 1 + cli/__snapshots__/run_spec.js | 7 ----- cli/lib/cli.js | 10 +------ cli/lib/errors.js | 6 ++++ cli/lib/exec/open.js | 3 +- cli/lib/exec/run.js | 3 +- cli/lib/exec/shared.js | 7 +++++ cli/test/lib/cypress_spec.js | 30 ------------------- cli/test/lib/exec/open_spec.js | 13 +++----- cli/test/lib/exec/run_spec.js | 21 +++---------- cli/types/cypress-npm-api.d.ts | 4 +-- cli/types/cypress.d.ts | 4 +-- cli/types/tests/cypress-npm-api-test.ts | 4 --- cli/types/tests/plugins-config.ts | 2 +- .../configFileUpdater.test.ts | 15 ---------- .../builders/cypress/cypressBuilderOptions.ts | 2 +- .../src/builders/cypress/schema.json | 3 +- packages/config/src/options.ts | 2 +- .../src/data/ProjectLifecycleManager.ts | 8 ----- .../driver/cypress/e2e/commands/request.cy.js | 4 +-- packages/driver/src/cypress/error_messages.ts | 6 +--- packages/errors/src/errors.ts | 2 +- .../src/config-file-formatted.tsx | 16 ---------- packages/server/lib/project_utils.ts | 2 -- packages/server/lib/util/args.js | 5 +--- .../server/test/integration/cypress_spec.js | 21 ------------- packages/server/test/unit/util/args_spec.js | 3 -- .../server/test/unit/util/settings_spec.js | 17 ----------- packages/types/src/modeOptions.ts | 2 +- packages/types/src/server.ts | 2 +- 31 files changed, 43 insertions(+), 187 deletions(-) diff --git a/cli/__snapshots__/cli_spec.js b/cli/__snapshots__/cli_spec.js index a3ecb387346a..e9f539f87e70 100644 --- a/cli/__snapshots__/cli_spec.js +++ b/cli/__snapshots__/cli_spec.js @@ -26,8 +26,7 @@ exports['shows help for open --foo 1'] = ` cypress.config.{ts|js}. -C, --config-file path to script file where configuration values are set. defaults to - "cypress.config.{ts|js}". pass "false" to - disable. + "cypress.config.{ts|js}". -d, --detached [bool] runs Cypress application in detached mode --e2e runs end to end tests -e, --env sets environment variables. separate @@ -72,7 +71,7 @@ exports['shows help for run --foo 1'] = ` --ci-build-id the unique identifier for a run on your CI provider. typically a "BUILD_ID" env var. this value is automatically detected for most CI providers --component runs component tests -c, --config sets configuration values. separate multiple values with a comma. overrides any value in cypress.config.{ts|js}. - -C, --config-file path to script file where configuration values are set. defaults to "cypress.config.{ts|js}". pass "false" to disable. + -C, --config-file path to script file where configuration values are set. defaults to "cypress.config.{ts|js}". --e2e runs end to end tests -e, --env sets environment variables. separate multiple values with a comma. overrides any value in cypress.config.{ts|js} or cypress.env.json --group a named group for recorded runs in the Cypress Dashboard diff --git a/cli/__snapshots__/errors_spec.js b/cli/__snapshots__/errors_spec.js index e085d953df00..12048eb9a2de 100644 --- a/cli/__snapshots__/errors_spec.js +++ b/cli/__snapshots__/errors_spec.js @@ -83,6 +83,7 @@ exports['errors individual has the following errors 1'] = [ "incompatibleTestTypeFlags", "incompatibleTestingTypeAndFlag", "invalidCacheDirectory", + "invalidConfigFile", "invalidCypressEnv", "invalidOS", "invalidRunProjectPath", diff --git a/cli/__snapshots__/run_spec.js b/cli/__snapshots__/run_spec.js index 5889b5b1b55e..1d9b1bb4bf72 100644 --- a/cli/__snapshots__/run_spec.js +++ b/cli/__snapshots__/run_spec.js @@ -21,13 +21,6 @@ exports['exec run .processRunOptions passes --record option 1'] = [ "my record id" ] -exports['exec run .processRunOptions passes --config-file false option 1'] = [ - "--run-project", - null, - "--config-file", - false -] - exports['exec run .processRunOptions defaults to e2e testingType 1'] = [ "--run-project", null diff --git a/cli/lib/cli.js b/cli/lib/cli.js index b4ecf5de5297..8c473558452f 100644 --- a/cli/lib/cli.js +++ b/cli/lib/cli.js @@ -22,10 +22,6 @@ function unknownOption (flag, type = 'option') { } commander.Command.prototype.unknownOption = unknownOption -const coerceFalseOrString = (arg) => { - return arg !== 'false' ? arg : false -} - const coerceFalse = (arg) => { return arg !== 'false' } @@ -106,7 +102,7 @@ const descriptions = { ciBuildId: 'the unique identifier for a run on your CI provider. typically a "BUILD_ID" env var. this value is automatically detected for most CI providers', component: 'runs component tests', config: 'sets configuration values. separate multiple values with a comma. overrides any value in cypress.config.{ts|js}.', - configFile: 'path to script file where configuration values are set. defaults to "cypress.config.{ts|js}". pass "false" to disable.', + configFile: 'path to script file where configuration values are set. defaults to "cypress.config.{ts|js}".', detached: 'runs Cypress application in detached mode', dev: 'runs cypress in development and bypasses binary check', e2e: 'runs end to end tests', @@ -315,10 +311,6 @@ const castCypressOptions = (opts) => { castOpts.port = coerceAnyStringToInt(opts.port) } - if (_.has(opts, 'configFile')) { - castOpts.configFile = coerceFalseOrString(opts.configFile) - } - return castOpts } diff --git a/cli/lib/errors.js b/cli/lib/errors.js index 2530e231cf8b..941aa10fa125 100644 --- a/cli/lib/errors.js +++ b/cli/lib/errors.js @@ -230,6 +230,11 @@ const incompatibleTestingTypeAndFlag = { solution: 'Either set `testingType` or pass a testing type flag, but not both.', } +const invalidConfigFile = { + description: '`--config-file` cannot be false.', + solution: 'Either pass a relative path to a valid Cypress config file or remove this option.', +} + /** * This error happens when CLI detects that the child Test Runner process * was killed with a signal, like SIGBUS @@ -424,5 +429,6 @@ module.exports = { invalidTestingType, incompatibleTestTypeFlags, incompatibleTestingTypeAndFlag, + invalidConfigFile, }, } diff --git a/cli/lib/exec/open.js b/cli/lib/exec/open.js index 4708d276db4d..e8e5ce17b1ea 100644 --- a/cli/lib/exec/open.js +++ b/cli/lib/exec/open.js @@ -2,7 +2,7 @@ const debug = require('debug')('cypress:cli') const util = require('../util') const spawn = require('./spawn') const verify = require('../tasks/verify') -const { processTestingType } = require('./shared') +const { processTestingType, checkConfigFile } = require('./shared') /** * Maps options collected by the CLI @@ -25,6 +25,7 @@ const processOpenOptions = (options = {}) => { } if (options.configFile !== undefined) { + checkConfigFile(options) args.push('--config-file', options.configFile) } diff --git a/cli/lib/exec/run.js b/cli/lib/exec/run.js index b8644b7a2921..405f3218c8ce 100644 --- a/cli/lib/exec/run.js +++ b/cli/lib/exec/run.js @@ -5,7 +5,7 @@ const util = require('../util') const spawn = require('./spawn') const verify = require('../tasks/verify') const { exitWithError, errors } = require('../errors') -const { processTestingType, throwInvalidOptionError } = require('./shared') +const { processTestingType, throwInvalidOptionError, checkConfigFile } = require('./shared') /** * Typically a user passes a string path to the project. @@ -58,6 +58,7 @@ const processRunOptions = (options = {}) => { } if (options.configFile !== undefined) { + checkConfigFile(options) args.push('--config-file', options.configFile) } diff --git a/cli/lib/exec/shared.js b/cli/lib/exec/shared.js index b3723d195266..21eb21e25ae3 100644 --- a/cli/lib/exec/shared.js +++ b/cli/lib/exec/shared.js @@ -47,7 +47,14 @@ const processTestingType = (options) => { return [] } +const checkConfigFile = (options) => { + if (options.configFile === 'false') { + throwInvalidOptionError(errors.invalidConfigFile) + } +} + module.exports = { throwInvalidOptionError, processTestingType, + checkConfigFile, } diff --git a/cli/test/lib/cypress_spec.js b/cli/test/lib/cypress_spec.js index 829c42f3e492..881c524f3a29 100644 --- a/cli/test/lib/cypress_spec.js +++ b/cli/test/lib/cypress_spec.js @@ -152,18 +152,6 @@ describe('cypress', function () { return cypress.run().then(snapshot) }) - it('passes configFile: false', () => { - const opts = { - configFile: false, - } - - return cypress.run(opts) - .then(getStartArgs) - .then((args) => { - expect(args).to.deep.eq(opts) - }) - }) - it('rejects if project is an empty string', () => { return expect(cypress.run({ project: '' })).to.be.rejected }) @@ -223,15 +211,6 @@ describe('cypress', function () { }) }) - it('coerces --config-file false to boolean', async () => { - const args = 'cypress run --config-file false'.split(' ') - const options = await cypress.cli.parseRunArguments(args) - - expect(options).to.deep.equal({ - configFile: false, - }) - }) - it('coerces --config-file cypress.config.js to string', async () => { const args = 'cypress run --config-file cypress.config.js'.split(' ') const options = await cypress.cli.parseRunArguments(args) @@ -241,15 +220,6 @@ describe('cypress', function () { }) }) - it('parses config file false', async () => { - const args = 'cypress run --config-file false'.split(' ') - const options = await cypress.cli.parseRunArguments(args) - - expect(options).to.deep.equal({ - configFile: false, - }) - }) - it('parses config', async () => { const args = 'cypress run --config baseUrl=localhost,video=true'.split(' ') const options = await cypress.cli.parseRunArguments(args) diff --git a/cli/test/lib/exec/open_spec.js b/cli/test/lib/exec/open_spec.js index fcb975f8dd9c..9e36bfcce90d 100644 --- a/cli/test/lib/exec/open_spec.js +++ b/cli/test/lib/exec/open_spec.js @@ -56,15 +56,6 @@ describe('exec open', function () { }) }) - it('spawns with --config-file false', function () { - return open.start({ configFile: false }) - .then(() => { - expect(spawn.start).to.be.calledWith( - ['--config-file', false], - ) - }) - }) - it('spawns with --config-file set', function () { return open.start({ configFile: 'special-cypress.config.js' }) .then(() => { @@ -144,5 +135,9 @@ describe('exec open', function () { it('throws if --testing-type is invalid', () => { expect(() => open.start({ testingType: 'randomTestingType' })).to.throw() }) + + it('throws if --config-file is false', () => { + expect(() => open.start({ configFile: 'false' })).to.throw() + }) }) }) diff --git a/cli/test/lib/exec/run_spec.js b/cli/test/lib/exec/run_spec.js index 340330ec4d3e..a79d1a6e0067 100644 --- a/cli/test/lib/exec/run_spec.js +++ b/cli/test/lib/exec/run_spec.js @@ -53,14 +53,6 @@ describe('exec run', function () { snapshot(args) }) - it('passes --config-file false option', () => { - const args = run.processRunOptions({ - configFile: false, - }) - - snapshot(args) - }) - it('does not allow setting paradoxical --headed and --headless flags', () => { os.platform.returns('linux') process.exit.returns() @@ -112,6 +104,10 @@ describe('exec run', function () { it('throws if both testingType and component are set', () => { expect(() => run.processRunOptions({ testingType: 'component', component: true })).to.throw() }) + + it('throws if --config-file is false', () => { + expect(() => run.processRunOptions({ configFile: 'false' })).to.throw() + }) }) context('.start', function () { @@ -148,15 +144,6 @@ describe('exec run', function () { }) }) - it('spawns with --config-file false', function () { - return run.start({ configFile: false }) - .then(() => { - expect(spawn.start).to.be.calledWith( - ['--run-project', process.cwd(), '--config-file', false], - ) - }) - }) - it('spawns with --config-file set', function () { return run.start({ configFile: 'special-cypress.config.js' }) .then(() => { diff --git a/cli/types/cypress-npm-api.d.ts b/cli/types/cypress-npm-api.d.ts index dde4c171a03b..f351ffec8a7f 100644 --- a/cli/types/cypress-npm-api.d.ts +++ b/cli/types/cypress-npm-api.d.ts @@ -146,11 +146,9 @@ declare namespace CypressCommandLine { /** * Path to the config file to be used. * - * If `false` is passed, no config file will be used. - * * @default "cypress.config.{ts|js}" */ - configFile: string | false + configFile: string /** * Specify environment variables. * TODO: isn't this duplicate of config.env?! diff --git a/cli/types/cypress.d.ts b/cli/types/cypress.d.ts index a0813ee54b80..b48d7dd97e5e 100644 --- a/cli/types/cypress.d.ts +++ b/cli/types/cypress.d.ts @@ -2887,9 +2887,9 @@ declare namespace Cypress { */ interface RuntimeConfigOptions extends Partial { /** - * Absolute path to the config file (default: /cypress.config.{ts|js}) or false + * Absolute path to the config file (default: /cypress.config.{ts|js}) */ - configFile: string | false + configFile: string /** * CPU architecture, from Node `os.arch()` * diff --git a/cli/types/tests/cypress-npm-api-test.ts b/cli/types/tests/cypress-npm-api-test.ts index d7979e17a39a..dd2bbd9f29f8 100644 --- a/cli/types/tests/cypress-npm-api-test.ts +++ b/cli/types/tests/cypress-npm-api-test.ts @@ -22,10 +22,6 @@ cypress.run().then(results => { cypress.open() // $ExpectType Promise cypress.run() // $ExpectType Promise -cypress.open({ - configFile: false -}) - cypress.run({ configFile: "abc123" }) diff --git a/cli/types/tests/plugins-config.ts b/cli/types/tests/plugins-config.ts index 1a86a0a3d110..2292a8e42fa4 100644 --- a/cli/types/tests/plugins-config.ts +++ b/cli/types/tests/plugins-config.ts @@ -6,7 +6,7 @@ const pluginConfig: Cypress.PluginConfig = (on, config) => {} // allows synchronous returns const pluginConfig2: Cypress.PluginConfig = (on, config) => { config // $ExpectType PluginConfigOptions - config.configFile // $ExpectType string | false + config.configFile // $ExpectType string config.fixturesFolder // $ExpectType string | false config.screenshotsFolder // $ExpectType string | false config.videoCompression // $ExpectType number | false diff --git a/npm/create-cypress-tests/src/component-testing/config-file-updater/configFileUpdater.test.ts b/npm/create-cypress-tests/src/component-testing/config-file-updater/configFileUpdater.test.ts index 0168ad9d95ef..c1a568ba0a06 100644 --- a/npm/create-cypress-tests/src/component-testing/config-file-updater/configFileUpdater.test.ts +++ b/npm/create-cypress-tests/src/component-testing/config-file-updater/configFileUpdater.test.ts @@ -44,21 +44,6 @@ const stripIndent = (strings: any, ...args: any) => { } describe('lib/util/config-file-updater', () => { - context('with configFile: false', () => { - beforeEach(function () { - this.projectRoot = path.join(projectRoot, '_test-output/path/to/project/') - }) - - it('.insertValuesInConfigFile does not create a file', function () { - return insertValuesInConfigFile(this.projectRoot, {}) - .then(() => { - throw Error('file shuold not have been created here') - }).catch((err) => { - expect(err.code).to.equal('ENOENT') - }) - }) - }) - context('with js files', () => { describe('#insertValueInJSString', () => { describe('es6 vs es5', () => { diff --git a/npm/cypress-schematic/src/builders/cypress/cypressBuilderOptions.ts b/npm/cypress-schematic/src/builders/cypress/cypressBuilderOptions.ts index bbbc51c88a97..9194fa1ba219 100644 --- a/npm/cypress-schematic/src/builders/cypress/cypressBuilderOptions.ts +++ b/npm/cypress-schematic/src/builders/cypress/cypressBuilderOptions.ts @@ -2,7 +2,7 @@ import { JsonObject } from '@angular-devkit/core' export interface CypressBuilderOptions extends JsonObject { baseUrl: string - configFile: string | false + configFile: string browser: 'electron' | 'chrome' | 'chromium' | 'canary' | 'firefox' | 'edge' | string devServerTarget: string env: Record diff --git a/npm/cypress-schematic/src/builders/cypress/schema.json b/npm/cypress-schematic/src/builders/cypress/schema.json index ea563eee0712..a09e8aee37c3 100644 --- a/npm/cypress-schematic/src/builders/cypress/schema.json +++ b/npm/cypress-schematic/src/builders/cypress/schema.json @@ -9,8 +9,7 @@ }, "configFile": { "type": [ - "string", - "boolean" + "string" ], "description": "Specify a path to a JSON file where configuration values are set or pass false to disable the use of a configuration file entirely." }, diff --git a/packages/config/src/options.ts b/packages/config/src/options.ts index 62888d6f75d8..7af36ffe9c20 100644 --- a/packages/config/src/options.ts +++ b/packages/config/src/options.ts @@ -420,7 +420,7 @@ const runtimeOptions: Array = [ }, { name: 'configFile', defaultValue: 'cypress.config.js', - validation: validate.isStringOrFalse, + validation: validate.isString, // not truly internal, but can only be set via cli, // so we don't consider it a "public" option isInternal: true, diff --git a/packages/data-context/src/data/ProjectLifecycleManager.ts b/packages/data-context/src/data/ProjectLifecycleManager.ts index feaff864b21d..35dbd63fa234 100644 --- a/packages/data-context/src/data/ProjectLifecycleManager.ts +++ b/packages/data-context/src/data/ProjectLifecycleManager.ts @@ -536,10 +536,6 @@ export class ProjectLifecycleManager { // } async getConfigFileContents () { - if (this.ctx.modeOptions.configFile === false) { - return {} - } - if (this._configResult.state === 'loaded') { return this._configResult.value.initialConfig } @@ -1126,10 +1122,6 @@ export class ProjectLifecycleManager { hasCypressEnvFile: fs.existsSync(this._pathToFile('cypress.env.json')), } - if (configFile === false) { - return metaState - } - try { // Find the suggested framework, starting with meta-frameworks first const packageJson = this.ctx.fs.readJsonSync(this._pathToFile('package.json')) diff --git a/packages/driver/cypress/e2e/commands/request.cy.js b/packages/driver/cypress/e2e/commands/request.cy.js index 97d759ca88db..f5f5e744a34a 100644 --- a/packages/driver/cypress/e2e/commands/request.cy.js +++ b/packages/driver/cypress/e2e/commands/request.cy.js @@ -853,7 +853,7 @@ describe('src/cy/commands/request', () => { cy.request('/foo/bar') }) - it('throws when url is not FQDN, notes that configFile is disabled', { + it('throws when url is not FQDN', { baseUrl: null, configFile: false, }, function (done) { @@ -865,7 +865,7 @@ describe('src/cy/commands/request', () => { assertLogLength(this.logs, 1) expect(lastLog.get('error')).to.eq(err) expect(lastLog.get('state')).to.eq('failed') - expect(err.message).to.eq('`cy.request()` must be provided a fully qualified `url` - one that begins with `http`. By default `cy.request()` will use either the current window\'s origin or the `baseUrl` in `cypress.config.{ts|js}` (currently disabled by --config-file=false). Neither of those values were present.') + expect(err.message).to.eq('`cy.request()` must be provided a fully qualified `url` - one that begins with `http`. By default `cy.request()` will use either the current window\'s origin or the `baseUrl` in `cypress.config.{ts|js}`. Neither of those values were present.') done() }) diff --git a/packages/driver/src/cypress/error_messages.ts b/packages/driver/src/cypress/error_messages.ts index 02d1e26d0c72..66c46ce31c10 100644 --- a/packages/driver/src/cypress/error_messages.ts +++ b/packages/driver/src/cypress/error_messages.ts @@ -25,11 +25,7 @@ function removeLeadingSlash (str: string) { return str.substring(li + 1) } -const formatConfigFile = (projectRoot: string, configFile: string | false) => { - if (configFile === false) { - return '`cypress.config.{ts|js}` (currently disabled by --config-file=false)' - } - +const formatConfigFile = (projectRoot: string, configFile: string) => { configFile = removeLeadingSlash(configFile.replace(projectRoot, '')) return `\`${format(configFile)}\`` diff --git a/packages/errors/src/errors.ts b/packages/errors/src/errors.ts index 66aa81acc510..792781f42474 100644 --- a/packages/errors/src/errors.ts +++ b/packages/errors/src/errors.ts @@ -497,7 +497,7 @@ export const AllCypressErrors = { https://on.cypress.io/dashboard` }, // TODO: make this relative path, not absolute - NO_PROJECT_ID: (configFilePath: string | false) => { + NO_PROJECT_ID: (configFilePath: string) => { return errTemplate`Can't find ${fmt.highlight(`projectId`)} in the config file: ${fmt.path(configFilePath || '')}` }, NO_PROJECT_FOUND_AT_PROJECT_ROOT: (projectRoot: string) => { diff --git a/packages/runner-shared/src/config-file-formatted.tsx b/packages/runner-shared/src/config-file-formatted.tsx index 4fc087d8e796..3b7ead96bb6d 100644 --- a/packages/runner-shared/src/config-file-formatted.tsx +++ b/packages/runner-shared/src/config-file-formatted.tsx @@ -1,22 +1,6 @@ import React from 'react' const configFileFormatted = (configFile) => { - if (configFile === false) { - return ( - <> - -cypress.config. - {`{ts | js}`} - - {' '} -file (currently disabled by - {' '} - --config-file false -) - - ) - } - if (['cypress.config.ts', 'cypress.config.js'].includes(configFile)) { return ( <> diff --git a/packages/server/lib/project_utils.ts b/packages/server/lib/project_utils.ts index 3e7b9d245945..bb47da562e6b 100644 --- a/packages/server/lib/project_utils.ts +++ b/packages/server/lib/project_utils.ts @@ -38,10 +38,8 @@ export const getSpecUrl = ({ export const checkSupportFile = async ({ supportFile, - configFile, }: { supportFile?: string | boolean - configFile?: string | false }) => { if (supportFile && typeof supportFile === 'string') { const found = await fs.pathExists(supportFile) diff --git a/packages/server/lib/util/args.js b/packages/server/lib/util/args.js index fc3b6992e423..a73329e38936 100644 --- a/packages/server/lib/util/args.js +++ b/packages/server/lib/util/args.js @@ -93,10 +93,7 @@ const normalizeBackslashes = (options) => { if (typeof options[property] === 'string') { options[property] = normalizeBackslash(options[property]) } else { - // configFile is a special case that can be set to false - if (property !== 'configFile') { - delete options[property] - } + delete options[property] } }) diff --git a/packages/server/test/integration/cypress_spec.js b/packages/server/test/integration/cypress_spec.js index 764d1ee10839..c0b2f2abbbc8 100644 --- a/packages/server/test/integration/cypress_spec.js +++ b/packages/server/test/integration/cypress_spec.js @@ -1170,27 +1170,6 @@ describe('lib/cypress', () => { }) describe('--config-file', () => { - // NOTE: --config-file=false is not supported - it.skip('false does not require cypress.config.js to run', function () { - return fs.statAsync(path.join(this.pristinePath, 'cypress.config.js')) - .then(() => { - throw new Error('cypress.config.js should not exist') - }).catch({ code: 'ENOENT' }, () => { - return cypress.start([ - `--run-project=${this.pristinePath}`, - '--no-run-mode', - '--config-file', - 'false', - ]).then(() => { - // uses default specPattern which is cypress/integration/**/* - // exits with 1 since there are not specs for this pristine project. - this.expectExitWithErr('NO_SPECS_FOUND', 'We searched for specs matching this glob pattern:') - this.expectExitWithErr('NO_SPECS_FOUND', 'Can\'t run because no spec files were found') - this.expectExitWith(1) - }) - }) - }) - // TODO: fix it.skip(`with a custom config file fails when it doesn't exist`, function () { this.filename = 'abcdefgh.test.js' diff --git a/packages/server/test/unit/util/args_spec.js b/packages/server/test/unit/util/args_spec.js index 3c40da317083..97792bc87da9 100644 --- a/packages/server/test/unit/util/args_spec.js +++ b/packages/server/test/unit/util/args_spec.js @@ -68,8 +68,6 @@ describe('lib/util/args', () => { // string properties project: true, appPath: '/foo/bar', - // this option can be string or false - configFile: false, // unknown properties will be preserved somethingElse: 42, } @@ -77,7 +75,6 @@ describe('lib/util/args', () => { expect(output).to.deep.equal({ appPath: '/foo/bar', - configFile: false, somethingElse: 42, }) }) diff --git a/packages/server/test/unit/util/settings_spec.js b/packages/server/test/unit/util/settings_spec.js index 592ed174453a..edfdfac9b4ad 100644 --- a/packages/server/test/unit/util/settings_spec.js +++ b/packages/server/test/unit/util/settings_spec.js @@ -77,23 +77,6 @@ describe.skip('lib/util/settings', () => { }) }) - context('with configFile: false', () => { - beforeEach(function () { - this.projectRoot = path.join(projectRoot, '_test-output/path/to/project/') - - this.options = { - configFile: false, - } - }) - - it('.read returns empty object', function () { - return settings.read(this.projectRoot, { configFile: false }) - .then((settings) => { - expect(settings).to.deep.equal({}) - }) - }) - }) - context('with js files', () => { it('.read returns from configFile when its a JavaScript file', function () { this.projectRoot = path.join(projectRoot, '_test-output/path/to/project/') diff --git a/packages/types/src/modeOptions.ts b/packages/types/src/modeOptions.ts index 27bc66d8a6f5..06c38f727f1e 100644 --- a/packages/types/src/modeOptions.ts +++ b/packages/types/src/modeOptions.ts @@ -2,7 +2,7 @@ export interface CommonModeOptions { invokedFromCli: boolean userNodePath?: string userNodeVersion?: string - configFile?: false | string | null + configFile?: string | null } export interface RunModeOptions extends CommonModeOptions { diff --git a/packages/types/src/server.ts b/packages/types/src/server.ts index 707b25a0b220..35fc59a94542 100644 --- a/packages/types/src/server.ts +++ b/packages/types/src/server.ts @@ -60,7 +60,7 @@ export interface OpenProjectLaunchOptions { */ skipPluginInitializeForTesting?: boolean - configFile?: string | false + configFile?: string // spec pattern to use when launching from CLI spec?: string From d3c15eb34e6c629ecf4a1b497bdb3ba8ae07b41c Mon Sep 17 00:00:00 2001 From: Zachary Williams Date: Wed, 16 Mar 2022 11:22:58 -0500 Subject: [PATCH 2/5] cleanup tests and types, catch errors when calling open --- cli/lib/exec/open.js | 13 +++++++++++- cli/lib/exec/shared.js | 7 ++++++- cli/test/lib/cypress_spec.js | 12 ----------- .../driver/cypress/e2e/commands/request.cy.js | 20 ------------------- packages/server/lib/config.ts | 4 ---- packages/server/lib/project-base.ts | 2 +- 6 files changed, 19 insertions(+), 39 deletions(-) diff --git a/cli/lib/exec/open.js b/cli/lib/exec/open.js index e8e5ce17b1ea..7c9e8b194d0e 100644 --- a/cli/lib/exec/open.js +++ b/cli/lib/exec/open.js @@ -3,6 +3,7 @@ const util = require('../util') const spawn = require('./spawn') const verify = require('../tasks/verify') const { processTestingType, checkConfigFile } = require('./shared') +const { exitWithError } = require('../errors') /** * Maps options collected by the CLI @@ -68,7 +69,17 @@ const processOpenOptions = (options = {}) => { module.exports = { processOpenOptions, start (options = {}) { - const args = processOpenOptions(options) + let args + + try { + args = processOpenOptions(options) + } catch (err) { + if (err.details) { + return exitWithError(err.details)() + } + + throw err + } function open () { return spawn.start(args, { diff --git a/cli/lib/exec/shared.js b/cli/lib/exec/shared.js index 21eb21e25ae3..3be0243dd9db 100644 --- a/cli/lib/exec/shared.js +++ b/cli/lib/exec/shared.js @@ -47,8 +47,13 @@ const processTestingType = (options) => { return [] } +/** + * Throws an error if configFile is string 'false' or boolean false + * @param {*} options + */ const checkConfigFile = (options) => { - if (options.configFile === 'false') { + // CLI will parse as string, module API can pass in boolean + if (options.configFile === 'false' || options.configFile === false) { throwInvalidOptionError(errors.invalidConfigFile) } } diff --git a/cli/test/lib/cypress_spec.js b/cli/test/lib/cypress_spec.js index 881c524f3a29..44579d7ac926 100644 --- a/cli/test/lib/cypress_spec.js +++ b/cli/test/lib/cypress_spec.js @@ -53,18 +53,6 @@ describe('cypress', function () { expect(args).to.deep.eq({ config: JSON.stringify(config) }) }) }) - - it('passes configFile: false', () => { - const opts = { - configFile: false, - } - - return cypress.open(opts) - .then(getStartArgs) - .then((args) => { - expect(args).to.deep.eq(opts) - }) - }) }) context('.run fails to write results file', function () { diff --git a/packages/driver/cypress/e2e/commands/request.cy.js b/packages/driver/cypress/e2e/commands/request.cy.js index f5f5e744a34a..562d414aeabf 100644 --- a/packages/driver/cypress/e2e/commands/request.cy.js +++ b/packages/driver/cypress/e2e/commands/request.cy.js @@ -853,26 +853,6 @@ describe('src/cy/commands/request', () => { cy.request('/foo/bar') }) - it('throws when url is not FQDN', { - baseUrl: null, - configFile: false, - }, function (done) { - cy.stub(cy, 'getRemoteLocation').withArgs('origin').returns('') - - cy.on('fail', (err) => { - const { lastLog } = this - - assertLogLength(this.logs, 1) - expect(lastLog.get('error')).to.eq(err) - expect(lastLog.get('state')).to.eq('failed') - expect(err.message).to.eq('`cy.request()` must be provided a fully qualified `url` - one that begins with `http`. By default `cy.request()` will use either the current window\'s origin or the `baseUrl` in `cypress.config.{ts|js}`. Neither of those values were present.') - - done() - }) - - cy.request('/foo/bar') - }) - it('throws when url is not FQDN, notes that configFile is non-default', { baseUrl: null, configFile: 'foo.json', diff --git a/packages/server/lib/config.ts b/packages/server/lib/config.ts index 213957fa7c0c..bc68b2774946 100644 --- a/packages/server/lib/config.ts +++ b/packages/server/lib/config.ts @@ -247,10 +247,6 @@ export function updateWithPluginValues (cfg, overrides) { configUtils.validate(overrides, (validationResult: ConfigValidationFailureInfo | string) => { let configFile = getCtx().lifecycleManager.configFile - if (configFile === false) { - configFile = '--config file set to "false" via CLI--' - } - if (_.isString(validationResult)) { return errors.throwErr('CONFIG_VALIDATION_MSG_ERROR', 'configFile', configFile, validationResult) } diff --git a/packages/server/lib/project-base.ts b/packages/server/lib/project-base.ts index 03549ad1bf8f..e3dd8071b2f1 100644 --- a/packages/server/lib/project-base.ts +++ b/packages/server/lib/project-base.ts @@ -234,7 +234,7 @@ export class ProjectBase extends EE { await this.saveState(stateToSave) await Promise.all([ - checkSupportFile({ configFile: cfg.configFile, supportFile: cfg.supportFile }), + checkSupportFile({ supportFile: cfg.supportFile }), ]) if (cfg.isTextTerminal) { From e59c687500afc6acda369596ecce78bc663117aa Mon Sep 17 00:00:00 2001 From: Zachary Williams Date: Wed, 16 Mar 2022 12:19:29 -0500 Subject: [PATCH 3/5] fix test --- cli/test/lib/exec/open_spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/test/lib/exec/open_spec.js b/cli/test/lib/exec/open_spec.js index 9e36bfcce90d..d351771ce06c 100644 --- a/cli/test/lib/exec/open_spec.js +++ b/cli/test/lib/exec/open_spec.js @@ -133,11 +133,11 @@ describe('exec open', function () { }) it('throws if --testing-type is invalid', () => { - expect(() => open.start({ testingType: 'randomTestingType' })).to.throw() + expect(() => open.processOpenOptions({ testingType: 'randomTestingType' })).to.throw() }) it('throws if --config-file is false', () => { - expect(() => open.start({ configFile: 'false' })).to.throw() + expect(() => open.processOpenOptions({ configFile: 'false' })).to.throw() }) }) }) From cd40998ed0648f89da31ba9cf652684efde28713 Mon Sep 17 00:00:00 2001 From: Zachary Williams Date: Wed, 16 Mar 2022 14:24:34 -0500 Subject: [PATCH 4/5] remove string check --- packages/data-context/src/data/ProjectLifecycleManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/data-context/src/data/ProjectLifecycleManager.ts b/packages/data-context/src/data/ProjectLifecycleManager.ts index 35dbd63fa234..177733927f3e 100644 --- a/packages/data-context/src/data/ProjectLifecycleManager.ts +++ b/packages/data-context/src/data/ProjectLifecycleManager.ts @@ -1140,7 +1140,7 @@ export class ProjectLifecycleManager { // No need to handle } - if (typeof configFile === 'string') { + if (configFile) { metaState.hasSpecifiedConfigViaCLI = this._pathToFile(configFile) if (configFile.endsWith('.json')) { metaState.needsCypressJsonMigration = true From 84bfdcc97c31c9acd6077486f9976bb6b18ddf73 Mon Sep 17 00:00:00 2001 From: Zachary Williams Date: Fri, 18 Mar 2022 10:14:19 -0500 Subject: [PATCH 5/5] code quality feedback --- cli/lib/exec/open.js | 32 +++++++++---------- cli/lib/exec/run.js | 16 ++++------ packages/driver/src/cypress/error_messages.ts | 2 +- packages/server/lib/project-base.ts | 4 +-- packages/server/lib/project_utils.ts | 6 +--- 5 files changed, 25 insertions(+), 35 deletions(-) diff --git a/cli/lib/exec/open.js b/cli/lib/exec/open.js index 7c9e8b194d0e..8765670b3685 100644 --- a/cli/lib/exec/open.js +++ b/cli/lib/exec/open.js @@ -69,24 +69,22 @@ const processOpenOptions = (options = {}) => { module.exports = { processOpenOptions, start (options = {}) { - let args - - try { - args = processOpenOptions(options) - } catch (err) { - if (err.details) { - return exitWithError(err.details)() - } - - throw err - } - function open () { - return spawn.start(args, { - dev: options.dev, - detached: Boolean(options.detached), - stdio: 'inherit', - }) + try { + const args = processOpenOptions(options) + + return spawn.start(args, { + dev: options.dev, + detached: Boolean(options.detached), + stdio: 'inherit', + }) + } catch (err) { + if (err.details) { + return exitWithError(err.details)() + } + + throw err + } } if (options.dev) { diff --git a/cli/lib/exec/run.js b/cli/lib/exec/run.js index 405f3218c8ce..e070205901b1 100644 --- a/cli/lib/exec/run.js +++ b/cli/lib/exec/run.js @@ -165,10 +165,14 @@ module.exports = { }) function run () { - let args - try { - args = processRunOptions(options) + const args = processRunOptions(options) + + debug('run to spawn.start args %j', args) + + return spawn.start(args, { + dev: options.dev, + }) } catch (err) { if (err.details) { return exitWithError(err.details)() @@ -176,12 +180,6 @@ module.exports = { throw err } - - debug('run to spawn.start args %j', args) - - return spawn.start(args, { - dev: options.dev, - }) } if (options.dev) { diff --git a/packages/driver/src/cypress/error_messages.ts b/packages/driver/src/cypress/error_messages.ts index 66c46ce31c10..d25f7e58c1ac 100644 --- a/packages/driver/src/cypress/error_messages.ts +++ b/packages/driver/src/cypress/error_messages.ts @@ -25,7 +25,7 @@ function removeLeadingSlash (str: string) { return str.substring(li + 1) } -const formatConfigFile = (projectRoot: string, configFile: string) => { +const formatConfigFile = (projectRoot: Cypress.Config['projectRoot'], configFile: Cypress.Config['configFile']) => { configFile = removeLeadingSlash(configFile.replace(projectRoot, '')) return `\`${format(configFile)}\`` diff --git a/packages/server/lib/project-base.ts b/packages/server/lib/project-base.ts index e3dd8071b2f1..6ea618cc90e9 100644 --- a/packages/server/lib/project-base.ts +++ b/packages/server/lib/project-base.ts @@ -233,9 +233,7 @@ export class ProjectBase extends EE { await this.saveState(stateToSave) - await Promise.all([ - checkSupportFile({ supportFile: cfg.supportFile }), - ]) + await checkSupportFile(cfg.supportFile) if (cfg.isTextTerminal) { return diff --git a/packages/server/lib/project_utils.ts b/packages/server/lib/project_utils.ts index bb47da562e6b..6d84f96a6767 100644 --- a/packages/server/lib/project_utils.ts +++ b/packages/server/lib/project_utils.ts @@ -36,11 +36,7 @@ export const getSpecUrl = ({ return specUrl } -export const checkSupportFile = async ({ - supportFile, -}: { - supportFile?: string | boolean -}) => { +export const checkSupportFile = async (supportFile: Cypress.Config['supportFile']) => { if (supportFile && typeof supportFile === 'string') { const found = await fs.pathExists(supportFile)