-
Notifications
You must be signed in to change notification settings - Fork 3.4k
chore: Refactor {launch} from @packages/launcher
#32687
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
base: develop
Are you sure you want to change the base?
Conversation
6e623b6
to
c238b66
Compare
debug('launching browser %o', { browser, url }) | ||
|
||
// We shouldn't need to check this, because FoundBrowser.path is | ||
// not optional. |
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.
Lots of places explicitly coerce unknown POJOs to FoundBrowser
, though, so it's important to keep it
import { removeDuplicateBrowsers } from '@packages/data-context/src/sources/BrowserDataSource' | ||
import { knownBrowsers } from './known-browsers' | ||
import * as darwinHelper from './darwin' | ||
import * as darwinHelper from './darwinHelpers' |
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 were renamed because intellisense was complaining that
/lib/darwin/index.ts
was too similar to /lib/platforms/Darwin.ts
🤷
cypress
|
Project |
cypress
|
Branch Review |
chore/launcher-launch-refactor
|
Run status |
|
Run duration | 43m 32s |
Commit |
|
Committer | Cacie Prins |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
1
|
|
10
|
|
970
|
|
4
|
|
21135
|
View all changes introduced in this branch ↗︎ |
Warning
Partial Report: The results for the Application Quality reports may be incomplete.
UI Coverage
45.56%
|
|
---|---|
|
185
|
|
159
|
Accessibility
97.97%
|
|
---|---|
|
4 critical
8 serious
2 moderate
2 minor
|
|
101
|
Tests for review
cypress/e2e/studio/studio.cy.ts • 1 failed test • app-e2e
Test | Artifacts | |
---|---|---|
Cypress Studio > updates an existing test with assertions |
Test Replay
Screenshots
|
issues/28527.cy.ts • 1 flaky test • 5x-driver-chrome
Test | Artifacts | |
---|---|---|
issue 28527 > fails and then retries and verifies about:blank is not displayed |
Test Replay
Screenshots
|
commands/waiting.cy.js • 2 flaky tests • 5x-driver-chrome:beta
Test | Artifacts | |
---|---|---|
... > errors > throws when route is never resolved |
Test Replay
|
|
... > errors > throws when waiting for 2nd response to route |
Test Replay
|
e2e/origin/config_env.cy.ts • 1 flaky test • 5x-driver-chrome:beta
Test | Artifacts | |
---|---|---|
cy.origin- Cypress.config() > serializable > overwrites different values in secondary if one exists in the primary |
Test Replay
|
issues/28527.cy.ts • 1 flaky test • 5x-driver-chrome:beta
Test | Artifacts | |
---|---|---|
issue 28527 > fails and then retries and verifies about:blank is not displayed |
Test Replay
Screenshots
|
issues/28527.cy.ts • 1 flaky test • 5x-driver-electron
Test | Artifacts | |
---|---|---|
issue 28527 > fails and then retries and verifies about:blank is not displayed |
Test Replay
Screenshots
|
The first 5 flaky specs are shown, see all 8 specs in Cypress Cloud.
Additional details
Browser process spawning was a little unwieldy. This applies the stranger fig pattern by:
packages/launcher/lib/launch.ts
(renamed frombrowsers.ts
to match its export) that focus on its side effects rather than implementationSeveral of the platform classes are empty as placeholders for browser detection logic, and should be expanded as strangler fig is applied to
detect
anddetectByPath
. Once the public interface here is strangled, we should consider whether the public interface for@packages/launcher
still fits its requirements, or if there are better patterns to expose.Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?