chore: wire up Cypress Studio #23413

lmiller1990 · 2022-08-18T03:04:22Z

Closes Wire Up Cypress Studio in Cypress 10 #23336

Note targeting a feature branch, feat/studio-cypress-10.

User facing changelog

N/A - internal for now

Additional details

Wire up Cypress Studio in Cypress 10. There's a bit going on, and it's not fully complete but works at a basic level. It's going into a feature branch, so things like tests, some cleanup etc will come before we merge the whole thing into develop.

Learn about Studio: https://docs.cypress.io/guides/references/cypress-studio#What-you-ll-learn

Steps to test

WIP, there's probably lots of edge cases that are kind of busted so I wouldn't go too far. Here's a demo of how it works, you are welcome to play around and try it out!

The final review will be towards the end once we've got a proper UI in place. It's hard to really review without the full Studio UI in place - that's why it's targeting a feature branch.

Here's a quick demo (not sure why the test fails, but the wiring is working - you can see it generated a spec from my actions).

studi0demo.mp4

Any small bugs or minor fixes will be done in #23461, which is the feature branch. The final review of this is where we need to be more thorough and deliberate.

How has the user experience changed?

PR Tasks

Have tests been added/updated?
Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
Has a PR for user-facing changes been opened in cypress-documentation?
Have API changes been updated in the type definitions?

cypress-bot · 2022-08-18T03:04:24Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

cypress · 2022-08-18T04:54:07Z

Test summary

4938 • 0 • 86 • 0 • 1

Run details


Project	cypress
Status	Passed
Commit	`c7d66f5`
Started	Aug 23, 2022 2:12 AM
Ended	Aug 23, 2022 2:26 AM
Duration	13:50 💡
OS	Linux Debian - 11.3
Browser	Firefox 99

View run in Cypress Dashboard ➡️

Flakiness

	cypress/e2e/commands/net_stubbing.cy.ts		1
1	network stubbing > intercepting request > can delay with deprecated delayMs param

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

lmiller1990 · 2022-08-19T09:01:28Z

packages/app/cypress/e2e/cypress-in-cypress-e2e.cy.ts

@@ -148,7 +148,7 @@ describe('Cypress In Cypress E2E', { viewportWidth: 1500, defaultCommandTimeout:
  })

  it('shows a compilation error with a malformed spec', { viewportHeight: 596, viewportWidth: 1000 }, () => {
-    const expectedAutHeight = 500 // based on explicitly setting viewport in this test to 596
+    const expectedAutHeight = 456 // based on explicitly setting viewport in this test to 596


I changed the <a> for the AUT URL to be an <input> so we can type into it to navigate to a URL for Cypress Studio, and for some reason the wrapping break points for flex-wrap is slightly different. I have no idea why and was quite stuck on this, but for all intensive purposes the UI is the same, except things wrap in the AUT header slightly earlier if you have your Cypress window really small.

We can probably revisit this in the final pass if needed, but I don't think it makes any difference.

lmiller1990 · 2022-08-19T09:01:47Z

packages/app/cypress/e2e/cypress-in-cypress.cy.ts

@@ -243,7 +243,7 @@ describe('Cypress in Cypress', { viewportWidth: 1500, defaultCommandTimeout: 100
      cy.visitApp()
      cy.contains('dom-content.spec').click()

-      cy.contains('http://localhost:4455/cypress/e2e/dom-content.html').should('be.visible')
+      cy.findByTestId('aut-url-input').invoke('val').should('contain', 'http://localhost:4455/cypress/e2e/dom-content.html')


Bit more specific about what's going on, just a general improvement.

lmiller1990 · 2022-08-19T12:11:14Z

packages/app/cypress/e2e/cypress-in-cypress-e2e.cy.ts

@@ -148,7 +148,7 @@ describe('Cypress In Cypress E2E', { viewportWidth: 1500, defaultCommandTimeout:
  })

  it('shows a compilation error with a malformed spec', { viewportHeight: 596, viewportWidth: 1000 }, () => {
-    const expectedAutHeight = 500 // based on explicitly setting viewport in this test to 596
+    const expectedAutHeight = 456 // based on explicitly setting viewport in this test to 596


Change <a> to <input> for the Studio URL input - this caused the breakpoint for flex: flex-wrap to change slightly. Seems like it's not a big deal.

ZachJW34

Looks like a good start! Didn't do too deep a dive but it looks like your goal of getting most of it wired up is working. Just a few comments, since it's WIP I'll leave it up to you to address them or not

packages/app/src/runner/useEventManager.ts

ZachJW34 · 2022-08-19T20:10:20Z

packages/app/src/store/studio-store.ts

+      }
+    },
+
+    setFileDetails (fileDetails) {


I'm not able to save the file as it fails with error:

ENOENT: no such file or directory, open '/Users/zachjw/work/component-test-repos/create-next-app/create-next-app/cypress/e2e/passing.cy.js'

Notice the projectName is being appended twice create-next-app/create-next-app. I traced it down to stripCustomProtocol, might need to tweak the regex

Weird, I am not seeing this. I may need help to reproduce this one.

ZachJW34 · 2022-08-19T20:18:48Z

packages/reporter/src/test/test.tsx

+          <WandIcon />
+        </a>
+      </Tooltip>,
+    )


Note for _launchStudio below:

I wasn't able to get studio to launch unless I clicked the icon, navigated away from the runner and then back in. I refactored the function to set the studioActive property and it started working

_launchStudio = (e: MouseEvent) => { e.preventDefault() e.stopPropagation() const { model, events } = this.props action('studio:init:test', () => { appState.setStudioActive(true) events.emit('studio:init:test', model.id) })() }

I am not able to reproduce this one. Let's sync up!

mike-plummer

Had to debug around a bit like Zach to get studio to activate, but once I figured out the trick I was able to record & save a couple specs without issue. Awesome work getting this much working so quickly!

packages/app/src/runner/event-manager.ts

lmiller1990 · 2022-08-22T05:06:53Z

@mike-plummer @ZachJW34 it sounds like you've both encountered a bug that I am not able to reproduce. I'm going to address all the other comments for now (mainly refactors/tweaks) and we can sync up so I can see how to reproduce the "cannot activate studio" problem you've both encountered.

Hmm what URL are you both entering?

Edit: pushing a few commits now anyway - may solve the issue, let's see! Thanks for the quick review.

lmiller1990 · 2022-08-22T05:36:16Z

New demo:

new-demop.mp4

mike-plummer · 2022-08-22T13:24:56Z

@lmiller1990 It's working for me after pulling down your latest commits 🥳 . I didn't dig too far into it on Friday but I think the issue was something around the showedStudioModal state flag - it looked like there used to be a "first studio run" init modal that isn't there anymore and maybe we were falling into that? (event-manager.ts, line 285) In any event, seems to be working now but I'll let Zach confirm

emilyrohrbough

We have very little test coverage for studio and we should add these as cypress-in-cypress tests before this is released. We are missing coverage in the app, launchpad and server (no socket tests for studio). We should also add tests to verify the studio json stats are correctly being read/written with file paths/OS.

emilyrohrbough · 2022-08-22T13:49:36Z

packages/reporter/src/instruments/instrument-model.ts

@@ -1,5 +1,5 @@
 import { observable } from 'mobx'
-import { TestState } from '../test/test-model'
+import { Instrument, TestState } from '@packages/types'


@packages/types needed added in the package.json

Done, thanks

emilyrohrbough · 2022-08-22T13:51:06Z

packages/reporter/src/runnables/runnable-and-suite.tsx

@@ -63,6 +69,7 @@ const Suite = observer(({ eventManager = events, model }: SuiteProps) => {
 export interface RunnableProps {
  model: TestModel | SuiteModel
  appState: AppState
+  experimentalStudioActive: boolean


nit: name-spacing this with experimental will require updating this when this comes out of experimental.

Hmm good point, maybe we just name it studioEnabled?

I'll think about this a bit more.

Hmm actually, maybe we just do a check against Cypress.config('experimentalStudio') and drop this prop entirely? This seems to be the pattern for experimentalSessionAndOrigin (using Cypress.config). *Edit: this is the pattern for driver, reporter never accesses the Cypress global).

I might just change this to studioEnabled.

I changed it to studioEnabled.

emilyrohrbough · 2022-08-22T13:53:49Z

packages/launchpad/cypress/e2e/config-warning.cy.ts

@@ -87,7 +87,9 @@ describe('experimentalSingleTabRunMode', () => {
  })
 })

-describe('experimentalStudio', () => {
+// TODO: Figure out this. experimentalStudio is back, but must be nested under E2E? Or, does


Is there an issue for this? & why are these tests skipped now? I'd expect them to run & pass before this merges

We should add the issue number to this TODO: #23338

There is #23338
Marked as blocking Studio release!

It's done here #23506

emilyrohrbough · 2022-08-22T13:56:56Z

packages/reporter/src/test/test.tsx

@@ -106,6 +106,7 @@ interface TestProps {
  runnablesStore: RunnablesStore
  scroller: Scroller
  model: TestModel
+  experimentalStudioEnabled: boolean


Default prop value for experimentalStudioEnabled should be added as false throughout the reporter.

I would have thought that as long as we set the top level prop in <Reporter /> it'll get passed down - eg, since it's not optional, we don't need a default value? And TS should complain if we forgot to pass it down somewhere -- or maybe the TS config for reporter isn't strict enough?

I'll take a look but I don't think setting a default value for a non nullable boolean prop makes sense, does it?

Will definitely get a chance to re-assess once thing final PR opens, but it seems like we should only need default values for nullable props.

emilyrohrbough · 2022-08-22T14:08:08Z

packages/reporter/src/test/test.cy.tsx

+        model={model}
+        appState={appState}
+        experimentalStudioEnabled={false}
+      />
    </div>)

    cy.percySnapshot()


I'd expect tests added for when studio is enabled. When is that work planned if it's not happening here? There are quite a skipped tests in the reporter atm for studio. Here's a few I know off-hand

https://github.com/cypress-io/cypress/blob/develop/packages/reporter/cypress/e2e/runnables.cy.ts#L174

https://github.com/cypress-io/cypress/blob/develop/packages/reporter/cypress/e2e/tests.cy.ts#L137

https://github.com/cypress-io/cypress/blob/develop/packages/reporter/cypress/e2e/suites.cy.ts#L133

Oh I did not realize we had these skipped - I'll go back and add them in now in #23461

Ok un-skipped.

I will write E2E tests in a separate PR once the UI stuff is done, and you'll be tagged to review 💯

9c24014
c7d66f5

emilyrohrbough · 2022-08-22T14:30:27Z

packages/app/src/runner/StudioControls.vue

@@ -0,0 +1,118 @@
+<template>
+  <div v-if="!studioStore.url && studioStore.isActive">


We are missing tests for this component.

Yep, thanks for calling this out -- DM'd you, gonna merge this to the feature branch and add a bunch of tests here: #23461

emilyrohrbough · 2022-08-22T14:35:20Z

Tried out your branch. It wasn't working for me:

Screen.Recording.2022-08-22.at.9.29.02.AM.mov

Also noticing we don't have the experiment listed in the settings page and the config option is not shown in the resolved settings:

We are also missing the setting's localized strings as well. The last two experiments we releases had them missing 😅

astone123 · 2022-08-22T15:00:08Z

Also noticing we don't have the experiment listed in the settings page and the config option is not shown in the resolved settings

@emilyrohrbough I think we're going to handle adding the flag back with this issue

astone123 · 2022-08-22T15:08:13Z

packages/reporter/src/runnables/runnable-and-suite.tsx

-        {model.type === 'test' ? <Test model={model as TestModel} /> : <Suite model={model as SuiteModel} />}
+        {model.type === 'test'
+          ? <Test model={model as TestModel} experimentalStudioEnabled={experimentalStudioActive} />
+          : <Suite model={model as SuiteModel} experimentalStudioActive={experimentalStudioActive} />}


I think we should use experimentalStudioEnabled for this prop name. This property describes whether or not Studio is enabled in the Cypress configuration, not whether Studio is currently active. We should be careful not to conflate the two

Done, globally renamed to studioEnabled. I also dropped the experimental prefix, which doesn't add much value internally as pointed out by https://github.com/cypress-io/cypress/pull/23413/files#r951467056

ZachJW34 · 2022-08-22T17:10:56Z

@lmiller1990 UI is now working for me. Also, the problem with saving the file seems to be only related to Next.js apps, it's working on other applications just fine now.

What I'm seeing for Next is that the originalFile that comes from the stack details is webpack://create-next-app/./cypress/e2e/passing.cy.js, whereas a normal webpack app is webpack:///./cypress/e2e/commands/actions/a.cy.js. Can you try saving a studio generated spec within a Next.js app and see if you run into the problem?

Not a blocker since I know your goal is to try and get close to a 1-to-1 implementation of the previous studio but would be good to followup on this.

lmiller1990 · 2022-08-22T21:05:01Z

We have very little test coverage for studio and we should add these as cypress-in-cypress tests before this is released. We are missing coverage in the app, launchpad and server (no socket tests for studio). We should also add tests to verify the studio json stats are correctly being read/written with file paths/OS.

@emilyrohrbough Yep, we have an entire ticket for writing a bunch of tests before moving the feature branch to review: #23461

(Note this branch is targeting said feature branch).

It isn't working for me

That is weird, I wonder what's going on 🤔 stupid question, I wonder if you need to yarn install (although yarn dev should get you the latest bundle...)

Oh it's likely around a modal that's supposed to show but isn't - I think I got my appPreferences into some weird state where it says "showedStudioModal: true" but yours is still false (and since the modal isn't implemented, it never shows).

This should be addressed by the UI update when it lands.

Either way, I will fix your basic comments - for tests and for the experimental flag, we've got separate tickets. I'll add you as a reviewer when the final PR is opened.

lmiller1990 · 2022-08-22T23:16:47Z

@ZachJW34 thanks for calling out Next.js weirdness, going to triage this separately -- I think 9.x suffers the same problem.

lmiller1990 · 2022-08-23T01:55:14Z

I unskipped the skipped tests in reporter:

In c7d66f5 there was some race condition I think - I just moved the tests around (reorder) and it's working fine now. I am not sure if this was originally flaky or something changed. Either way, these tests are only really unit/integration - I'm excited to add some real E2E tests next using Cypress in Cypress.

lmiller1990 · 2022-08-23T02:34:47Z

Ok, gonna merge this up and continue working on tests in #23461.

* chore: wire up Cypress Studio (#23413) * 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 * fix a bug in the assertion API * fix bug in assertions [skip ci] * wip - bugs [skip ci] * feat: add experimentalStudio flag back (#23506) Co-authored-by: astone123 <adams@cypress.io> * chore: Add Studio UI to Cypress 10 (#23537) * 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> * test: studio e2e tests (#23546) * 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 * chore: UI feedback * fix race condition * update tests * rename test * improve types in reporter * remove dead code * improve tests * merge tests into one spec * chore: Cap instruction modal width; exit studio mode when new spec is chosen * chore: Only render studio error when test has failed; add test for studioEnabled * correctly check if command is studio or not * improve specs and hopefully reduce flake * communicate studio state from app->reporter * receive studio save state validity from app * fix test * improve test coverage * fix external link Co-authored-by: astone123 <adams@cypress.io>

lmiller1990 added 9 commits August 4, 2022 15:14

wip

66387f0

wip

1131c73

Merge remote-tracking branch 'origin/develop' into lmiller/spike-studio

0404134

wip - spike

a61e667

merge in develop [skip ci]

0e9863f

more wip [skip ci]

ab31dcc

update style

87fb624

fix ts

ba24e88

Merge remote-tracking branch 'origin/develop' into lmiller/spike-studio

42043a1

lmiller1990 changed the title ~~Lmiller/spike studio~~ chore: wire up Cypress Studio Aug 18, 2022

lmiller1990 mentioned this pull request Aug 18, 2022

spike: Cypress Studio #23181

Closed

4 tasks

lmiller1990 added 3 commits August 18, 2022 13:56

move types around

09d2786

extract types

c3506a6

lint

db23066

mschile added triage and removed triage labels Aug 18, 2022

lmiller1990 added 9 commits August 19, 2022 15:13

Merge remote-tracking branch 'origin/develop' into lmiller/spike-studio

a0438da

fixing tests

ca309ea

fix component test

5b07449

skip some tests

770e033

do not error on experimentalStudio flag

a7b714e

add studio controls placeholder

fc66414

fixing tests

55e3a31

revert

08d023f

revert changes

62a4f51

lmiller1990 changed the base branch from develop to feat/studio-cypress-10 August 19, 2022 08:57

lmiller1990 commented Aug 19, 2022

View reviewed changes

lmiller1990 requested a review from a team August 19, 2022 10:58

lmiller1990 commented Aug 19, 2022

View reviewed changes

ZachJW34 approved these changes Aug 19, 2022

View reviewed changes

mike-plummer approved these changes Aug 19, 2022

View reviewed changes

packages/app/src/runner/event-manager.ts Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/develop' into lmiller/spike-studio

219fc41

lmiller1990 added 3 commits August 22, 2022 15:13

simplify code

fd0ee2b

simplify code

fa5c9eb

lift check into useEventManager

4fdb73f

lmiller1990 added 2 commits August 22, 2022 15:42

correctly hide create studio prompt based on flag;

9a4e5fd

remove superfulous css

65eeae3

emilyrohrbough requested changes Aug 22, 2022

View reviewed changes

astone123 reviewed Aug 22, 2022

View reviewed changes

lmiller1990 added 5 commits August 23, 2022 09:42

rename variables

5f3bd1f

fix bugs

82b43c4

wip

4af852a

unskip tests

9c24014

unskip more tests

c7d66f5

lmiller1990 merged commit c38e019 into feat/studio-cypress-10 Aug 23, 2022

lmiller1990 deleted the lmiller/spike-studio branch August 23, 2022 02:34

		@@ -0,0 +1,118 @@
		<template>
		<div v-if="!studioStore.url && studioStore.isActive">

chore: wire up Cypress Studio #23413

chore: wire up Cypress Studio #23413

Conversation

lmiller1990 commented Aug 18, 2022 • edited Loading

User facing changelog

Additional details

Steps to test

How has the user experience changed?

PR Tasks

cypress-bot bot commented Aug 18, 2022

cypress bot commented Aug 18, 2022 • edited Loading

Test summary

Run details

Flakiness

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZachJW34 left a comment

Choose a reason for hiding this comment

ZachJW34 Aug 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmiller1990 Aug 22, 2022 • edited Loading

Choose a reason for hiding this comment

mike-plummer left a comment

Choose a reason for hiding this comment

lmiller1990 commented Aug 22, 2022 • edited Loading

lmiller1990 commented Aug 22, 2022

mike-plummer commented Aug 22, 2022

emilyrohrbough left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmiller1990 Aug 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmiller1990 Aug 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emilyrohrbough commented Aug 22, 2022

astone123 commented Aug 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZachJW34 commented Aug 22, 2022

lmiller1990 commented Aug 22, 2022 • edited Loading

lmiller1990 commented Aug 22, 2022

lmiller1990 commented Aug 23, 2022

lmiller1990 commented Aug 23, 2022

lmiller1990 commented Aug 18, 2022 •

edited

Loading

cypress bot commented Aug 18, 2022 •

edited

Loading

ZachJW34 Aug 19, 2022 •

edited

Loading

lmiller1990 Aug 22, 2022 •

edited

Loading

lmiller1990 commented Aug 22, 2022 •

edited

Loading

lmiller1990 Aug 22, 2022 •

edited

Loading

lmiller1990 Aug 22, 2022 •

edited

Loading

lmiller1990 commented Aug 22, 2022 •

edited

Loading