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: Use .config files #18578

Merged
merged 80 commits into from
Nov 2, 2021

Conversation

estrada9166
Copy link
Member

@estrada9166 estrada9166 commented Oct 21, 2021

User facing changelog

How to test it:

The way to test it is adding cypress.config.{ts|js} to an existing project, if there's acypress.json file it should fail, but if there's no, it should work as it was before.

A user should be able to pass --config-file flag as well, and use custom config files with the file extension ts|js.

Also, it should work when init the component testing.

Additional details

  • Use cypress.config.{ts|js} instead of cypress.json
  • Renamed isFirstTimeCT for isCTConfigured making it clear if when the config is loaded it is set or not
  • Throw error if cypress.json exists - migrate, or remove It depending of the case
  • Do not create default config file
  • Set the config on the cache after reading it, and using that cache if it exists to prevent the need of spawning a child process - code

Todo:

  • Add listener to cypress.config.{ts|js}

How has the user experience changed?

PR Tasks

  • Have tests been added/updated?
  • Has the original issue or this PR been tagged with a release in ZenHub?
  • Has a PR for user-facing changes been opened in cypress-documentation?
  • Have API changes been updated in the type definitions?
  • Have new configuration options been added to the cypress.schema.json?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Oct 21, 2021

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Oct 22, 2021



Test summary

4207 1 50 2Flakiness 0


Run details

Project cypress
Status Failed
Commit e3ead50
Started Nov 2, 2021 2:24 PM
Ended Nov 2, 2021 2:35 PM
Duration 11:18 💡
OS Linux Debian - 10.9
Browser Electron 93

View run in Cypress Dashboard ➡️


Failures

cypress/integration/commands/net_stubbing_spec.ts Failed
1 network stubbing > waiting and aliasing > can timeout incrementally waiting on responses

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

@estrada9166
Copy link
Member Author

@lmiller1990 it seems we were testing it incorrectly yesterday - if you add "cypress:open": "cross-env TZ=America/New_York node ../../scripts/cypress open --project ${PWD} --config-file my-cypress.config.ts", on packages/launchpad/package.json and rename the config file for my-cypress.config.ts - it should be working fine

@lmiller1990
Copy link
Contributor

lmiller1990 commented Nov 1, 2021

There is a few things that need to be addressed. I'm guessing they are out of scope of this specific PR, but maybe we can do them as features fork from this branch, before we merge this branch.

There is a lot more complexity with the new .config.ts API and how we handle migration with the UI than I expected.

1. Conflicting cypress.json and cypress.config.js

I ran cd packages/launchpad && LAUNCHPAD=1 yarn cypress:open in a project with both cypress.json and cypress.config.js and got:

(alejandro/unified/use-config-file*) $ LAUNCHPAD=1 yarn cypress:open
yarn run v1.22.11
$ cross-env TZ=America/New_York node ../../scripts/cypress open --project ${PWD}
(node:47676) Warning: Accessing non-existent property 'padLevels' of module exports inside circular dependency
(Use `node --trace-warnings ...` to show where the warning was created)
(node:47677) Warning: Accessing non-existent property 'padLevels' of module exports inside circular dependency
(Use `Cypress --trace-warnings ...` to show where the warning was created)
Error: There is both a `cypress.config.ts` and a cypress.json file at the location below:
/Users/lachlan/code/work/cypress4/packages/launchpad

Cypress no longer supports 'cypress.json' config, please remove it from your project.

    at get (/Users/lachlan/code/work/cypress4/packages/server/lib/errors.js:1049:15)
    at Object.throwErr [as throw] (/Users/lachlan/code/work/cypress4/packages/server/lib/errors.js:1067:9)
    at Object.error (/Users/lachlan/code/work/cypress4/packages/server/lib/makeDataContext.ts:121:45)
    at ConfigDataSource.getDefaultConfigBasename (/Users/lachlan/code/work/cypress4/packages/data-context/src/sources/ConfigDataSource.ts:36:49)
    at /Users/lachlan/code/work/cypress4/packages/data-context/src/sources/ConfigDataSource.ts:14:36
    at ProjectDataSource.isTestingTypeConfigured (/Users/lachlan/code/work/cypress4/packages/data-context/src/sources/ProjectDataSource.ts:80:24)
    at ProjectActions.setActiveProject (/Users/lachlan/code/work/cypress4/packages/data-context/src/actions/ProjectActions.ts:45:29)
    at Function.all (<anonymous>:null:null)
 {
  isCypressErr: true,
  type: 'LEGACY_CONFIG_FILE',
  details: undefined
}
Error: There is both a `cypress.config.ts` and a cypress.json file at the location below:
/Users/lachlan/code/work/cypress4/packages/launchpad

Cypress no longer supports 'cypress.json' config, please remove it from your project.

    at get (/Users/lachlan/code/work/cypress4/packages/server/lib/errors.js:1049:15)
    at Object.throwErr [as throw] (/Users/lachlan/code/work/cypress4/packages/server/lib/errors.js:1067:9)
    at Object.error (/Users/lachlan/code/work/cypress4/packages/server/lib/makeDataContext.ts:121:45)
    at ConfigDataSource.getDefaultConfigBasename (/Users/lachlan/code/work/cypress4/packages/data-context/src/sources/ConfigDataSource.ts:36:49)
    at /Users/lachlan/code/work/cypress4/packages/data-context/src/sources/ConfigDataSource.ts:14:36
    at ProjectDataSource.isTestingTypeConfigured (/Users/lachlan/code/work/cypress4/packages/data-context/src/sources/ProjectDataSource.ts:80:24)
    at ProjectActions.setActiveProject (/Users/lachlan/code/work/cypress4/packages/data-context/src/actions/ProjectActions.ts:45:29)
    at Function.all (<anonymous>:null:null)

✨  Done in 5.03s.

Seems right - except the error is printed twice, a little strange.

Adding a new project without a config file:

I tried adding a project without any cypress config. I got this error. You can reproduce it by removing cypress.config.js from packages/runner than adding that project using global mode.

image

The current behavior is to create a new cypress.json containing {}. I'm not sure what is supposed to happen now, or if it's part of this PR.

First time setup

I opened packages/runner and it's asking me to set up e2e.

CT Setup

I did what it said (copy the code into my cypress.config.js but I got an error - have we implemented the component part of cypress.config.ts yet?

image

Recommending ES Modules when we have cypress.config.js (as commonjs)

The runner project has cypress.config.js (JS file) containing:

module.exports = {
  // ...
}

But the CLI is recommending I add a ES module to my TS file (when I have a JS file):

Screen Shot 2021-11-01 at 2 37 15 pm

@estrada9166
Copy link
Member Author

  1. Conflicting cypress.json and cypress.config.js
    The reason for this duplicated error is because we are throwing the errors from the data-context, and those are being handled here where basically the defaultErrorHandler has this
console.log(err)
console.log(err.stack)

Not sure if we should explore changes for this in this PR on a different one

Adding a new project without a config file:

We may need to decide the next steps here in the upcoming PR's, we are not creating config files anymore

CT Setup

This has not been implemented yet

Recommending ES Modules when we have cypress.config.js (as commonjs)

The file name is hardcoded

Copy link
Contributor

@marktnoonan marktnoonan left a comment

Choose a reason for hiding this comment

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

I was able to get into some states where it's not possible to recover without using the terminal, eg when adding a project in global mode that uses an older version of Cypress. I know we plan to address the error handling and edge cases in future PRs so I'll document what I found separately to make it into tickets.

Also confirmed that if I'm not doing things that are already known to cause errors, everything else is looking good for me and merging this doesn't seem like it would interfere with any other work.

Copy link
Member

@tgriesser tgriesser left a comment

Choose a reason for hiding this comment

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

I think we should go ahead and merge, with the understanding that if it's a major blocker for anything we can always revert - and we can bugfix any of the edge cases we run into in the meantime - having more eyes on it in the unify branch will be helpful

@tgriesser tgriesser merged commit 081dd19 into unified-desktop-gui Nov 2, 2021
@tgriesser tgriesser deleted the alejandro/unified/use-config-file branch November 2, 2021 14:24
tgriesser added a commit that referenced this pull request Nov 4, 2021
…ve-activeProject

* unified-desktop-gui: (57 commits)
  chore: Add e2e tests for global mode (#18719)
  chore: add percy to app and launchpad package (#18781)
  chore: update test
  refactor: move settings in app (#18729)
  feat: setup launchpad lifecycle (#18734)
  feat(app): decouple event manager from driver (#18695)
  chore: Force single resolution for core modules, infinite loop guard (#18764)
  fix(driver): Sticky elements within a fixed container will not prevent an element from being scrolled to (#18441)
  chore: cleaning up the runner container pattern (#18741)
  feat: Use .config files (#18578)
  chore(app): basic style and example to stop scrollIntoView bug (#18736)
  chore: make `create` function on server.ts obsolete (#18615)
  feat: add codegen utility (#18708)
  docs: Add instructions to squash commits to develop in Contributing (#18728)
  fix(@cypress/react): throw if using Next.js swc-loader without nodeVersion=system (#18686)
  refactor: remove Ramda (#18723)
  fix: support using create-cypress-tests as part of build process (#18714)
  chore: Increase paralleled machines for desktop-gui tests (#18725)
  fix(app): do not cache graphql (#18716)
  chore: Update Chrome (stable) to 95.0.4638.69 (#18696)
  ...
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.

None yet

5 participants