-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
fix: scaffold commands file #20398
fix: scaffold commands file #20398
Conversation
Thanks for taking the time to open a PR!
|
Test summaryRun details
View run in Cypress Dashboard ➡️ Flakiness
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, would be good to make the tests a little more thorough if possible, what do you think?
@@ -737,6 +743,7 @@ describe('Launchpad: Setup Project', () => { | |||
cy.contains('cypress.config.ts') | |||
cy.containsPath('cypress/component/index.html') | |||
cy.containsPath(`cypress/support/component.ts`) | |||
cy.containsPath(`cypress/support/commands.ts`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we take this chance to add some tests to actually verify the file is created on the filesystem? These tests just verify we show the file we will create in the UI, not that they are actually written.
We have a pattern for this, see this function: https://github.com/cypress-io/cypress/blob/10.0-release/packages/launchpad/cypress/e2e/scaffold-component-testing.cy.ts#L11-L19 . This one actually verifies the content, too (might be worth doing something similar here?)
I also notice the support/component.ts
contains // TODO
- might be worth finding out what needs to go here and adding that, too? What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works great! Might be worth adding a system test to verify what we scaffold works when all linked up, like adding a custom command to commands.js
is usable in a test, but doesn't have to be in this PR
|
||
private commandsFileBody (language: CodeLanguageEnum) { | ||
return dedent` | ||
// *********************************************** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the language is typescript, Should this file include the example types for these custom commands as well?
* 10.0-release: fix: comment link to accurate docs (#20437) fix: scaffold commands file (#20398) fix(launchpad): support default export (#20383) feat(launchpad): support for Vue CLI 5 (#20413) fix: UNIFY-676 browsers should be configurable in setupNodeEvents (#20367) fix: make launchpad link open default browser (#20399) fix(icons): publish the files in the package fix: build icons in build-prod (#20411) test: migrate module-api to 10.0 chore: build this branch test: migrate module_api to system tests (#20265) chore: remove pkg/driver //@ts-nocheck final (#20169) chore: fix "cannot find module" in clone-repo-and-checkout-release-branch (#20293) chore: Update Chrome (beta) to 99.0.4844.45 (#20234) chore: fix CI cache state for darwin (#20339) Add TODO comments feedback chore: move tests to its own file.
User facing changelog
Additional details
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?cypress.schema.json
?