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: Cypress Studio for Cypress 10 #23544

Merged
merged 30 commits into from Aug 29, 2022
Merged

Conversation

lmiller1990
Copy link
Contributor

@lmiller1990 lmiller1990 commented Aug 25, 2022

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

lmiller1990 and others added 7 commits August 23, 2022 12:34
* 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>
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Aug 25, 2022

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Aug 25, 2022



Test summary

39662 0 3348 0Flakiness 0


Run details

Project cypress
Status Passed
Commit 06debea
Started Aug 29, 2022 9:17 PM
Ended Aug 29, 2022 9:32 PM
Duration 14:25 💡
OS Linux Debian - 11.3
Browser Multiple

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

astone123 and others added 3 commits August 25, 2022 14:47
* 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
@lmiller1990 lmiller1990 changed the title feat: Cypress Studio for Cypress 10 [WIP] feat: Cypress Studio for Cypress 10 Aug 26, 2022
@@ -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') {
Copy link
Contributor

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

@ZachJW34
Copy link
Contributor

I love how studio works with the SelectorPlayground, allowing you to have fine grained control on what you are selecting in the DOM!

Copy link
Contributor

@ZachJW34 ZachJW34 left a 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

packages/app/src/runner/studio/StudioInstructionsModal.vue Outdated Show resolved Hide resolved
</template>
</Tooltip>

<Tooltip
Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Contributor

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'
Copy link
Contributor

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?

Screen Shot 2022-08-26 at 9 55 31 AM

Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor Author

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 🤔

Copy link
Contributor

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

@astone123 astone123 requested review from emilyrohrbough and a team August 26, 2022 17:31
Copy link
Contributor

@mike-plummer mike-plummer left a 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! 💥

@marktnoonan
Copy link
Contributor

marktnoonan commented Aug 29, 2022

@lmiller1990 Percy shows some changes in general modals that were unexpected for me, here and here - I wonder if the change to the no-help prop has jostled some things.

@astone123
Copy link
Contributor

@lmiller1990 Percy shows some changes in general modals that were unexpected for me, here and here - I wonder if the change to the no-help prop has jostled some things.

In actual usage those components are doing what is expected as far as I can tell, so not a blocker.

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,
Copy link
Contributor

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

Copy link
Contributor

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.

@lmiller1990
Copy link
Contributor Author

@lmiller1990 Percy shows some changes in general modals that were unexpected for me, here and here - I wonder if the change to the no-help prop has jostled some things.

Fixed 06debea

@lmiller1990 lmiller1990 merged commit 72b8a65 into develop Aug 29, 2022
@lmiller1990 lmiller1990 deleted the feat/studio-cypress-10 branch August 29, 2022 21:45
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Aug 30, 2022

Released in 10.7.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v10.7.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Aug 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prepare Studio Feature branch for review
6 participants