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.

Sign up for GitHub

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

Jump to bottom

feat: relaunch browser when switching testing types from app #19961

Merged
merged 10 commits into from
Feb 1, 2022
Merged

feat: relaunch browser when switching testing types from app #19961

merged 10 commits into from
Feb 1, 2022

Conversation

ZachJW34
Copy link
Contributor

@ZachJW34 ZachJW34 commented Jan 29, 2022 •
edited by lmiller1990
Loading

User facing changelog

Relaunch browser when switching testing type

Additional details

Allows the browser to relaunch when switching testing type and also fixes some minor UI issues with the OpenBrowserList (icons).

There was a bug with reconfigureProject that could be seen in the terminal when switching from e2e -> component. reconfigureProject was calling initializeActiveProject but the ProjectLifeCycleManager was calling this as well when you switch testing type. The lifecycle manager should be in charge of spawning a new project since it relies on the config being configured properly so I removed the call in reconfigureProject.

How has the user experience changed?

Screen.Recording.2022-01-28.at.6.17.38.PM.mov

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?
  • Have new configuration options been added to the cypress.schema.json?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jan 29, 2022

Thanks for taking the time to open a PR!

@ZachJW34 ZachJW34 marked this pull request as ready for review January 31, 2022 16:17
ryanthemanuel
ryanthemanuel reviewed Jan 31, 2022
View reviewed changes
packages/launchpad/cypress/e2e/choose-a-browser.cy.ts Outdated
@@ -117,7 +117,8 @@ describe('Choose a Browser Page', () => {
}).as('launchProject')

cy.get('@launchButton').click()
cy.contains('button', 'Opening E2E Testing in Chrome')
// TODO: Relies on ctx.emitter.toLaunchpad which doesn't refresh AUT
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this work being tracked somewhere?

Copy link
Contributor Author

@ZachJW34 ZachJW34 Jan 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I'll create an issue and update this comment. I talked with @tgriesser a bit about this and tried to find out why this wasn't working but had no luck

Edit: I was wrong, it's working 😅 . Since we are stubbing the request, I need to manually emit. Working on updating

@cypress
Copy link

cypress bot commented Jan 31, 2022 •
edited
Loading


Test summary

17925 • 0 • 211 • 0 • Flakiness 1

Run details
Project cypress
Status Passed
Commit 18de8c3
Started Feb 1, 2022 7:46 PM
Ended Feb 1, 2022 7:58 PM
Duration 12:01 💡
OS Linux Debian - 10.10
Browser Multiple

View run in Cypress Dashboard ➡️

Flakiness

cypress/e2e/commands/net_stubbing.cy.ts 1 Flakiness
1 network stubbing > waiting and aliasing > can timeout waiting on a single request using "alias.request"

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

@ZachJW34 ZachJW34 requested a review from estrada9166 January 31, 2022 20:46
lmiller1990
lmiller1990 previously approved these changes Feb 1, 2022
View reviewed changes
packages/frontend-shared/src/assets/icons/testing-type-component_x16.svg
<path d="M2 10H6V14H2V10Z" fill="white" fill-opacity="0.5"/>
<path d="M10 2H14V6H10V2Z" fill="white" fill-opacity="0.5"/>
<path d="M2 6V5C1.44772 5 1 5.44772 1 6H2ZM6 6H7V5H6V6ZM2 14H1C1 14.5523 1.44772 15 2 15V14ZM10 2V1H9V2H10ZM14 2H15C15 1.44772 14.5523 1 14 1V2ZM14 6V7H15V6H14ZM10 6H9V7H10V6ZM10 10H11V9H10V10ZM10 14V15C10.5523 15 11 14.5523 11 14H10ZM2 7H6V5H2V7ZM5 6V10H7V6H5ZM6 9H2V11H6V9ZM3 10V6H1V10H3ZM5 10V14H7V10H5ZM6 13H2V15H6V13ZM3 14V10H1V14H3ZM10 3H14V1H10V3ZM13 2V6H15V2H13ZM14 5H10V7H14V5ZM11 6V2H9V6H11ZM6 11H10V9H6V11ZM9 10V14H11V10H9ZM10 13H6V15H10V13Z" fill="white"/>
</svg>
Copy link
Contributor

@lmiller1990 lmiller1990 Feb 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Random q: where do the icons/markdown come from? I read the icon docs but still don't really understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From Figma. You can click on the icon and export it as an svg. There is also the design-system page that has all the icons as well

https://www.figma.com/file/1WJ3GVQyMV5e7xVxPg3yID/Design-System?node-id=2448%3A3902

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, ok! I thought it was coming from some library or plugin, for some reason. This makes sense now.

@ZachJW34
Copy link
Contributor Author

ZachJW34 commented Feb 1, 2022

Working on fixing the cypress-in-cypress tests. They are passing locally and just passed when rerunning job with ssh, but refuse to pass when running normally 😕

@ZachJW34
Copy link
Contributor Author

ZachJW34 commented Feb 1, 2022

Figured out why cypress-in-cypress tests were failing. The changes I made to sidebar_navigation.cy.ts caused side effects that resulted in those tests failing. Guess you shouldn't stub ctx.actions.project.launchProject as the function will remain stubbed. The reason it passed when I was sshing into the job was that the order that the specs run was preserved whereas when the run in CI normally, they are parallelized and the order is shuffled, causing sidebar_navigation.cy.ts to run before the cypress in cypress tests and leaking the stubbed function into it.

@ZachJW34 ZachJW34 merged commit 02a669d into 10.0-release Feb 1, 2022
@ZachJW34 ZachJW34 deleted the relaunch-browser branch February 1, 2022 21:45
tgriesser added a commit that referenced this pull request Feb 4, 2022
@tgriesser
Merge branch '10.0-release' into tgriesser/refactor/error-improvements
dd5e8e3 
* 10.0-release:
  feat: validate specPattern root level (#19980)
  feat(unify): unsupported browsers (#19997)
  feat: persist isSideNavigationOpen (#20026)
  feat: add types for urqlCacheKeys (#20027)
  fix: rename spec.js to spec.cy.js (#20029)
  feat: waiting for dependencies to be installed in wizard (#19955)
  fix: migrate config fields to correct the location (#19940)
  feat: relaunch browser when switching testing types from app (#19961)
  test: final ct-audit tests and component tweaks (#19948)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmiller1990 lmiller1990 lmiller1990 left review comments

@ryanthemanuel ryanthemanuel ryanthemanuel left review comments

@estrada9166 estrada9166 estrada9166 approved these changes

@elevatebart elevatebart Awaiting requested review from elevatebart

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ZachJW34 @ryanthemanuel @estrada9166 @lmiller1990