chore: adding granular Firefox browser validation #23164

tbiethman · 2022-08-05T21:51:20Z

We recently ran into an issue where Cypress failed to connect to Firefox on Windows. A bug was identified within Firefox and a fix was released with v103, but v101 and v102 were not patched. Those versions will never work with Cypress on Windows.

I wanted to ensure we could communicate this incompatibility through the app, rather than have users run into this and have to be redirected to the closed issue. However, there wasn't a clean way of marking these specific version/architecture combinations as unsupported with our current logic. So I did a bit of refactoring to allow our default browsers objects to have an associated validator function that can more dynamically determine Cypress support.

User facing changelog

Stable Firefox versions 101 and 102 are now explicitly rendered as unsupported in Windows due to an unpatched browser incompatibility in those versions.

Additional details

Steps to test

We have pretty good test coverage around our minimum version checks. The new Firefox/Windows checks can be tested by installing 101 (or 102) and launching Cypress. You should see this:

103 should render as normal.

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)
[n/a] Has a PR for user-facing changes been opened in cypress-documentation?
[n/a] Have API changes been updated in the type definitions?

…tbiethman/browser-validation

cypress-bot · 2022-08-05T21:51:23Z

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.

tbiethman · 2022-08-05T21:55:36Z

packages/launcher/lib/browsers.ts


 /** list of the browsers we can detect and use by default */

+export const browsers: Browser[] = [


With the additional logic in play here, it didn't feel right keeping the default browsers defined within the types package. It was a little weird to have them there in the first place? These were originally moved
with 5856776, but I don't see an impact to typings by moving these back.

This array was copied wholesale, with the firefox validator below being the only change.

tbiethman · 2022-08-05T21:59:46Z

packages/launcher/lib/browsers.ts

+      }
+
+      return validateMinVersion(browser)
+    },


I did not utilize the same validator for the dev/nightly channels as the impacts are much more undefined; some 100/101/102/103/104 versions will be impacted, some won't.

In theory we could use the info property to set more a of "ymmv" version of the message for those channels, but it is not actually rendering in the UI at the moment: #21769

This is early in my list to ask to prioritize after the search work is done.

Thanks for the heads up Mark, using info (when it works) would be perfect

tbiethman · 2022-08-05T22:02:37Z

packages/launcher/lib/detect.ts

  }

-  return foundBrowser
+  browser.unsupportedVersion = true
+  browser.warning = warningMessage


I'm noticing now that I abort early if isSupported is truthy; anyone see value 'it is supported but still has a warning' use case?

We sometimes use .warning for supported browsers, like with experimentalFirefoxSupport, there was a warning; data-context adds a warning too if chromeWebSecurity: false is used w/ Firefox... but I can't think of a use case that would apply at /this/ level of the browser detection so this seems fine.

cypress · 2022-08-05T22:43:35Z

Test summary

37846 • 0 • 469 • 0 • 4

Run details


Project	cypress
Status	Passed
Commit	`9100208`
Started	Aug 11, 2022 2:31 PM
Ended	Aug 11, 2022 2:51 PM
Duration	19:28 💡
OS	Linux Debian - 11.3
Browser	Multiple

View run in Cypress Dashboard ➡️

Flakiness

	cypress/proxy-logging.cy.ts	2
1	Proxy Logging > request logging > xhr log has response body/status code when xhr response is logged second
2	Proxy Logging > request logging > xhr log has response body/status code when xhr response is logged second
	next.cy.ts	1
1	Working with next-12.1.6 > should detect new spec
	e2e/origin/commands/assertions.cy.ts	1
1	cy.origin assertions > #consoleProps > .should() and .and()

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

tbiethman · 2022-08-08T14:30:12Z

packages/launcher/test/unit/detect_spec.ts

@@ -110,7 +145,7 @@ describe('browser detection', () => {
            info: 'Loaded from /foo/bar/browser',
            custom: true,
            version: '9001.1.2.3',
-            majorVersion: 9001,
+            majorVersion: '9001',


We were previously setting the majorVersion as an integer if it could be parsed in setMajorVersion. But it's defined as a string on the FoundBrowser type and looks like it should to be (sometimes the version contains chars). So I normalized the type being set, and updated these tests.

…tbiethman/browser-validation

flotwig

Nice, tested that this blocks FF 101 on Windows and not on Linux, looks good.

Noticed a new defect though in the process 😄 #23200

packages/launcher/lib/browsers.ts

packages/launcher/test/unit/browsers_spec.ts

Co-authored-by: Zach Bloomquist <git@chary.us>

packages/launcher/lib/detect.ts

Co-authored-by: Ryan Manuel <ryanm@cypress.io>

ryanthemanuel

One small comment. Tested this and it is working well. Nice work!

cypress-bot · 2022-08-15T22:07:37Z

Released in 10.5.0.

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

tbiethman added 8 commits July 31, 2022 23:38

chore: render FF 101 and 102 as unsupported on Windows

09f0e2b

Merge branch 'develop' of https://github.com/cypress-io/cypress into …

157388c

…tbiethman/browser-validation

Updating specs

35715a4

Little refactor

df942b1

Adding a few more specs

579bfb4

Moving browsers to launcher

72af04d

Adding a few more specs

5aefc51

Removing exports that no longer exist

d498a53

tbiethman commented Aug 5, 2022

View reviewed changes

Merge branch 'develop' into tbiethman/browser-validation

fea836f

tbiethman commented Aug 8, 2022

View reviewed changes

tbiethman added 2 commits August 8, 2022 09:33

Few more tests

3798506

Merge branch 'develop' of https://github.com/cypress-io/cypress into …

ef956b8

…tbiethman/browser-validation

tbiethman marked this pull request as ready for review August 8, 2022 15:19

tbiethman requested a review from a team as a code owner August 8, 2022 15:19

tbiethman changed the title ~~chore: adding granular browser validation~~ chore: adding granular Firefox browser validation Aug 8, 2022

flotwig requested review from flotwig and ryanthemanuel and removed request for a team August 8, 2022 15:34

flotwig approved these changes Aug 8, 2022

View reviewed changes

packages/launcher/lib/browsers.ts Outdated Show resolved Hide resolved

tbiethman commented Aug 9, 2022

View reviewed changes

packages/launcher/test/unit/browsers_spec.ts Outdated Show resolved Hide resolved

tbiethman commented Aug 9, 2022

View reviewed changes

packages/launcher/test/unit/browsers_spec.ts Outdated Show resolved Hide resolved

Apply suggestions from code review for more specific warning message

abd9734

Co-authored-by: Zach Bloomquist <git@chary.us>

ryanthemanuel reviewed Aug 9, 2022

View reviewed changes

packages/launcher/lib/detect.ts Outdated Show resolved Hide resolved

Update packages/launcher/lib/detect.ts

1d444ed

Co-authored-by: Ryan Manuel <ryanm@cypress.io>

ryanthemanuel approved these changes Aug 9, 2022

View reviewed changes

tbiethman added 3 commits August 10, 2022 09:33

Merge branch 'develop' into tbiethman/browser-validation

93d928d

Merge branch 'develop' into tbiethman/browser-validation

99ca917

Merge branch 'develop' into tbiethman/browser-validation

9100208

tbiethman merged commit 6d3b60a into develop Aug 11, 2022

tbiethman deleted the tbiethman/browser-validation branch August 11, 2022 14:51

cypress-bot bot locked as resolved and limited conversation to collaborators Aug 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: adding granular Firefox browser validation #23164

chore: adding granular Firefox browser validation #23164

tbiethman commented Aug 5, 2022 •

edited

Loading

cypress-bot bot commented Aug 5, 2022

tbiethman Aug 5, 2022 •

edited

Loading

flotwig Aug 8, 2022

tbiethman Aug 5, 2022

marktnoonan Aug 9, 2022

tbiethman Aug 10, 2022

tbiethman Aug 5, 2022

flotwig Aug 8, 2022

cypress bot commented Aug 5, 2022 •

edited

Loading

tbiethman Aug 8, 2022 •

edited

Loading

flotwig left a comment

ryanthemanuel left a comment

cypress-bot bot commented Aug 15, 2022


		/** list of the browsers we can detect and use by default */

		export const browsers: Browser[] = [

chore: adding granular Firefox browser validation #23164

chore: adding granular Firefox browser validation #23164

Conversation

tbiethman commented Aug 5, 2022 • edited Loading

User facing changelog

Additional details

Steps to test

How has the user experience changed?

PR Tasks

cypress-bot bot commented Aug 5, 2022

tbiethman Aug 5, 2022 • edited Loading

Choose a reason for hiding this comment

flotwig Aug 8, 2022

Choose a reason for hiding this comment

tbiethman Aug 5, 2022

Choose a reason for hiding this comment

marktnoonan Aug 9, 2022

Choose a reason for hiding this comment

tbiethman Aug 10, 2022

Choose a reason for hiding this comment

tbiethman Aug 5, 2022

Choose a reason for hiding this comment

flotwig Aug 8, 2022

Choose a reason for hiding this comment

cypress bot commented Aug 5, 2022 • edited Loading

Test summary

Run details

Flakiness

tbiethman Aug 8, 2022 • edited Loading

Choose a reason for hiding this comment

flotwig left a comment

Choose a reason for hiding this comment

ryanthemanuel left a comment

Choose a reason for hiding this comment

cypress-bot bot commented Aug 15, 2022

tbiethman commented Aug 5, 2022 •

edited

Loading

tbiethman Aug 5, 2022 •

edited

Loading

cypress bot commented Aug 5, 2022 •

edited

Loading

tbiethman Aug 8, 2022 •

edited

Loading