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: Resolve Git Info when using alternative shells or Windows #22741

Merged
merged 14 commits into from
Jul 15, 2022

Conversation

mike-plummer
Copy link
Contributor

@mike-plummer mike-plummer commented Jul 11, 2022

  • Update *nix flow to use /bin/sh rather than user's active shell
  • Add unit tests for filename edge cases

User facing changelog

  • Allow Git Information to be retrieved for specs when using less common UNIX shells like Fish
  • Fix issue that prevented displayed "Last Updated" information on Windows for Git repositories without a commit history

Additional details

Existing implementation attempted to use the user's active shell when executing Git info commands, falling back to Bash if no shell was specified on the env variable. This worked fine for most users since most Linux flavors default to Bash, and Mac uses Zsh which happens to understand the syntax we were using. However, some users with less common shells like Fish were seeing failures since it uses a different variable syntax.

This PR updates our Posix logic to use /bin/sh (the execa default) which should be available on all *nix flavors (Bash isn't 100% available on all flavors). It also addresses an edge case where files with double quotes in their path would break the command, and adds unit tests to validate special character edge cases.

Steps to test

UNIX Systems
Validate that files with special characters are properly handled and that alternative user shells don't break the experience

  1. Create a new empty Cypress project
  2. Initialize your project as a Git repo (git init)
  3. Open project in the app
  4. Create an empty spec named file"withOneDoubleQuote.cy.js
  5. Observe that "Last Updated" info displays in the Specs Explorer along with a "Created" icon
    • App successfully handled file with double quote character when processing Last Updated info
  6. Close the app
  7. Install the FISH shell (brew install fish) or via another install mechanism
  8. In your command prompt switch your active shell to Fish (fish) and open Cypress
  9. Observe that "Last Updated" info displays in the Specs Explorer along with a "Created" icon
    • App successfully acquired Git information despite running in Fish shell

Windows Systems
Validate that "Last Updated" info is displayed for empty Git repositories

  1. Create a new empty Cypress project, open in the app
  2. Create an empty spec
  3. Observe that "Last Updated" info displays in Specs Explorer, but no "created/modified" icon is displayed
  4. Close the app
  5. Initialize your project as a Git repo (git init)
  6. Reopen project in Cypress, observe that last updated info still displays and now shows a "Created" icon in Specs Explorer
    • App successfully acquired "Last Updated" info on Windows for an empty Git repository (no commit history)

How has the user experience changed?

  • Last Updated information now populates when using an alternative shell on Unix systems
State Before After
Non-Git Project in Fish FISH-Before-NoGit FISH-After-NoGit
Git project in Fish FISH-Before-EmptyGit FISH-After-EmptyGit
  • Last Updated information now populates on Windows for projects that are newly-initialized Git repositories
State Before After
Non-Git Project WIN-Before-NoGit WIN-After-NoGit
Empty Git project WIN-Before-EmptyGit WIN-After-EmptyGit
Git Project w/ history WIN-Before-NonEmptyGit WIN-After-NonEmptyGit

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?- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

* Update *nix flow to use /bin/sh rather than user's active shell
* Add unit tests for filename edge cases
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 11, 2022

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Jul 11, 2022



Test summary

37721 0 456 0Flakiness 10


Run details

Project cypress
Status Passed
Commit 0f21321
Started Jul 15, 2022 11:10 PM
Ended Jul 15, 2022 11:26 PM
Duration 16:45 💡
OS Linux Debian - 10.11
Browser Multiple

View run in Cypress Dashboard ➡️


Flakiness

commands/actions/click.cy.js Flakiness
1 ... > scroll-behavior > can scroll to and click elements in html with scroll-behavior: smooth
commands/xhr.cy.js Flakiness
1 ... > logs Method, Status, URL, and XHR
2 ... > logs response
3 ... > logs response
next.cy.ts Flakiness
1 Working with next-12 > should detect new spec
This comment includes only the first 5 flaky tests. See all 10 flaky tests in the Cypress Dashboard.

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

@mike-plummer mike-plummer changed the title fix: Use default shell for Git Info on *nix fix: Git Info does not always resolve when using alternative shells or Windows Jul 14, 2022
@mike-plummer mike-plummer changed the title fix: Git Info does not always resolve when using alternative shells or Windows fix: Resolve Git Info when using alternative shells or Windows Jul 14, 2022
@mike-plummer mike-plummer marked this pull request as ready for review July 14, 2022 13:18
@mike-plummer mike-plummer requested review from a team as code owners July 14, 2022 13:18
@mike-plummer mike-plummer removed request for a team July 14, 2022 13:18
Copy link
Contributor

@astone123 astone123 left a comment

Choose a reason for hiding this comment

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

This works for me using Fish 👍🏻 🐟

mike-plummer and others added 3 commits July 14, 2022 18:10
* Ensure escaping is performed on unix when only one file is being checked
* Rearrange escaping logic to perform escaping after resolving path
* Add comment to test identifying associated GH issue
Copy link
Contributor

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

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

Just some small comments, sorry I took so long to review this.

packages/data-context/src/sources/GitDataSource.ts Outdated Show resolved Hide resolved
@mike-plummer mike-plummer merged commit 59c926b into develop Jul 15, 2022
@mike-plummer mike-plummer deleted the 22454-use-default-shell-for-git-info branch July 15, 2022 23:55
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 19, 2022

Released in 10.3.1.

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

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Jul 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support getting spec Git info when active shell differs in syntax from bash
4 participants