From af3f7345c219af0cbe5dd6fab2a79e7a9e6b3a9f Mon Sep 17 00:00:00 2001 From: Vitalii Budnik Date: Tue, 12 Dec 2023 18:47:59 +0300 Subject: [PATCH 1/4] [match] combine clone_branch_directly and shallow_clone --- match/lib/match/storage/git_storage.rb | 9 ++++-- match/spec/storage/git_storage_spec.rb | 40 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/match/lib/match/storage/git_storage.rb b/match/lib/match/storage/git_storage.rb index 71a82a6836e..99447303b78 100644 --- a/match/lib/match/storage/git_storage.rb +++ b/match/lib/match/storage/git_storage.rb @@ -92,9 +92,14 @@ def download command << " -c http.extraheader='Authorization: Bearer #{self.git_bearer_authorization}'" unless self.git_bearer_authorization.nil? if self.shallow_clone - command << " --depth 1 --no-single-branch" - elsif self.clone_branch_directly + command << " --depth 1" + end + + if self.clone_branch_directly command += " -b #{self.branch.shellescape} --single-branch" + elsif self.shallow_clone + # shallow clone all branches if not cloning branch directly + command += " --no-single-branch" end command = command_from_private_key(command) unless self.git_private_key.nil? diff --git a/match/spec/storage/git_storage_spec.rb b/match/spec/storage/git_storage_spec.rb index 59c9cc10ba6..5b30154d33d 100644 --- a/match/spec/storage/git_storage_spec.rb +++ b/match/spec/storage/git_storage_spec.rb @@ -43,6 +43,46 @@ expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) # because the README is being added when committing the changes end + it "directly clones a single branch wtih shallow clone" do + # GIVEN + path = Dir.mktmpdir # to have access to the actual path + allow(Dir).to receive(:mktmpdir).and_return(path) + + git_url = "https://github.com/fastlane/fastlane/tree/master/certificates" + git_branch = "master" + + storage = Match::Storage::GitStorage.new( + git_url: git_url, + branch: git_branch, + # Test case: + clone_branch_directly: true, + shallow_clone: true + ) + + # EXPECTATIONS + expected_commands = [ + "git clone #{git_url.shellescape} #{path.shellescape} --depth 1 -b #{git_branch} --single-branch", + "git --no-pager branch --list origin/#{git_branch} --no-color -r", + "git checkout --orphan #{git_branch}", + "git reset --hard" + ] + expected_commands.each do |command| + expect(FastlaneCore::CommandExecutor).to receive(:execute).once.with({ + command: command, + print_all: nil, + print_command: nil + }).and_return("") + end + + # WHEN + storage.download + + # THEN + # expected_commands above are executed + expect(File.directory?(storage.working_directory)).to eq(true) + expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) + end + it "clones the repo (not shallow)" do path = Dir.mktmpdir # to have access to the actual path expect(Dir).to receive(:mktmpdir).and_return(path) From 1170dd1455d5ac71cc45b549cb9a324f352202b2 Mon Sep 17 00:00:00 2001 From: Vitalii Budnik Date: Wed, 13 Dec 2023 11:27:35 +0300 Subject: [PATCH 2/4] fix: typo Co-authored-by: Roger Oba --- match/spec/storage/git_storage_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/match/spec/storage/git_storage_spec.rb b/match/spec/storage/git_storage_spec.rb index 5b30154d33d..4774c8fcaef 100644 --- a/match/spec/storage/git_storage_spec.rb +++ b/match/spec/storage/git_storage_spec.rb @@ -43,7 +43,7 @@ expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) # because the README is being added when committing the changes end - it "directly clones a single branch wtih shallow clone" do + it "directly clones a single branch with shallow clone" do # GIVEN path = Dir.mktmpdir # to have access to the actual path allow(Dir).to receive(:mktmpdir).and_return(path) From 85c8aceea91f83bbe3f42d5eb955522307fc526c Mon Sep 17 00:00:00 2001 From: Vitalii Budnik Date: Wed, 13 Dec 2023 12:53:43 +0300 Subject: [PATCH 3/4] chore: refactor git_storage tests --- match/spec/storage/git_storage_spec.rb | 429 ++++++++---------- match/spec/storage/git_storage_spec_helper.rb | 20 + 2 files changed, 210 insertions(+), 239 deletions(-) create mode 100644 match/spec/storage/git_storage_spec_helper.rb diff --git a/match/spec/storage/git_storage_spec.rb b/match/spec/storage/git_storage_spec.rb index 4774c8fcaef..e2dca6bf104 100644 --- a/match/spec/storage/git_storage_spec.rb +++ b/match/spec/storage/git_storage_spec.rb @@ -1,5 +1,16 @@ +require_relative 'git_storage_spec_helper' + describe Match do describe Match::Storage::GitStorage do + + let(:git_url) { "https://github.com/fastlane/fastlane/tree/master/certificates" } + let(:git_branch) { "test" } + + before(:each) do + @path = Dir.mktmpdir # to have access to the actual path + allow(Dir).to receive(:mktmpdir).and_return(@path) + end + describe "#generate_commit_message" do it "works" do storage = Match::Storage::GitStorage.new( @@ -12,286 +23,226 @@ end describe "#download" do - it "clones the repo" do - path = Dir.mktmpdir # to have access to the actual path - expect(Dir).to receive(:mktmpdir).and_return(path) - git_url = "https://github.com/fastlane/fastlane/tree/master/certificates" - git_branch = "master" - shallow_clone = true - - expected_commands = [ - "git clone #{git_url.shellescape} #{path.shellescape} --depth 1 --no-single-branch", - "git --no-pager branch --list origin/#{git_branch} --no-color -r", - "git checkout --orphan #{git_branch}", - "git reset --hard" - ] - expected_commands.each do |command| - expect(FastlaneCore::CommandExecutor).to receive(:execute).once.with({ - command: command, - print_all: nil, - print_command: nil - }).and_return("") - end - - storage = Match::Storage::GitStorage.new( - git_url: git_url, - shallow_clone: shallow_clone - ) - storage.download + describe "when no branch is specified" do + it("checkouts the master branch ") do + # Override default "test" branch name. + git_branch = "master" - expect(File.directory?(storage.working_directory)).to eq(true) - expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) # because the README is being added when committing the changes - end + storage = Match::Storage::GitStorage.new( + git_url: git_url + ) - it "directly clones a single branch with shallow clone" do - # GIVEN - path = Dir.mktmpdir # to have access to the actual path - allow(Dir).to receive(:mktmpdir).and_return(path) + clone_command = "git clone #{git_url.shellescape} #{@path.shellescape}" - git_url = "https://github.com/fastlane/fastlane/tree/master/certificates" - git_branch = "master" + expect_command_execution(clone_command) + expect_command_execution(branch_checkout_commands(git_branch)) - storage = Match::Storage::GitStorage.new( - git_url: git_url, - branch: git_branch, - # Test case: - clone_branch_directly: true, - shallow_clone: true - ) + storage.download - # EXPECTATIONS - expected_commands = [ - "git clone #{git_url.shellescape} #{path.shellescape} --depth 1 -b #{git_branch} --single-branch", - "git --no-pager branch --list origin/#{git_branch} --no-color -r", - "git checkout --orphan #{git_branch}", - "git reset --hard" - ] - expected_commands.each do |command| - expect(FastlaneCore::CommandExecutor).to receive(:execute).once.with({ - command: command, - print_all: nil, - print_command: nil - }).and_return("") + expect(File.directory?(storage.working_directory)).to eq(true) + expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) # because the README is being added when committing the changes end + end - # WHEN - storage.download + describe "when using shallow_clone" do + it("clones the repo with correct clone command") do + storage = Match::Storage::GitStorage.new( + git_url: git_url, + branch: git_branch, + # Test case: + shallow_clone: true + ) - # THEN - # expected_commands above are executed - expect(File.directory?(storage.working_directory)).to eq(true) - expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) - end + clone_command = "git clone #{git_url.shellescape} #{@path.shellescape} --depth 1 --no-single-branch" - it "clones the repo (not shallow)" do - path = Dir.mktmpdir # to have access to the actual path - expect(Dir).to receive(:mktmpdir).and_return(path) - git_url = "https://github.com/fastlane/fastlane/tree/master/certificates" - git_branch = "master" - shallow_clone = false - - expected_commands = [ - "git clone #{git_url.shellescape} #{path.shellescape}", - "git --no-pager branch --list origin/#{git_branch} --no-color -r", - "git checkout --orphan #{git_branch}", - "git reset --hard" - ] - expected_commands.each do |command| - expect(FastlaneCore::CommandExecutor).to receive(:execute).once.with({ - command: command, - print_all: nil, - print_command: nil - }).and_return("") - end + expect_command_execution(clone_command) + expect_command_execution(branch_checkout_commands(git_branch)) - storage = Match::Storage::GitStorage.new( - git_url: git_url, - shallow_clone: shallow_clone - ) - storage.download + storage.download - expect(File.directory?(storage.working_directory)).to eq(true) - expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) # because the README is being added when committing the changes + expect(File.directory?(storage.working_directory)).to eq(true) + expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) # because the README is being added when committing the changes + end end - it "checks out a branch" do - path = Dir.mktmpdir # to have access to the actual path - expect(Dir).to receive(:mktmpdir).and_return(path) - git_url = "https://github.com/fastlane/fastlane/tree/master/certificates" - git_branch = "test" - shallow_clone = false - - expected_commands = [ - "git clone #{git_url.shellescape} #{path.shellescape}", - "git --no-pager branch --list origin/#{git_branch} --no-color -r", - "git checkout --orphan #{git_branch}", - "git reset --hard" - ] - expected_commands.each do |command| - expect(FastlaneCore::CommandExecutor).to receive(:execute).once.with({ - command: command, - print_all: nil, - print_command: nil - }).and_return("") - end + describe "when using shallow_clone and clone_branch_directly" do + it("clones the repo with correct clone command") do + storage = Match::Storage::GitStorage.new( + git_url: git_url, + branch: git_branch, + # Test case: + clone_branch_directly: true, + shallow_clone: true + ) - storage = Match::Storage::GitStorage.new( - git_url: git_url, - shallow_clone: shallow_clone, - branch: git_branch - ) - storage.download + clone_command = "git clone #{git_url.shellescape} #{@path.shellescape} --depth 1 -b #{git_branch} --single-branch" - expect(File.directory?(storage.working_directory)).to eq(true) - expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) # because the README is being added when committing the changes + expect_command_execution(clone_command) + expect_command_execution(branch_checkout_commands(git_branch)) + + storage.download + + expect(File.directory?(storage.working_directory)).to eq(true) + expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) # because the README is being added when committing the changes + end end - end - describe "#save_changes" do - it "skips README file generation if so requested" do - path = Dir.mktmpdir # to have access to the actual path - expect(Dir).to receive(:mktmpdir).and_return(path) - git_url = "https://github.com/fastlane/fastlane/tree/master/certificates" - git_branch = "master" - random_file = "random_file" + describe "when using clone_branch_directly" do + it("clones the repo with correct clone command") do + storage = Match::Storage::GitStorage.new( + git_url: git_url, + branch: git_branch, + # Test case: + clone_branch_directly: true + ) - storage = Match::Storage::GitStorage.new( - type: "appstore", - platform: "ios", - git_url: git_url, - shallow_clone: false, - skip_docs: true - ) + clone_command = "git clone #{git_url.shellescape} #{@path.shellescape} -b #{git_branch} --single-branch" + + expect_command_execution(clone_command) + expect_command_execution(branch_checkout_commands(git_branch)) + + storage.download - expected_commands = [ - "git clone #{git_url.shellescape} #{path.shellescape}", - "git --no-pager branch --list origin/#{git_branch} --no-color -r", - "git checkout --orphan #{git_branch}", - "git reset --hard", - "git add #{random_file}", - "git add match_version.txt", - "git commit -m " + '[fastlane] Updated appstore and platform ios'.shellescape, - "git push origin #{git_branch}" - ] - - expected_commands.each do |command| - expect(FastlaneCore::CommandExecutor).to receive(:execute).once.with({ - command: command, - print_all: nil, - print_command: nil - }).and_return("") + expect(File.directory?(storage.working_directory)).to eq(true) + expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) # because the README is being added when committing the changes end + end + + describe "when not using shallow_clone and clone_branch_directly" do + it("clones the repo with correct clone command") do + storage = Match::Storage::GitStorage.new( + git_url: git_url, + branch: git_branch, + # Test case: + clone_branch_directly: false, + shallow_clone: false + ) + + clone_command = "git clone #{git_url.shellescape} #{@path.shellescape}" - expect(storage).to receive(:clear_changes).and_return(nil) # so we can inspect the folder + expect_command_execution(clone_command) + expect_command_execution(branch_checkout_commands(git_branch)) - storage.download - storage.save_changes!(files_to_commit: [random_file]) + storage.download - expect(File.directory?(storage.working_directory)).to eq(true) - expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) - expect(File.read(File.join(storage.working_directory, 'match_version.txt'))).to eq(Fastlane::VERSION) + expect(File.directory?(storage.working_directory)).to eq(true) + expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) # because the README is being added when committing the changes + end end end - describe "authentication" do - it "wraps the git command in ssh-agent shell when using a private key file" do - path = Dir.mktmpdir # to have access to the actual path - expect(Dir).to receive(:mktmpdir).and_return(path) - expect(File).to receive(:file?).twice.and_return(true) - git_url = "https://github.com/fastlane/fastlane/tree/master/certificates" - shallow_clone = false - private_key = "#{path}/fastlane.match.id_dsa" - clone_command = "ssh-agent bash -c 'ssh-add #{private_key.shellescape}; git clone #{git_url.shellescape} #{path.shellescape}'" - - git_branch = "master" - - expected_commands = [ - clone_command, - "git --no-pager branch --list origin/#{git_branch} --no-color -r", - "git checkout --orphan #{git_branch}", - "git reset --hard" - ] - - expected_commands.each do |command| - expect(FastlaneCore::CommandExecutor).to receive(:execute).once.with({ - command: command, - print_all: nil, - print_command: nil - }).and_return("") + describe "#save_changes" do + describe "when skip_docs is true" do + it "skips README file generation" do + random_file_to_commit = "random_file_to_commit" + + storage = Match::Storage::GitStorage.new( + type: "appstore", + platform: "ios", + git_url: git_url, + branch: git_branch, + skip_docs: true + ) + + checkout_command = "git clone #{git_url.shellescape} #{@path.shellescape}" + expect_command_execution(checkout_command) + expect_command_execution(branch_checkout_commands(git_branch)) + + expected_commit_commands = [ + # Stage new file for commit. + "git add #{random_file_to_commit}", + # Stage match_version.txt for commit. + "git add match_version.txt", + # Commit changes. + "git commit -m " + '[fastlane] Updated appstore and platform ios'.shellescape, + # Push changes to the remote origin. + "git push origin #{git_branch}" + ] + expect_command_execution(expected_commit_commands) + + expect(storage).to receive(:clear_changes).and_return(nil) # so we can inspect the folder + + storage.download + storage.save_changes!(files_to_commit: [random_file_to_commit]) + + expect(File.directory?(storage.working_directory)).to eq(true) + expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) + expect(File.read(File.join(storage.working_directory, 'match_version.txt'))).to eq(Fastlane::VERSION) end - - storage = Match::Storage::GitStorage.new( - git_url: git_url, - shallow_clone: shallow_clone, - git_private_key: private_key - ) - storage.download - - expect(File.directory?(storage.working_directory)).to eq(true) - expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) # because the README is being added when committing the changes end + end - it "wraps the git command in ssh-agent shell when using a raw private key" do - path = Dir.mktmpdir # to have access to the actual path - expect(Dir).to receive(:mktmpdir).and_return(path) - expect(File).to receive(:file?).twice.and_return(false) - git_url = "https://github.com/fastlane/fastlane/tree/master/certificates" - shallow_clone = false - private_key = "-----BEGIN PRIVATE KEY-----\n-----END PRIVATE KEY-----\n" - clone_command = "ssh-agent bash -c 'ssh-add - <<< \"#{private_key}\"; git clone #{git_url.shellescape} #{path.shellescape}'" - - git_branch = "master" - - expected_commands = [ - clone_command, - "git --no-pager branch --list origin/#{git_branch} --no-color -r", - "git checkout --orphan #{git_branch}", - "git reset --hard" - ] - - expected_commands.each do |command| - expect(FastlaneCore::CommandExecutor).to receive(:execute).once.with({ - command: command, - print_all: nil, - print_command: nil - }).and_return("") + describe "#authentication" do + describe "when using a private key file" do + it "wraps the git command in ssh-agent shell" do + expect(File).to receive(:file?).twice.and_return(true) + private_key = "#{@path}/fastlane.match.id_dsa" + clone_command = "ssh-agent bash -c 'ssh-add #{private_key.shellescape}; git clone #{git_url.shellescape} #{@path.shellescape}'" + + expect_command_execution(clone_command) + expect_command_execution(branch_checkout_commands(git_branch)) + + storage = Match::Storage::GitStorage.new( + git_url: git_url, + branch: git_branch, + git_private_key: private_key + ) + storage.download + + expect(File.directory?(storage.working_directory)).to eq(true) + expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) # because the README is being added when committing the changes end + end - storage = Match::Storage::GitStorage.new( - git_url: git_url, - shallow_clone: shallow_clone, - git_private_key: private_key - ) - storage.download + describe "when using a raw private key" do + it "wraps the git command in ssh-agent shell" do + expect(File).to receive(:file?).twice.and_return(false) + private_key = "-----BEGIN PRIVATE KEY-----\n-----END PRIVATE KEY-----\n" + clone_command = "ssh-agent bash -c 'ssh-add - <<< \"#{private_key}\"; git clone #{git_url.shellescape} #{@path.shellescape}'" - expect(File.directory?(storage.working_directory)).to eq(true) - expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) # because the README is being added when committing the changes + expect_command_execution(clone_command) + expect_command_execution(branch_checkout_commands(git_branch)) + + storage = Match::Storage::GitStorage.new( + git_url: git_url, + branch: git_branch, + git_private_key: private_key + ) + storage.download + + expect(File.directory?(storage.working_directory)).to eq(true) + expect(File.exist?(File.join(storage.working_directory, 'README.md'))).to eq(false) # because the README is being added when committing the changes + end end end - describe "ssh-agent utilities" do - it "wraps any given command in ssh-agent shell when using a raw private key" do - given_command = "any random command" - private_key = "-----BEGIN PRIVATE KEY-----\n-----END PRIVATE KEY-----\n" + describe "#ssh-agent utilities" do + describe "when using a raw private key" do + it "wraps any given command in ssh-agent shell" do + given_command = "any random command" + private_key = "-----BEGIN PRIVATE KEY-----\n-----END PRIVATE KEY-----\n" - storage = Match::Storage::GitStorage.new( - git_private_key: private_key - ) + storage = Match::Storage::GitStorage.new( + git_private_key: private_key + ) - expected_command = "ssh-agent bash -c 'ssh-add - <<< \"#{private_key}\"; #{given_command}'" - expect(storage.command_from_private_key(given_command)).to eq(expected_command) + expected_command = "ssh-agent bash -c 'ssh-add - <<< \"#{private_key}\"; #{given_command}'" + expect(storage.command_from_private_key(given_command)).to eq(expected_command) + end end - it "wraps any given command in ssh-agent shell when using a private key file" do - given_command = "any random command" - private_key = "#{Dir.mktmpdir}/fastlane.match.id_dsa" + describe "when using a private key file" do + it "wraps any given command in ssh-agent shell" do + given_command = "any random command" + private_key = "#{Dir.mktmpdir}/fastlane.match.id_dsa" - storage = Match::Storage::GitStorage.new( - git_private_key: private_key - ) + storage = Match::Storage::GitStorage.new( + git_private_key: private_key + ) - expected_command = "ssh-agent bash -c 'ssh-add - <<< \"#{File.expand_path(private_key).shellescape}\"; #{given_command}'" - expect(storage.command_from_private_key(given_command)).to eq(expected_command) + expected_command = "ssh-agent bash -c 'ssh-add - <<< \"#{File.expand_path(private_key).shellescape}\"; #{given_command}'" + expect(storage.command_from_private_key(given_command)).to eq(expected_command) + end end end end diff --git a/match/spec/storage/git_storage_spec_helper.rb b/match/spec/storage/git_storage_spec_helper.rb new file mode 100644 index 00000000000..93a4f4983eb --- /dev/null +++ b/match/spec/storage/git_storage_spec_helper.rb @@ -0,0 +1,20 @@ +def branch_checkout_commands(git_branch) + [ + # Check if branch exists. + "git --no-pager branch --list origin/#{git_branch} --no-color -r", + # Checkout branch. + "git checkout --orphan #{git_branch}", + # Reset all changes in the working branch copy. + "git reset --hard" + ] +end + +def expect_command_execution(commands) + [commands].flatten.each do |command| + expect(FastlaneCore::CommandExecutor).to receive(:execute).once.with({ + command: command, + print_all: nil, + print_command: nil + }).and_return("") + end +end From d01e66e7c2d3cd5969294d22d7cdc000f9c95dd6 Mon Sep 17 00:00:00 2001 From: Roger Oba Date: Wed, 13 Dec 2023 10:46:20 -0300 Subject: [PATCH 4/4] Remove parenthesis around "it" statements. --- match/spec/storage/git_storage_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/match/spec/storage/git_storage_spec.rb b/match/spec/storage/git_storage_spec.rb index e2dca6bf104..5a2e33271e6 100644 --- a/match/spec/storage/git_storage_spec.rb +++ b/match/spec/storage/git_storage_spec.rb @@ -24,7 +24,7 @@ describe "#download" do describe "when no branch is specified" do - it("checkouts the master branch ") do + it "checkouts the master branch" do # Override default "test" branch name. git_branch = "master" @@ -45,7 +45,7 @@ end describe "when using shallow_clone" do - it("clones the repo with correct clone command") do + it "clones the repo with correct clone command" do storage = Match::Storage::GitStorage.new( git_url: git_url, branch: git_branch, @@ -66,7 +66,7 @@ end describe "when using shallow_clone and clone_branch_directly" do - it("clones the repo with correct clone command") do + it "clones the repo with correct clone command" do storage = Match::Storage::GitStorage.new( git_url: git_url, branch: git_branch, @@ -88,7 +88,7 @@ end describe "when using clone_branch_directly" do - it("clones the repo with correct clone command") do + it "clones the repo with correct clone command" do storage = Match::Storage::GitStorage.new( git_url: git_url, branch: git_branch, @@ -109,7 +109,7 @@ end describe "when not using shallow_clone and clone_branch_directly" do - it("clones the repo with correct clone command") do + it "clones the repo with correct clone command" do storage = Match::Storage::GitStorage.new( git_url: git_url, branch: git_branch,