cypress-io / cypress

Fast, easy and reliable testing for anything that runs in a browser.
https://cypress.io
MIT License
47.08k stars 3.19k forks source link

Import Cypress functions, no more globals #2773

Open sholladay opened 6 years ago

sholladay commented 6 years ago

Current behavior:

Cypress uses global variables: describe, it, etc. This is implicit and makes it unclear where they are coming from. Linters also don't like it and for good reason. On top of that, I really dislike the BDD style and would much prefer TDD (naming these suite and test), but I can't do that since Cypress dictates the name for me and I would never want test as a global variable (too generic and easy to conflict with other pieces of code). There are numerous downsides to global variables and the entire web platform is moving away from them.

Desired behavior:

I can import what I need from Cypress. Something like:


const { describe, it } = require('cypress');

Versions

All versions.

brian-mann commented 6 years ago

This is understandable, but I think there are some counterpoints.

Cypress actually wraps mocha and this is where these globals come from. Both mocha and jest use globals, and they are by far the top two JS testing frameworks by a whole, whole, lot. Does that make it necessarily right? No, but Cypress itself wraps mocha and if we were to make this one part different, it may cause a lot more confusion.

For instance, did you know that it and describe was coming from mocha and not from Cypress?

We also make Cypress and cy globals: which do come from Cypress. We also make expect and assert globals which come from chai.

A bigger discussion could be had in regards to changing these.

As for linters, you can use our cypress-eslint-plugin module which sets those globals for you, or I believe eslint actually supports mocha: true out of the box for things like the env.

Worth discussing.

tnrich commented 6 years ago

@brian-mann I was wondering about linters. Is there a reason that the default cypress folder structure doesn't include a .eslintrc file? Is it just one more thing for users to worry about?

brian-mann commented 6 years ago

@tnrich to be honest I think that's just something we missed and it likely should be part of the scaffolding. It's a good idea. @chrisbreiding thoughts?

chrisbreiding commented 6 years ago

The biggest reason I can see is that eslint-plugin-cypress needs to be installed separately. If we scaffolded an .eslintrc, eslint would error unless you installed it or removed/changed the .eslintrc, which might be annoying.

sholladay commented 6 years ago

did you know that it and describe was coming from mocha and not from Cypress?

Yes, and perhaps it may require some cooperation from mocha to fully resolve. But I didn't mean to single out those variables, it's a general problem. What I think Cypress should do as a first step is to export them so they can be imported from the cypress module namespace. There are many benefits to this: Cypress could extend mocha more easily if you wanted to, linters would no longer need any config, it would be backwards compatible, and it would be sooo much more clear how to Google for them due to the import statement (you would find Cypress docs, which would link to mocha).

We also make Cypress and cy globals: which do come from Cypress. We also make expect and assert globals which come from chai.

Yeah, the global namespace is quite crowded. :)

Also, I love chai, it's a fantastic library. But I don't really like that it is in here by default and made global. I should be able to use any assertion library that I want to, right?. Arguably, it should be a peerDependency. If you want to bundle chai for a slightly more convenient developer experience, fine. But I should still be explicitly importing it.

ccorcos commented 5 years ago

Not being able to do this makes it a nightmare to move to Cypress from another testing suite...

ccorcos commented 5 years ago

Any workaround in the meantime?

maxime1992 commented 5 years ago

Not quite there yet apparently but if you've got typing issues, a huge PR just got merged and hopefully it should help :smile: https://github.com/cypress-io/cypress/pull/3425

janosh commented 3 years ago

I've only recently started using Cypress and so far the onboarding experience has been great. Props to you!

However, regarding this particular question, I think Cypress still has room for improvement. Without meaning to sound condescending, who the heck thought using globals for testing was a good idea in the first place?

Saying other popular frameworks do it this way is no legitimate argument in my opinion. You're still following suit on a bad practice that has proven time and again in software development to lead to confusion, unnecessary tooling and setup steps, and in the end "magical" seeming code. And in return what are the benefits?

With ESM, JavaScript now has such a nice and widely adopted standard for imports and exports. Let's all #UseThePlatform.

sholladay commented 3 years ago

I've stopped using Cypress because of too many rough edges like this. I would strongly encourage the maintainers to drop mocha. It seems to be holding things up quite a bit.

janosh commented 3 years ago

@sholladay What do you use instead (if anything)?

sholladay commented 3 years ago

I've been using AVA with Puppeteer for a little over a year now. It works quite well and I'm very happy with it. To be fair to Cypress, it's not exactly an apples to apples comparison. Cypress has some really nice features that don't come out of the box with an AVA + Puppeteer set up. But there are suitable replacement modules in almost all cases, such as a test recorder to turn your browser activity into a Puppeteer script. And I've had fewer overall problems with the AVA + Puppeteer combo.

janosh commented 3 years ago

I've been looking into Puppeteer as well and had a good time with Ava. Wasn't sure how much convenience you give up but sounds like it's worth a try.

xiaoxiangmoe commented 2 years ago

Any update about this feature?

It will help us a lot for monorepo users. See: https://github.com/cypress-io/cypress/issues/1319#issuecomment-517489190

xiaoxiangmoe commented 2 years ago

@jennifer-shehane What do we need to do to advance this feature to the next stage?

kleinfreund commented 2 years ago

We also make Cypress and cy globals: which do come from Cypress. We also make expect and assert globals which come from chai.

All of these globals should be required to be imported. It would remove pretty much all the weird hoops one has to jump through to get a (still incomplete!) hybrid setup running like it is outlined in https://github.com/cypress-io/cypress/issues/22059. Then it wouldn’t be necessary to catch-all override describe, it, expect, cy, etc., etc. via ambient type declarations that aren’t package-scoped. And sure, it would be nice if Jest did the same (by using the @jest/globals package, you can; see https://jestjs.io/docs/api). Vitest does do this.

mct-dev commented 1 year ago

Throwing in my 2 cents here as well. Would definitely prefer importing over globals. I use cypress + vitest together in a typescript monorepo. The global types for context, expect, etc conflict with each other and cause headaches. The user experience would be the same if you had to import them from cypress instead of using the globals and it would help with these sort of conflicts.

camden-brown commented 1 year ago

Cypress Docs

You can also group your Cypress and Jest tests inside separate folders (not co-located with components).

This is probably one of the current biggest negatives I have with Cypress atm. I totally understand doing this for e2e tests but for component tests it doesn't make a lot of sense. I want to be able to look at every component and see it's corresponding *.cy.ts test otherwise I have to go search for it. As someone who is setting up Cypress in a few different big projects for the first time, immediately having to implement work-arounds and hacks to get the typing to work along-side my Jest tests is disheartening.

If anyone has a hack that doesn't require moving the component tests out of the project src directory feel free to share.

nickshanks commented 1 year ago

@jennifer-shehane or @nagash77 Please remove the breaking-change label. This ticket is only asking that the existing global objects and their types to be re-exported under new locations, in the manner of the '@jest/globals' package. @brian-mann can confirm that such a change would not break anyone.

Notably, it is not asking that the existing globals no longer be exported (which would be a breaking change).

shealavington commented 6 months ago

I've had an issue ('cy' is not defined no-undef). This issue was the first result I found that seemed somewhat related. I found a resolution that may be helpful to some others. By using eslint globals, I added the following to my .eslintrc and the error is no longer thrown for cy/it/describe.

  globals: {
    'cy': 'readonly',
    'it': 'readonly',
    'describe': 'readonly',
  },

You can add any other items to this list that become problematic too, such as beforeEach