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: support ct/e2e specific overrides in cypress.json #15444

Merged
merged 10 commits into from
Mar 13, 2021

Conversation

lmiller1990
Copy link
Contributor

@lmiller1990 lmiller1990 commented Mar 12, 2021

Note: This behavior and API was discussed over the course of two meetings and validated with Brian, Zach, and Jess.

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 mode to plugins function

Note the third argument is mode. This is component or e2e. The user can have conditional plugins using this flag.

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

  if (mode === '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] NO. NEED TO DO THIS PRE 7.0.
  • Have API changes been updated in the type definitions? Yes.
  • Have new configuration options been added to the cypress.schema.json? Yes.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Mar 12, 2021

Thanks for taking the time to open a PR!

module.exports = (on, config) => {
module.exports = (on, config, mode) => {
if (mode !== 'component') {
throw Error('This is an component project. mode should be `component`.')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a good way to test that mode is passed correctly. We expect this project to be only used with runner-ct.

@github-actions
Copy link
Contributor

Internal Jira issue: TR-702

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

isComponentTesting (options = {}) {
return options.experimentalComponentTesting || options.componentTesting || options.testingType === 'component'
},
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 have a lot of flags for CT, apparently... we should reduce these to just one or two eventually.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oy vey. Yeah we need to do this like ASAP.

@cypress
Copy link

cypress bot commented Mar 12, 2021



Test summary

3982 0 49 2


Run details

Project cypress
Status Passed
Commit 3245164
Started Mar 13, 2021 12:57 AM
Ended Mar 13, 2021 1:07 AM
Duration 09:18 💡
OS Linux Debian - 10.5
Browser Chrome 83

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

Copy link
Contributor

@elevatebart elevatebart left a comment

Choose a reason for hiding this comment

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

Overall great PR.
A few improvements to the json schema.

I believe the word mode is used to differentiate run from open.
Should it be used as well for component vs e2e?
That is confusing.
How about context or buildMode?

cli/schema/cypress.schema.json Outdated Show resolved Hide resolved
npm/vue/cypress/plugins/index.js Show resolved Hide resolved
packages/server-ct/src/project-ct.ts Show resolved Hide resolved
@elevatebart
Copy link
Contributor

elevatebart commented Mar 12, 2021

Is there a place where we can test the new JSON schema?
I do not see it used anywhere.

An idea could be to use the $schema flag at the top of a file containing a valid cypress.json with all options just to make sure it still works.
This can be done in a spec using ajv
EDIT: will be fined in https://cypress-io.atlassian.net/browse/CT-347

Copy link
Contributor

@elevatebart elevatebart left a comment

Choose a reason for hiding this comment

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

To avoid blocking the PR, I change my review to comment

JessicaSachs
JessicaSachs previously approved these changes Mar 12, 2021
@chrisbreiding
Copy link
Contributor

We had to revert this for 6.7.1 in #15499. I'd recommend re-submitting it against the 7.0-release branch so it will go out with 7.0.0.

I don't think we should add a 3rd argument to the plugins file function. It should be a property on the config argument. We've augmented that object before for environment details.

@gdibble
Copy link

gdibble commented Jul 8, 2021

We had to revert this for 6.7.1 in #15499. I'd recommend re-submitting it against the 7.0-release branch so it will go out with 7.0.0.

I don't think we should add a 3rd argument to the plugins file function. It should be a property on the config argument. We've augmented that object before for environment details.

Hi @chrisbreiding, did cypress.config.[js,ts] land in v7?
🙏 Thanks

@lmiller1990
Copy link
Contributor Author

@gdibble not yet, it is getting worked on. You can follow the progress here: #17000

@gdibble
Copy link

gdibble commented Jul 8, 2021

TY @lmiller1990 - I appreciate the tracking info 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants