-
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
feat: Cypress Studio for Cypress 10 #23544
Conversation
* wip * wip * wip - spike * more wip [skip ci] * update style * fix ts * move types around * extract types * lint * fixing tests * fix component test * skip some tests * do not error on experimentalStudio flag * add studio controls placeholder * fixing tests * revert * revert changes * rename store * rename method * remove comment * refactor * correctly feature flag studio * simplify code * simplify code * lift check into useEventManager * correctly hide create studio prompt based on flag; * remove superfulous css * rename variables * fix bugs * wip * unskip tests * unskip more tests
Co-authored-by: astone123 <adams@cypress.io>
Thanks for taking the time to open a PR!
|
Test summaryRun details
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 |
* wip * wip * wip - spike * more wip [skip ci] * update style * fix ts * move types around * extract types * lint * fixing tests * fix component test * skip some tests * do not error on experimentalStudio flag * add studio controls placeholder * fixing tests * revert * revert changes * rename store * rename method * remove comment * refactor * correctly feature flag studio * chore: wip add barebones studio modals * simplify code * simplify code * lift check into useEventManager * correctly hide create studio prompt based on flag; * remove superfulous css * chore: style studio toolbar * chore: misc feedback * chore: remove studio store prop * chore: studio URL prompt and other changes * update component * chore: UI styling and remove studio init modal * chore: revert unnecessary changes * chore: fix types * chore: fix some tests, minor refactor (#23545) * fix test * fix test * add noHelp link to StandardModal Co-authored-by: Lachlan Miller <lachlan.miller.1990@outlook.com>
* add basic e2e test * add some e2e tests for studio and a note on limitations * additional spec * add more tests, refactor helper * fix bug in studio * remove test code
@@ -182,7 +183,7 @@ class Test extends Component<TestProps> { | |||
_controls () { | |||
let controls: Array<JSX.Element> = [] | |||
|
|||
if (this.props.model.state) { | |||
if (this.props.model.state === 'failed') { |
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.
This is a drive-by fix. Right now we render this control even when the test hasn't failed
I love how studio works with the SelectorPlayground, allowing you to have fine grained control on what you are selecting in the DOM! |
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.
Nothing blocking from my comments, mostly nits (and I'm curious about the tests). Going to test on my windows real quick.
Works great!!
Edit: Works perfect on Windows
</template> | ||
</Tooltip> | ||
|
||
<Tooltip |
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.
Tooltip flash when clicking and hovering out could use some tweaks. There is SelectorPlaygroundTooltip.vue
that handles this behavior well
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.
I played around but couldn't find anything weird. Maybe you can demo on a call?
Maybe we want to lift SelectorPlaygroundTooltip
out of selector-playground
and make it a more generic RunnerTooltip
or something? Seems like a nice to have -- not going to block merging this, but let's sync up on this.
Ultimately, I guess this component would go in a design system of sorts.
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.
Pretty minor so not blocking but this is what I was seeing.
Screen.Recording.2022-08-26.at.10.54.13.AM.mov
@@ -0,0 +1,242 @@ | |||
import { launchStudio } from './helper' |
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.
These test are failing when I run them locally, but are passing in CI? Is there something I'm missing when running these tests?
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.
Looking into this... this test is also failing for me but for a different reason
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.
Yeah the tests in this file are really flakey for me... I'm still looking into it. Seems like there's some situations where after launchStudio
runs, we're not in studio mode. Probably a race condition of sorts
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.
Couldn't get this exact error. I tried a little commit 24d8718 to reduce flake. These run reliably for me, and on CI 🤔
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.
Yeah I'm still seeing them flake a bunch in studio.cy.ts
. I've run them several times and can't get them all to pass at once 🤔 Must just be an issue with my machine. If they're running well in CI then I'd say this isn't a blocker
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, amazing job! 💥
@lmiller1990 Percy shows some changes in general modals that were unexpected for me, here and here - I wonder if the change to the |
Yeah it looks like changing that prop added a few "need help?" links to modals that didn't have them before. For example, the generate spec modal has one that is just a blank link. |
}>() | ||
noHelp?: boolean | ||
}>(), { | ||
noHelp: false, |
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.
Because this defaults to false, existing modals that don't pass a helpLink
prop will have links in the modal title that are dead. Thanks @marktnoonan for pointing this out
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.
Glad you checked! Meanwhile I've edited my comment to remove this line:
In actual usage those components are doing what is expected as far as I can tell, so not a blocker.
Because I was checking on a different branch.
Fixed 06debea |
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
User facing changelog
Re-enable Cypress Studio for End to End testing via the
experimentalStudio
flag.Additional details
Steps to test
Watch this video walkthrough on how the feature works (Cypress team only): https://cypressio.slack.com/archives/C011BAPC614/p1661517276740949?thread_ts=1661517030.718579&cid=C011BAPC614 (#team-component-testing channel)
How has the user experience changed?
PR Tasks
cypress-documentation
? Update Cypress Studio documentation cypress-documentation#4696type definitions
?