Skip to content

fix(cli): decrease the length of pre-release cache path#20961

Merged
flotwig merged 3 commits intodevelopfrom
unify-1449-beta-slug-length
Apr 7, 2022
Merged

fix(cli): decrease the length of pre-release cache path#20961
flotwig merged 3 commits intodevelopfrom
unify-1449-beta-slug-length

Conversation

@flotwig
Copy link
Copy Markdown
Contributor

@flotwig flotwig commented Apr 7, 2022 •
edited
Loading

User facing changelog

  • Decreased the length of the Cypress cache path when installing pre-releases. This should improve the experience for Windows users, where the max path length is 260 characters by default.

Additional details

  • Pre-release paths are getting ridiculously long since fix: use unique install cache folders for betas #20296 leading to issues like trying to unpack C:\Users\Administrator\AppData\Local\Cypress\Cache\beta-10.0.0-10.0-release-404447e397190ebb0f8ce0bb270ea9a806ca78c6\Cypress\resources\app\node_modules\remark-mdx\node_modules\@babel\plugin-proposal-object-rest-spread\node_modules\@babel\helper-plugin-utils\lib and failing on Windows because the path is too long (261 characters)
  • Removed version from the path altogether since it's not relevant in development. Keeping version as a rough indicator of when the pre-release was installed.
  • Trimmed 40-character Git SHA to 8 characters. Combined with the branch name this is still highly unlikely to collide.

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)
  • [na] Has a PR for user-facing changes been opened in cypress-documentation?
  • [na] Have API changes been updated in the type definitions?
  • [na] Have new configuration options been added to the cypress.schema.json?

@cypress-bot
Copy link
Copy Markdown
Contributor

cypress-bot bot commented Apr 7, 2022

Thanks for taking the time to open a PR!

@cypress
Copy link
Copy Markdown

cypress bot commented Apr 7, 2022 •
edited
Loading


Test summary

19363 • 0 • 218 • 0 • Flakiness 3

Run details
Project cypress
Status Passed
Commit d89262e
Started Apr 7, 2022 5:02 PM
Ended Apr 7, 2022 5:14 PM
Duration 11:39 💡
OS Linux Debian - 10.10
Browser Multiple

View run in Cypress Dashboard ➡️

Flakiness

settings_spec.js 1 Flakiness
1 Settings > file preference panel > loads preferred editor, available editors and shows spinner
commands/files_spec.js 1 Flakiness
1 ... > has implicit existence assertion, retries and throws a specific error when file does not exist for null encoding
cypress/proxy-logging_spec.ts 1 Flakiness
1 Proxy Logging > request logging > xhr log has response body/status code when xhr response is logged second

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

@flotwig flotwig marked this pull request as ready for review April 7, 2022 15:32
@flotwig flotwig requested a review from a team as a code owner April 7, 2022 15:32
@flotwig flotwig requested review from jennifer-shehane and marktnoonan and removed request for a team and jennifer-shehane April 7, 2022 15:32
mjhenkes
mjhenkes previously approved these changes Apr 7, 2022
View reviewed changes
emilyrohrbough
emilyrohrbough reviewed Apr 7, 2022
View reviewed changes
cli/__snapshots__/install_spec.js
* Commit Timestamp: 1996-11-27Txx:xx:xx.000Z

Installing Cypress (version: https://cdn.cypress.io/beta/binary/0.0.0-development/darwin-x64/aBranchName-abc123/cypress.zip)
Installing Cypress (version: https://cdn.cypress.io/beta/binary/0.0.0-development/darwin-x64/aBranchName-3b7f0b5c59def1e9b5f385bd585c9b2836706c29/cypress.zip)
Copy link
Copy Markdown
Member

@emilyrohrbough emilyrohrbough Apr 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

shouldn't the sha be 8 chars & the version dropped?

Copy link
Copy Markdown
Contributor Author

@flotwig flotwig Apr 7, 2022

Choose a reason for hiding this comment

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

This is the CDN URL, which this PR isn't changing

Copy link
Copy Markdown
Member

@emilyrohrbough emilyrohrbough Apr 7, 2022

Choose a reason for hiding this comment

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

ahh gottchya

emilyrohrbough
emilyrohrbough reviewed Apr 7, 2022
View reviewed changes
cli/lib/tasks/state.js

const getVersionDir = (version = util.pkgVersion(), buildInfo = util.pkgBuildInfo()) => {
if (buildInfo && !buildInfo.stable) {
version = ['beta', version, buildInfo.commitBranch, buildInfo.commitSha].join('-')
Copy link
Copy Markdown
Member

@emilyrohrbough emilyrohrbough Apr 7, 2022

Choose a reason for hiding this comment

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

I personally like the version being included so you know what you are working with 🤷🏻‍♀️

Copy link
Copy Markdown
Member

@emilyrohrbough emilyrohrbough Apr 7, 2022

Choose a reason for hiding this comment

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

NVM. your above comment clarified.

Since this is just the cache dir, beta could also prob beta since the branch name shows its a non-release ver

Copy link
Copy Markdown
Contributor Author

@flotwig flotwig Apr 7, 2022

Choose a reason for hiding this comment

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

I was thinking that version is sorta useless since we use semantic versioning and it can change based on your branch's base commits, but I guess it is still useful as a rough indicator of when the pre-release came out w.r.t Cypress versions. I can add it back in.

Copy link
Copy Markdown
Contributor Author

@flotwig flotwig Apr 7, 2022

Choose a reason for hiding this comment

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

d89262e

emilyrohrbough
emilyrohrbough previously approved these changes Apr 7, 2022
View reviewed changes
@flotwig flotwig dismissed stale reviews from emilyrohrbough and mjhenkes via d89262e April 7, 2022 16:57
@flotwig flotwig requested a review from mjhenkes April 7, 2022 16:58
@flotwig flotwig merged commit 6a4823b into develop Apr 7, 2022
@flotwig flotwig deleted the unify-1449-beta-slug-length branch April 7, 2022 17:41
@cypress-bot
Copy link
Copy Markdown
Contributor

cypress-bot bot commented Apr 11, 2022

Released in 9.5.4.

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

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Apr 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@emilyrohrbough emilyrohrbough emilyrohrbough approved these changes

@marktnoonan marktnoonan Awaiting requested review from marktnoonan

@davidmunechika davidmunechika Awaiting requested review from davidmunechika

@mjhenkes mjhenkes Awaiting requested review from mjhenkes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@flotwig @mjhenkes @emilyrohrbough