Skip to content

Commit

Permalink
[ensure_git_status_clean] fix incorrect "ignored" param handling (#20976
Browse files Browse the repository at this point in the history
)

* [ensure_git_status_clean ] fix unclear param documentation

See also: https://git-scm.com/docs/git-status#Documentation/git-status.txt---ignoredltmodegt

* Fix incorrect test name

* Remove duplicated test

* Fix misleading local variable name

* Fix typo in test names

* [ensure_git_status_clean] fix incorrect "ignored" param handling

* [ensure_git_status_clean] fix incorrect "no" value handling

* [ensure_git_status_clean] add missing invalid values handling

* [ensure_git_status_clean] improve param documentation

* Fix RuboCop line length error
  • Loading branch information
revolter committed Mar 9, 2023
1 parent 3b937c0 commit e55138d
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 11 deletions.
19 changes: 15 additions & 4 deletions fastlane/lib/fastlane/actions/ensure_git_status_clean.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ module SharedValues
class EnsureGitStatusCleanAction < Action
def self.run(params)
if params[:ignored]
ignored_file = params[:ignored]
repo_status = Actions.sh("git status --porcelain --ignored #{ignored_file}")
ignored_mode = params[:ignored]
ignored_mode = 'no' if ignored_mode == 'none'
repo_status = Actions.sh("git status --porcelain --ignored='#{ignored_mode}'")
else
repo_status = Actions.sh("git status --porcelain")
end
Expand Down Expand Up @@ -74,8 +75,18 @@ def self.available_options
type: Boolean),
FastlaneCore::ConfigItem.new(key: :ignored,
env_name: "FL_ENSURE_GIT_STATUS_CLEAN_IGNORED_FILE",
description: "The flag whether to ignore file the git status if the repo is dirty",
optional: true)
description: [
"The handling mode of the ignored files. The available options are: `'traditional'`, `'none'` (default) and `'matching'`.",
"Specifying `'none'` to this parameter is the same as not specifying the parameter at all, which means that no ignored file will be used to check if the repo is dirty or not.",
"Specifying `'traditional'` or `'matching'` causes some ignored files to be used to check if the repo is dirty or not (more info in the official docs: https://git-scm.com/docs/git-status#Documentation/git-status.txt---ignoredltmodegt)"
].join(" "),
optional: true,
verify_block: proc do |value|
mode = value.to_s
modes = %w(traditional none matching)

UI.user_error!("Unsupported mode, must be: #{modes}") unless modes.include?(mode)
end)
]
end

Expand Down
47 changes: 40 additions & 7 deletions fastlane/spec/actions_specs/ensure_git_status_clean_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@
context "when git status is not clean" do
before :each do
allow(Fastlane::Actions).to receive(:sh).with("git status --porcelain").and_return("M fastlane/lib/fastlane/actions/ensure_git_status_clean.rb")
allow(Fastlane::Actions).to receive(:sh).with("git status --porcelain --ignored='traditional'").and_return("M fastlane/lib/fastlane/actions/ensure_git_status_clean.rb\n!! .DS_Store\n!! fastlane/")
allow(Fastlane::Actions).to receive(:sh).with("git status --porcelain --ignored='no'").and_return("M fastlane/lib/fastlane/actions/ensure_git_status_clean.rb")
allow(Fastlane::Actions).to receive(:sh).with("git status --porcelain --ignored='matching'").and_return("M fastlane/lib/fastlane/actions/ensure_git_status_clean.rb\n!! .DS_Store\n!! fastlane/.DS_Store")
allow(Fastlane::Actions).to receive(:sh).with("git diff").and_return("+ \"this is a new line\"")
end

context "with show_uncommitted_changes flag" do
context "true" do
it "outputs reach error message" do
it "outputs rich error message" do

This comment has been minimized.

Copy link
@getaaron

getaaron Mar 14, 2023

Collaborator

😍

expect(FastlaneCore::UI).to receive(:user_error!).with("Git repository is dirty! Please ensure the repo is in a clean state by committing/stashing/discarding all changes first.\nUncommitted changes:\nM fastlane/lib/fastlane/actions/ensure_git_status_clean.rb")
Fastlane::FastFile.new.parse("lane :test do
ensure_git_status_clean(show_uncommitted_changes: true)
Expand All @@ -46,16 +49,17 @@
end

context "without show_uncommitted_changes flag" do
it "outputs short error message with full diff" do
it "outputs short error message" do
expect(FastlaneCore::UI).to receive(:user_error!).with("Git repository is dirty! Please ensure the repo is in a clean state by committing/stashing/discarding all changes first.")
Fastlane::FastFile.new.parse("lane :test do
ensure_git_status_clean
end").runner.execute(:test)
end
end

context "with show_diff flag" do
context "true" do
it "outputs reach error message" do
it "outputs rich error message" do
expect(FastlaneCore::UI).to receive(:user_error!).with("Git repository is dirty! Please ensure the repo is in a clean state by committing/stashing/discarding all changes first.\nGit diff: \n+ \"this is a new line\"")
Fastlane::FastFile.new.parse("lane :test do
ensure_git_status_clean(show_diff: true)
Expand All @@ -73,11 +77,40 @@
end
end

context "without show_uncommitted_changes flag" do
it "outputs short error message" do
expect(FastlaneCore::UI).to receive(:user_error!).with("Git repository is dirty! Please ensure the repo is in a clean state by committing/stashing/discarding all changes first.")
context "with ignored mode" do
context "traditional" do
it "outputs error message with ignored files" do
expect(FastlaneCore::UI).to receive(:user_error!).with("Git repository is dirty! Please ensure the repo is in a clean state by committing/stashing/discarding all changes first.\nUncommitted changes:\nM fastlane/lib/fastlane/actions/ensure_git_status_clean.rb\n!! .DS_Store\n!! fastlane/")
Fastlane::FastFile.new.parse("lane :test do
ensure_git_status_clean(show_uncommitted_changes: true, ignored: 'traditional')
end").runner.execute(:test)
end
end

context "none" do
it "outputs error message without ignored files" do
expect(FastlaneCore::UI).to receive(:user_error!).with("Git repository is dirty! Please ensure the repo is in a clean state by committing/stashing/discarding all changes first.\nUncommitted changes:\nM fastlane/lib/fastlane/actions/ensure_git_status_clean.rb")
Fastlane::FastFile.new.parse("lane :test do
ensure_git_status_clean(show_uncommitted_changes: true, ignored: 'none')
end").runner.execute(:test)
end
end

context "matching" do
it "outputs error message with ignored files" do
expect(FastlaneCore::UI).to receive(:user_error!).with("Git repository is dirty! Please ensure the repo is in a clean state by committing/stashing/discarding all changes first.\nUncommitted changes:\nM fastlane/lib/fastlane/actions/ensure_git_status_clean.rb\n!! .DS_Store\n!! fastlane/.DS_Store")
Fastlane::FastFile.new.parse("lane :test do
ensure_git_status_clean(show_uncommitted_changes: true, ignored: 'matching')
end").runner.execute(:test)
end
end
end

context "without ignored mode" do
it "outputs error message without ignored files" do
expect(FastlaneCore::UI).to receive(:user_error!).with("Git repository is dirty! Please ensure the repo is in a clean state by committing/stashing/discarding all changes first.\nUncommitted changes:\nM fastlane/lib/fastlane/actions/ensure_git_status_clean.rb")
Fastlane::FastFile.new.parse("lane :test do
ensure_git_status_clean
ensure_git_status_clean(show_uncommitted_changes: true)
end").runner.execute(:test)
end
end
Expand Down

0 comments on commit e55138d

Please sign in to comment.