feat: support ct/e2e specific overrides in cypress.json #15526

lmiller1990 · 2021-03-16T15:51:48Z

Closes https://cypress-io.atlassian.net/browse/CT-305?focusedCommentId=13398

Note: Some other changes here to support create-cypress-tests wizard. Ignore those for the purpose of this review.

User facing changelog

1. Add support for runner specific overrides in `cypress.json`. Like this:

// cypress.json
{
    // defaults
    video: true,

    // added this e2e key
    e2e: {
	  video: false // override!
	},

    // added component
    component: { 
      viewportWidth: 500 // video is true - inherited.
    }
}

2. Pass `testingType` to `config` in plugin function

config now includes a testingType property. It is either e2e or component. The user can have conditional plugins using this flag.

module.exports = (on, config) => {
  if (config.testingType === 'component') {
    // start webpack-dev-server, etc.
  }

  if (config.testingType === 'e2e') {
    // e2e things
  }

  // You still need to return a config file.
  // e2e or CT specific config can be done in `cypress.json` using `e2e` or `component` keys.
  return config
}

Additional details

It's very useful now that we have two runners, CT and E2E.

How has the user experience changed?

You can have runner specific config, and runner specific plugins.

PR Tasks

Have tests been added/updated?
Has a PR for user-facing changes been opened in [cypress-documentation] Here it is: docs: document mode argument to plugins cypress-documentation#3693
Have API changes been updated in the type definitions? Yes.
ZenHub something something.
Have new configuration options been added to the cypress.schema.json? Yes.

cypress-bot · 2021-03-16T15:51:50Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

lmiller1990 · 2021-03-16T15:53:42Z

packages/server/lib/util/settings.js

@@ -93,6 +93,10 @@ module.exports = {
    }, _.cloneDeep(obj))
  },

+  isComponentTesting (options = {}) {


Need to simplify this to a single flag at some point.

github-actions · 2021-03-16T16:02:29Z

Internal Jira issue: TR-716

cypress · 2021-03-16T16:16:33Z

Test summary

3982 • 0 • 52 • 1 • 0

Run details


Project	cypress
Status	Passed
Commit	`c813e36`
Started	Mar 22, 2021 1:07 AM
Ended	Mar 22, 2021 1:18 AM
Duration	10:50 💡
OS	Linux Debian - 10.5
Browser	Firefox 77

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

brian-mann · 2021-03-16T16:49:14Z

Worth noting the latest working session notes are here: https://www.notion.so/cypressdx/Config-Changes-for-7-0-ce5f6221b6d64b55b5cc633460463b13

…ement

chrisbreiding

I think we should add mode to the config object argument instead of adding a 3rd argument. It would be consistent with what we've done with other properties that aren't strictly "config".

I also wonder if we should rethink calling it mode as well. We often refer to "run mode" and "interactive mode" depending on whether you run cypress with cypress run or cypress open, so it could be confusing to have another type of "mode" that refers to a different concept.

packages/server/lib/util/settings.js

JessicaSachs · 2021-03-16T18:04:20Z

It would be consistent with what we've done with other properties that aren't strictly "config".

I also thought it was weird the @brian-mann and @flotwig wanted it as a third argument instead of on config. Were there reasons for that?

I also wonder if we should rethink calling it mode as well. We often refer to "run mode" and "interactive mode" depending on whether you run cypress with cypress run or cypress open, so it could be confusing to have another type of "mode" that refers to a different concept.

@elevatebart had the same feedback regarding naming. I'm okay with changing it.

The comments in cypress.schema.json regarding e2e vs component say Any component runner specific overrides and Any e2e runner specific overrides. That leads me to want to name it as "runnerType", "testRunnerMode".

Thoughts?

brian-mann · 2021-03-16T18:26:54Z

My points were more broadly related to the conventions of "what exactly is configuration" - and I mentioned things like... config is really Cypress.config() which is really mutable properties that can change runtime behavior.

Whereas things like isTextTerminal, browser, testingType, etc are not config values - they are static immutable descriptors.

Yes I know we currently put things like isTextTerminal in config, but we do properly slice those out and put them as separate objects on Cypress. My point is basically to come up with a convention and agree on how to handle the different sets of values in arguments and also on the global Cypress object. It's currently inconsistent and I would rather think about the solution here to get the pattern "right" before adding more.

brian-mann · 2021-03-16T18:28:09Z

For instance - knowing whether or not you are in open mode vs interactive mode is currently checked for by an undocumented isTextTerminal config value (which isn't really config, since its immutable) and inadequately describes the runtime environment mode.

With adding componentTesting, we now have a matrix of runtime modes.

JessicaSachs · 2021-03-16T18:38:45Z

My points were more broadly related to the conventions of "what exactly is configuration" - and I mentioned things like... config is really Cypress.config() which is really mutable properties that can change runtime behavior.

Whereas things like isTextTerminal, browser, testingType, etc are not config values - they are static immutable descriptors.

Yes I know we currently put things like isTextTerminal in config, but we do properly slice those out and put them as separate objects on Cypress. My point is basically to come up with a convention and agree on how to handle the different sets of values in arguments and also on the global Cypress object. It's currently inconsistent and I would rather think about the solution here to get the pattern "right" before adding more.

Then, perhaps do we need an "immutable stuff" variable instead of just a third "mode"?

chrisbreiding · 2021-03-16T18:39:22Z

My points were more broadly related to the conventions of "what exactly is configuration" - and I mentioned things like... config is really Cypress.config() which is really mutable properties that can change runtime behavior.

I generally agree and my initial implementation of providing the projectRoot and configFile to the plugins file utilized a 3rd argument for that reason. You can see the commit here. Unfortunately, I can't find the feedback/discussion that convinced me to add the properties to config instead. I think there was some dislike of calling it env and no better alternative, so we decided it was much more straightforward to simply put the properties on config.

I think we could still utilize a 3rd argument as long as it's an object. But I think that's outside the scope of this work. I advocate we add it to config for now. We can add a 3rd argument in the future that holds the extra properties, then we deprecate their usage on config.

chrisbreiding · 2021-03-16T18:48:21Z

In regards to the name, just tossing out some ideas:

testType
testingType
specType

brian-mann · 2021-03-16T18:49:47Z

i agree for now too.

i think maybe we could just put all of those immutable environment kinds of things on a standard key on the config object...

this is where things like browser, testingType, isTextTerminal (renamed to something better), etc could go...

Maybe config.runtime or something like that...

We would then slice out runtime from the actual config sent to Cypress.config() and would then mount those under new API's on the Cypress object.

lmiller1990 · 2021-03-17T14:42:15Z

For now we will move testingType to config, second arg to the plugins function.
Make testingType available on Cypress
It will not be available on Cypress.config, we will slice it out. This is similar to browser, platform, arch etc.

See the API summary here: https://www.notion.so/cypressdx/Config-Changes-for-7-0-ce5f6221b6d64b55b5cc633460463b13

dmtrKovalenko · 2021-03-17T14:44:21Z

npm/create-cypress-tests/src/component-testing/babel/babelTransform.ts

+          const ifComponentMode = babelTypes.ifStatement(
+            babelTypes.binaryExpression(
+              '===',
+              babelTypes.identifier('config.mode'),


@lmiller1990 once you will came up with the final API – please edit this line

Suggested change

babelTypes.identifier('config.mode'),

babelTypes.identifier('config.testingType'),

And update snapshots to verify everything works as expected

Ok, I can do it.

lmiller1990 · 2021-03-18T02:58:11Z

I made some improvements as per this specification file.

testingType is now on config when received as the second argument to the plugins function. The way is not the cleanest - I'm not sure the best place to merge it onto config. Is there a better pattern for this?
testingType is available on window.Cypress.testingType. It is not available in Cypress.config('testingType'). This matches other variables like arch and browser.

packages/server/lib/plugins/child/run_plugins.js

packages/server/lib/plugins/index.js

packages/server/lib/util/settings.js

packages/server/lib/config_options.ts

packages/server/lib/util/settings.js

lmiller1990 · 2021-03-18T23:52:12Z

Improved validation on nested config:

chrisbreiding

Great stuff! 👍

…)" This reverts commit 43c8ae2.

cypress-bot · 2021-04-05T22:41:02Z

Released in 7.0.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v7.0.0, please open a new issue.

feat: cypress.json and plugin runner specific overrides

7bc0f0d

lmiller1990 commented Mar 16, 2021

View reviewed changes

lmiller1990 requested review from chrisbreiding, flotwig, brian-mann, dmtrKovalenko and JessicaSachs March 16, 2021 15:53

Merge branch 'develop' into cypress-json

9fd4ab0

lmiller1990 marked this pull request as ready for review March 16, 2021 15:54

lmiller1990 added the internal-priority label Mar 16, 2021

Change wizard to inject component testing configuration under if stat…

9be6d0c

…ement

chrisbreiding requested changes Mar 16, 2021

View reviewed changes

chrisbreiding reviewed Mar 16, 2021

View reviewed changes

packages/server/lib/util/settings.js Show resolved Hide resolved

dmtrKovalenko reviewed Mar 17, 2021

View reviewed changes

lmiller1990 added 3 commits March 18, 2021 11:21

Merge branch 'develop' into cypress-json

952341a

merge mode argument into config object

4e03a43

add validation

d3d281d

lmiller1990 added 4 commits March 18, 2021 13:01

make testingType accessible

05696da

fix tests

47d9076

mode -> testingType

d228106

change mode -> testingType in tests

088e432

lmiller1990 requested a review from chrisbreiding March 18, 2021 03:57

correctly configure testingType on Cypress

8f65206

lmiller1990 mentioned this pull request Mar 18, 2021

docs: document mode argument to plugins cypress-io/cypress-documentation#3693

Merged

chrisbreiding requested changes Mar 18, 2021

View reviewed changes

lmiller1990 added 2 commits March 19, 2021 08:54

Merge branch 'develop' into cypress-json

bd11c66

fix basic style of code

a88eac9

lmiller1990 added 3 commits March 19, 2021 10:07

return true for valid config

8e0ad91

add more tests

fec1b69

Merge branch 'develop' into cypress-json

56f450d

chrisbreiding approved these changes Mar 19, 2021

View reviewed changes

lmiller1990 added 3 commits March 19, 2021 23:37

Merge branch 'develop' into cypress-json

08932a8

Merge branch 'develop' into cypress-json

243cd31

Merge branch 'develop' into cypress-json

c813e36

chrisbreiding merged commit 43c8ae2 into develop Mar 22, 2021

chrisbreiding added a commit that referenced this pull request Mar 22, 2021

Revert "feat: support ct/e2e specific overrides in cypress.json (#15526…

e37ade2

…)" This reverts commit 43c8ae2.

lmiller1990 deleted the cypress-json branch March 22, 2021 03:49

cypress-bot bot locked as resolved and limited conversation to collaborators Apr 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support ct/e2e specific overrides in cypress.json #15526

feat: support ct/e2e specific overrides in cypress.json #15526

lmiller1990 commented Mar 16, 2021 •

edited by jennifer-shehane

cypress-bot bot commented Mar 16, 2021

lmiller1990 Mar 16, 2021

github-actions bot commented Mar 16, 2021

cypress bot commented Mar 16, 2021 •

edited

brian-mann commented Mar 16, 2021

chrisbreiding left a comment

JessicaSachs commented Mar 16, 2021 •

edited

brian-mann commented Mar 16, 2021

brian-mann commented Mar 16, 2021

JessicaSachs commented Mar 16, 2021

chrisbreiding commented Mar 16, 2021

chrisbreiding commented Mar 16, 2021

brian-mann commented Mar 16, 2021

lmiller1990 commented Mar 17, 2021

dmtrKovalenko Mar 17, 2021

lmiller1990 Mar 17, 2021

lmiller1990 commented Mar 18, 2021 •

edited

lmiller1990 commented Mar 18, 2021

chrisbreiding left a comment

cypress-bot bot commented Apr 5, 2021

	babelTypes.identifier('config.mode'),
	babelTypes.identifier('config.testingType'),

feat: support ct/e2e specific overrides in cypress.json #15526

feat: support ct/e2e specific overrides in cypress.json #15526

Conversation

lmiller1990 commented Mar 16, 2021 • edited by jennifer-shehane

User facing changelog

1. Add support for runner specific overrides in cypress.json. Like this:

2. Pass testingType to config in plugin function

Additional details

How has the user experience changed?

PR Tasks

cypress-bot bot commented Mar 16, 2021

lmiller1990 Mar 16, 2021

Choose a reason for hiding this comment

github-actions bot commented Mar 16, 2021

cypress bot commented Mar 16, 2021 • edited

Test summary

Run details

brian-mann commented Mar 16, 2021

chrisbreiding left a comment

Choose a reason for hiding this comment

JessicaSachs commented Mar 16, 2021 • edited

brian-mann commented Mar 16, 2021

brian-mann commented Mar 16, 2021

JessicaSachs commented Mar 16, 2021

chrisbreiding commented Mar 16, 2021

chrisbreiding commented Mar 16, 2021

brian-mann commented Mar 16, 2021

lmiller1990 commented Mar 17, 2021

dmtrKovalenko Mar 17, 2021

Choose a reason for hiding this comment

lmiller1990 Mar 17, 2021

Choose a reason for hiding this comment

lmiller1990 commented Mar 18, 2021 • edited

lmiller1990 commented Mar 18, 2021

chrisbreiding left a comment

Choose a reason for hiding this comment

cypress-bot bot commented Apr 5, 2021

lmiller1990 commented Mar 16, 2021 •

edited by jennifer-shehane

1. Add support for runner specific overrides in `cypress.json`. Like this:

2. Pass `testingType` to `config` in plugin function

cypress bot commented Mar 16, 2021 •

edited

JessicaSachs commented Mar 16, 2021 •

edited

lmiller1990 commented Mar 18, 2021 •

edited