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

Wrong typings in cypress-npm-api.d.ts and cli/types/cypress.d.ts #23010

Open
mirobo opened this issue Jul 29, 2022 · 6 comments
Open

Wrong typings in cypress-npm-api.d.ts and cli/types/cypress.d.ts #23010

mirobo opened this issue Jul 29, 2022 · 6 comments
Labels
E2E Issue related to end-to-end testing good first issue Good for newcomers Triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team. type: bug type: typings Issue related to Cypress types (for TypeScript)

Comments

@mirobo
Copy link
Contributor

mirobo commented Jul 29, 2022

Current behavior

When using Cypress NPM API, i.e. cypress.run(..) or cypress.open(..) the parameter "specPattern" inside "config" does not exist. Yet it can be used and it actually works :-)

Please don't change that it works :-P

  const config = {
    project: absoluteTestPath,
    reporter: 'junit',
    headless: false,
    browser: 'electron',
    testingType: 'e2e',
    config: {
      specPattern: specList,
      video: false
    }
  };

The error was introduced here: 54f3b90#diff-9fde1fb2f7eaba5ea49b585c811461495bfbba88e614e900a96863ecfe6b980aR2985

  /**
   * Config options that can be assigned on cypress.config.{ts|js} file
   */
  type UserConfigOptions<ComponentDevServerOpts = any> = Omit<ResolvedConfigOptions<ComponentDevServerOpts>, 'baseUrl' | 'excludeSpecPattern' | 'supportFile' | 'specPattern'>

Not quite sure why the UserConfigOptions are omitting excludeSpecPattern and specPattern (personally I don't care about supportFile, but if it works, it should not be omitted). Same goes for baseUrl.

I think it would be helpful to A) clean wrong omissions and maybe don't maintain two "sources of truth" for which parameters are possible: https://docs.cypress.io/guides/guides/module-api

I'd rather like to see just an automatically generated list of options, derived from the type UserConfigOptions<ComponentDevServerOpts = any> ? But this is probably something cypress-documentation should think about :D. If the type is correct I'm happy but the documentation should be up to date.

Desired behavior

Fixed types

Test code to reproduce

n/a

Cypress Version

10.3.1

Other

No response

@cypress-bot cypress-bot bot added the stage: investigating Someone from Cypress is looking into this label Jul 29, 2022
@tbiethman
Copy link
Contributor

@mirobo thanks for the ticket. I believe the current typing indicates our desired usage, which would be to nest the specPattern under an associated e2e or component key:

  const config = {
    project: absoluteTestPath,
    reporter: 'junit',
    headless: false,
    browser: 'electron',
    testingType: 'e2e',
    config: {
      e2e: {
        specPattern: specList,
      },
      video: false
    }
  };

It's a little verbose, as you're already specifying the testingType separately, but the config types and config docs do communicate a consistent API. I don't see any mention of specPattern being supported directly in the module api docs. Let me know if I'm overlooking something or if something could be made clearer.

@cypress-bot cypress-bot bot added stage: awaiting response Potential fix was proposed; awaiting response and removed stage: investigating Someone from Cypress is looking into this labels Aug 1, 2022
@mirobo
Copy link
Contributor Author

mirobo commented Aug 2, 2022

Thanks for the hint. But I wonder why it works if it's wrong. I fixed it in my project.
But I've found other stuff:

config.e2e shows these options (and they work):
image

but the documentation shows only couple of options:
https://docs.cypress.io/guides/references/configuration#e2e
image

  1. Is it immediately clear based on the documentation that the base options are also configurable inside "e2e"?

  2. slowTestThreshold can be configured in both places (but the documentation doesn't mention this as a base configuration)

image

  1. Also the option "modifyObstructiveCode" disappeared from the type? It's wrong inside e2e and also inside config..

image
image

@cypress-bot cypress-bot bot added stage: investigating Someone from Cypress is looking into this and removed stage: awaiting response Potential fix was proposed; awaiting response labels Aug 2, 2022
@tbiethman
Copy link
Contributor

@mirobo ah yeah, I can confirm your findings here.

1 and 2: I believe some typings are being reused here without the expected omits, which is very confusing. Especially so because as you said the values work in both locations. We can do better here.

3: It does appear modifyObstructiveCode has been incorrectly removed from the config type, we will get that fixed.

@cypress-bot cypress-bot bot added stage: routed to e2e-core and removed stage: investigating Someone from Cypress is looking into this labels Aug 2, 2022
@mjhenkes mjhenkes added type: typings Issue related to Cypress types (for TypeScript) E2E-core type: bug labels Aug 2, 2022
@mschile mschile added triage and removed triage labels Aug 18, 2022
@mirobo
Copy link
Contributor Author

mirobo commented Aug 22, 2022

Point 3 is already adressed here #22146

@nagash77 nagash77 added the good first issue Good for newcomers label Oct 24, 2022
@nagash77 nagash77 added E2E Issue related to end-to-end testing and removed E2E-core labels Nov 8, 2022
@nagash77 nagash77 added Triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team. and removed routed-to-e2e labels Apr 19, 2023
@jennifer-shehane
Copy link
Member

Is this still an issue in 13.x versions? We made some updates to typings.

@mirobo
Copy link
Contributor Author

mirobo commented Oct 5, 2023

Is this still an issue in 13.x versions? We made some updates to typings.

I didn't check it yet. I'm closing the issue and I'd reopen a new one if still relevant. First we'll need to check if the Cypress Gateway Connector still let's us use sorry-cypress with 13.x (I know it should!).

On a side note:
Sad that it had to come to this: https://currents.dev/posts/v13-blocking because we need an on-premise solution and not getting future upgrades might (in long term) just lead us back to some simple parallel test execution by splitting tests based on the number of runners. This won't pay for the future development of Cypress and I hope that there will be maybe an initiative to fund it's development with tiny voluntary support payments by the users themselves? I personally think that the Cypress - the company - deserves that and I still very much prefer Cypress over Playwright, no matter how many people tell "I'm switching to Playwright". I wonder why Google or Meta doesn't publicly show interest in the tool, I think it would nice if they also support it :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E2E Issue related to end-to-end testing good first issue Good for newcomers Triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team. type: bug type: typings Issue related to Cypress types (for TypeScript)
Projects
None yet
Development

No branches or pull requests

7 participants