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

fix: match alternate edge version string format #25457

Conversation

CarbonCollins
Copy link
Contributor

@CarbonCollins CarbonCollins commented Jan 13, 2023

User facing changelog

Additional details

Edge was returning an alternate format for its version string which meant the browser detection version regex was not matching. This change allows the regex to match on both the original form and the new format which switches the version number and channel places arround.

Affects of this change would allow Cypress to match version strings from Edge in both Microsoft Edge Beta 109.0.1518.49 and Microsoft Edge 109.0.1518.49 beta forms. This also applies to the Edge Canary and Edge Dev browsers.

Steps to test

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?

@CarbonCollins
Copy link
Contributor Author

I've been looking into this one a little more and I'm not sure if the current format of these regex in the develop branch are even correct in the first place... I get this alternate format on both my linux machine and on a MacOS machine too.

Do we know if they worked prior to me raising this PR as I would like to determine if we need to still support the existing version string format?

@CarbonCollins
Copy link
Contributor Author

To further follow up im seeing it in Microsoft Edge 110.0.1587.6 dev on my machine and Microsoft Edge 110.0.1587.6 Dev on the mac hence the case insensitivity addtion, however with the way that the code currently extracts the version from the regex the commit in this PR wont work...

@CarbonCollins CarbonCollins force-pushed the add-alt-version-forms-for-edge-browser branch from ee5a41e to 124b7a1 Compare January 13, 2023 15:13
@CarbonCollins CarbonCollins marked this pull request as ready for review January 26, 2023 08:10
@cypress
Copy link

cypress bot commented Jan 26, 2023

2 failed and 43 flaky tests on run #43464 ↗︎

2 26598 1274 0 Flakiness 43

Details:

Merge branch 'develop' into add-alt-version-forms-for-edge-browser
Project: cypress Commit: e127b8d995
Status: Failed Duration: 19:25 💡
Started: Jan 26, 2023 8:25 AM Ended: Jan 26, 2023 8:44 AM
Failed  e2e/origin/commands/waiting.cy.ts • 1 failed test • 5x-driver-electron

View Output Video

Test
cy.origin waiting > alias > waits for the route alias to have a response
Failed  migration.cy.ts • 1 failed test • launchpad-e2e

View Output Video

Test
global mode > migrates 2 projects in global mode Screenshot
Flakiness  e2e/origin/cookie_behavior.cy.ts • 4 flaky tests • 5x-driver-electron

View Output Video

Test
... > same site / cross origin > XMLHttpRequest > sets cookie on same-site request if withCredentials is true, and attaches to same-site request if withCredentials is true
... > same site / cross origin > fetch > sets same-site cookies if "include" credentials option is specified from request, but does not attach same-site cookies to request by default (same-origin)
... > same site / cross origin > XMLHttpRequest > sets cookie on same-site request if withCredentials is true, and attaches to same-site request if withCredentials is true
... > same site / cross origin > fetch > sets same-site cookies if "include" credentials option is specified from request, but does not attach same-site cookies to request by default (same-origin)
Flakiness  cypress/cypress.cy.js • 3 flaky tests • 5x-driver-electron

View Output Video

Test
... > correctly returns currentRetry
... > correctly returns currentRetry
... > correctly returns currentRetry
Flakiness  commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-firefox

View Output Video

Test
network stubbing > intercepting request > can delay and throttle a StaticResponse
Flakiness  e2e/origin/commands/waiting.cy.ts • 1 flaky test • 5x-driver-firefox

View Output Video

Test
cy.origin waiting > alias > waits for the route alias to have a response
Flakiness  cypress/cypress.cy.js • 3 flaky tests • 5x-driver-firefox

View Output Video

Test
... > correctly returns currentRetry
... > correctly returns currentRetry
... > correctly returns currentRetry

The first 5 flaky specs are shown, see all 21 specs in Cypress Cloud.

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@CarbonCollins
Copy link
Contributor Author

Just to extend from my previous message at least, the updated regex should allow for both forms of the version string, the original Microsoft Edge Dev 110.0.0.0 and the one I'm seeing Microsoft Edge 110.0.0.0 Dev / Microsoft Edge 110.0.0.0 dev Without needing to change the way its decoded further down using a hardcoded capture group number...

I'm not sure if its the most appropriate way of doing this but the regex in the PR should return the version in either form as the 1st capture group regardless allowing the downstream code to remain unchanged.

cli/CHANGELOG.md Outdated Show resolved Hide resolved
cli/CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@AtofStryker AtofStryker left a comment

Choose a reason for hiding this comment

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

I'd like to get some unit tests in the future inside browsers_spec to validate Edge discovery seeing we don't have any, but I feel that's likely out of scope here.

@AtofStryker AtofStryker merged commit 53db0ea into cypress-io:develop Jan 30, 2023
tgriesser added a commit that referenced this pull request Jan 31, 2023
* develop:
  chore: fix changlelog section parsing and reference right ENV (#25633)
  feat: Debug page [IATR] (#25488)
  fix(deps): update dependency underscore.string to v3.3.6 🌟 (#25574)
  chore: Use upstream cypress-testing-library again (#25548)
  fix: match alternate edge version string format (#25457)
  chore: update name for graphql batch operations (#25610)
  chore: clean up config for external contibutors (#25552)
  chore: fix childProcess.execSync encoding (#25625)
  chore: update next-version to handle using the next bump package.json… (#25599)
  chore: update packages/example deployment script and cleanup package/example (#25091)
  fix: allow version 9 of the babel-loader peer dependency (#25569)
  docs: remove cypress-example-todomvc-redux from release-process (#25613)
  chore: bump version and remove misleading changelog entry (#25612)
BlueWinds pushed a commit that referenced this pull request Feb 1, 2023
* fix: match alternate edge version string format (#25457)

* fix: match alternate edge version string format

* chore: add changelog entry

* Apply suggestions from code review

* Update cli/CHANGELOG.md

* Update cli/CHANGELOG.md

* chore: update changelog to release on Tuesday for 12.5

* [run ci]

---------

Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>
Co-authored-by: Bill Glesias <bglesias@gmail.com>

* chore: Use upstream cypress-testing-library again (#25548)

* chore: Use upstream cypress-testing-library again

* Update cypress-example-kitchensink commit hash

* Revert "Update cypress-example-kitchensink commit hash"

This reverts commit 8de5d1f.

---------

Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>

* fix(deps): update dependency underscore.string to v3.3.6 🌟 (#25574)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Bill Glesias <bglesias@gmail.com>

* feat: Debug page [IATR] (#25488)

Co-authored-by: Zachary Williams <ZachJW34@gmail.com>
Co-authored-by: Ankit <ankit@cypress.io>
Co-authored-by: Stokes Player <stokes.player@gmail.com>
Co-authored-by: elevatebart <bart@cypress.io>
Co-authored-by: Rocky <25568640+rockindahizzy@users.noreply.github.com>
Co-authored-by: Stokes Player <stokes@cypress.io>
Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>
Co-authored-by: Mark Noonan <mark@cypress.io>
Co-authored-by: Mike Plummer <mikep@cypress.io>
Co-authored-by: amehta265 <65267668+amehta265@users.noreply.github.com>
Co-authored-by: Adam Stone-Lord <adams@cypress.io>
Co-authored-by: Mike Plummer <mike-plummer@users.noreply.github.com>
Co-authored-by: Lachlan Miller <lachlan.miller.1990@outlook.com>

* chore: fix changlelog section parsing and reference right ENV (#25633)

* test: skip flaky migration test (#25378)

* chore: Update README to add Cloud badges (#25645)

* perf: remove reporter logs for collapsed tests in run mode (#25632)

Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>

* chore: 12.5.0 release (#25648)

* dependency: update dependency simple-git to v3.16.0 [security] (#25603)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Zachary Williams <zachjw34@gmail.com>
Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>

* chore: renovate semantic types and percy ci updates (#25651)

---------

Co-authored-by: Steven Collins <steven@carboncollins.uk>
Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>
Co-authored-by: Bill Glesias <bglesias@gmail.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Stokes Player <stokes@cypress.io>
Co-authored-by: Zachary Williams <ZachJW34@gmail.com>
Co-authored-by: Ankit <ankit@cypress.io>
Co-authored-by: Stokes Player <stokes.player@gmail.com>
Co-authored-by: elevatebart <bart@cypress.io>
Co-authored-by: Rocky <25568640+rockindahizzy@users.noreply.github.com>
Co-authored-by: Mark Noonan <mark@cypress.io>
Co-authored-by: Mike Plummer <mikep@cypress.io>
Co-authored-by: amehta265 <65267668+amehta265@users.noreply.github.com>
Co-authored-by: Adam Stone-Lord <adams@cypress.io>
Co-authored-by: Mike Plummer <mike-plummer@users.noreply.github.com>
Co-authored-by: Lachlan Miller <lachlan.miller.1990@outlook.com>
Co-authored-by: Jennifer Shehane <jennifer@cypress.io>
Co-authored-by: Matt Schile <mschile@cypress.io>
@AtofStryker AtofStryker removed their assignment Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cypress not finding beta/dev builds of Edge on linux
3 participants