From d3e86f130ead1f61233e96f722f22428e612d147 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 1 Sep 2021 08:33:30 +0200 Subject: [PATCH 1/2] test: make the `check_connection` test use the local ssh setup With GitHub shutting down user/pass for Git access, let's take the opportunity to fix this failing test by using our own local setup instead of having to write down a password to an account. --- test/remote_test.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/remote_test.rb b/test/remote_test.rb index 692c3f09c..fac134dec 100644 --- a/test/remote_test.rb +++ b/test/remote_test.rb @@ -33,10 +33,11 @@ def test_remote_check_connection_push end def test_remote_check_connection_push_credentials - skip_if_unreachable - remote = @repo.remotes.create_anonymous('https://github.com/libgit2-push-test/libgit2-push-test.git') - credentials = Rugged::Credentials::UserPassword.new(username: "libgit2-push-test", password: "123qwe123") - assert remote.check_connection(:push, credentials: credentials) + skip unless Rugged.features.include?(:ssh) && ssh_creds? + + url = ENV['GITTEST_REMOTE_SSH_URL'] + remote = @repo.remotes.create_anonymous(url) + assert remote.check_connection(:push, credentials: ssh_key_credential) end def test_remote_check_connection_invalid From 61bb3f538fab12d84f569e5568700e3d6bb5a6f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 1 Sep 2021 08:33:44 +0200 Subject: [PATCH 2/2] test: don't hide online skipped tests behind definition-time conditionals Instead do show them as skipped to more accurately represent to the caller that we have tests which are being skipped. --- test/online/clone_test.rb | 93 +++++++++++++----------- test/online/fetch_test.rb | 149 ++++++++++++++++++++------------------ test/online/ls_test.rb | 64 ++++++++-------- test/online/push_test.rb | 87 +++++++++++----------- test/remote_test.rb | 2 +- test/test_helper.rb | 4 +- 6 files changed, 207 insertions(+), 192 deletions(-) diff --git a/test/online/clone_test.rb b/test/online/clone_test.rb index 51d876849..ad451c934 100644 --- a/test/online/clone_test.rb +++ b/test/online/clone_test.rb @@ -1,66 +1,71 @@ require 'test_helper' class OnlineCloneTest < Rugged::OnlineTestCase - if git_creds? - def test_clone_over_git - Dir.mktmpdir do |dir| - repo = Rugged::Repository.clone_at(ENV['GITTEST_REMOTE_GIT_URL'], dir) + def test_clone_over_git + skip unless git_creds? + Dir.mktmpdir do |dir| + repo = Rugged::Repository.clone_at(ENV['GITTEST_REMOTE_GIT_URL'], dir) - assert_instance_of Rugged::Repository, repo - end + assert_instance_of Rugged::Repository, repo end end - if Rugged.features.include?(:ssh) && ssh_creds? - def test_clone_over_ssh_with_credentials - Dir.mktmpdir do |dir| - repo = Rugged::Repository.clone_at(ENV['GITTEST_REMOTE_SSH_URL'], dir, { - credentials: ssh_key_credential - }) + def test_clone_over_ssh_with_credentials + skip unless Rugged.features.include?(:ssh) && ssh_creds? - assert_instance_of Rugged::Repository, repo - end + Dir.mktmpdir do |dir| + repo = Rugged::Repository.clone_at(ENV['GITTEST_REMOTE_SSH_URL'], dir, { + credentials: ssh_key_credential + }) + + assert_instance_of Rugged::Repository, repo end + end - def test_clone_over_ssh_with_credentials_from_agent - Dir.mktmpdir do |dir| - repo = Rugged::Repository.clone_at(ENV['GITTEST_REMOTE_SSH_URL'], dir, { - credentials: ssh_key_credential_from_agent - }) + def test_clone_over_ssh_with_credentials_from_agent + skip unless Rugged.features.include?(:ssh) && ssh_creds? - assert_instance_of Rugged::Repository, repo - end + Dir.mktmpdir do |dir| + repo = Rugged::Repository.clone_at(ENV['GITTEST_REMOTE_SSH_URL'], dir, { + credentials: ssh_key_credential_from_agent + }) + + assert_instance_of Rugged::Repository, repo end + end - def test_clone_over_ssh_with_credentials_callback - Dir.mktmpdir do |dir| - repo = Rugged::Repository.clone_at(ENV['GITTEST_REMOTE_SSH_URL'], dir, { - credentials: lambda { |url, username, allowed_types| - return ssh_key_credential - } - }) + def test_clone_over_ssh_with_credentials_callback + skip unless Rugged.features.include?(:ssh) && ssh_creds? - assert_instance_of Rugged::Repository, repo - end + Dir.mktmpdir do |dir| + repo = Rugged::Repository.clone_at(ENV['GITTEST_REMOTE_SSH_URL'], dir, { + credentials: lambda { |url, username, allowed_types| + return ssh_key_credential + } + }) + + assert_instance_of Rugged::Repository, repo end + end - def test_clone_callback_args_with_username - Dir.mktmpdir do |dir| - url, username, allowed_types = nil, nil, nil + def test_clone_callback_args_with_username + skip unless Rugged.features.include?(:ssh) && ssh_creds? - assert_raises Rugged::SshError do - Rugged::Repository.clone_at("git@github.com:libgit2/TestGitRepository", dir, { - credentials: lambda { |*args| - url, username, allowed_types = *args - return nil - } - }) - end + Dir.mktmpdir do |dir| + url, username, allowed_types = nil, nil, nil - assert_equal "git@github.com:libgit2/TestGitRepository", url - assert_equal "git", username - assert_equal [:ssh_key].sort, allowed_types.sort + assert_raises Rugged::SshError do + Rugged::Repository.clone_at("git@github.com:libgit2/TestGitRepository", dir, { + credentials: lambda { |*args| + url, username, allowed_types = *args + return nil + } + }) end + + assert_equal "git@github.com:libgit2/TestGitRepository", url + assert_equal "git", username + assert_equal [:ssh_key].sort, allowed_types.sort end end end diff --git a/test/online/fetch_test.rb b/test/online/fetch_test.rb index 101495433..1aca249ab 100644 --- a/test/online/fetch_test.rb +++ b/test/online/fetch_test.rb @@ -5,100 +5,109 @@ def setup @repo = FixtureRepo.empty end - if git_creds? - def test_fetch_over_git - @repo.remotes.create("origin", ENV['GITTEST_REMOTE_GIT_URL']) + def test_fetch_over_git + skip unless git_creds? + @repo.remotes.create("origin", ENV['GITTEST_REMOTE_GIT_URL']) - @repo.fetch("origin") - end + @repo.fetch("origin") end - if Rugged.features.include?(:https) - def test_fetch_over_https - @repo.remotes.create("origin", "https://github.com/libgit2/TestGitRepository.git") + def test_fetch_over_https + skip unless Rugged.features.include?(:https) - @repo.fetch("origin") + @repo.remotes.create("origin", "https://github.com/libgit2/TestGitRepository.git") - assert_equal [ - "refs/remotes/origin/first-merge", - "refs/remotes/origin/master", - "refs/remotes/origin/no-parent", - "refs/tags/annotated_tag", - "refs/tags/blob", - "refs/tags/commit_tree" - ], @repo.refs.map(&:name).sort - end + @repo.fetch("origin") + + assert_equal [ + "refs/remotes/origin/first-merge", + "refs/remotes/origin/master", + "refs/remotes/origin/no-parent", + "refs/tags/annotated_tag", + "refs/tags/blob", + "refs/tags/commit_tree" + ], @repo.refs.map(&:name).sort + end - def test_fetch_over_https_with_certificate_callback - @repo.remotes.create("origin", "https://github.com/libgit2/TestGitRepository.git") + def test_fetch_over_https_with_certificate_callback + skip unless Rugged.features.include?(:https) + + @repo.remotes.create("origin", "https://github.com/libgit2/TestGitRepository.git") + + args = {} + @repo.fetch( + "origin", + certificate_check: lambda { |valid, host| + args[:valid] = valid + args[:host] = host + true + } + ) + + assert_equal({ valid: true, host: "github.com" }, args) + end - args = {} + def test_fetch_over_https_with_certificate_callback_fail + skip unless Rugged.features.include?(:https) + + @repo.remotes.create("origin", "https://github.com/libgit2/TestGitRepository.git") + + exception = assert_raises Rugged::HTTPError do @repo.fetch( "origin", - certificate_check: lambda { |valid, host| - args[:valid] = valid - args[:host] = host - true - } + certificate_check: lambda { |valid, host| false } ) - - assert_equal({ valid: true, host: "github.com" }, args) end - def test_fetch_over_https_with_certificate_callback_fail - @repo.remotes.create("origin", "https://github.com/libgit2/TestGitRepository.git") + assert_equal "user rejected certificate for github.com", exception.message + end + + def test_fetch_over_https_with_certificate_callback_exception + skip unless Rugged.features.include?(:https) - exception = assert_raises Rugged::HTTPError do - @repo.fetch( - "origin", - certificate_check: lambda { |valid, host| false } - ) - end + @repo.remotes.create("origin", "https://github.com/libgit2/TestGitRepository.git") - assert_equal "user rejected certificate for github.com", exception.message + exception = assert_raises RuntimeError do + @repo.fetch( + "origin", + certificate_check: lambda { |valid, host| + raise "Exception from callback" + } + ) end - def test_fetch_over_https_with_certificate_callback_exception - @repo.remotes.create("origin", "https://github.com/libgit2/TestGitRepository.git") + assert_equal "Exception from callback", exception.message + end - exception = assert_raises RuntimeError do - @repo.fetch( - "origin", - certificate_check: lambda { |valid, host| - raise "Exception from callback" - } - ) - end + def test_fetch_over_ssh_with_credentials + skip unless Rugged.features.include?(:ssh) && ssh_creds? - assert_equal "Exception from callback", exception.message - end + @repo.remotes.create("origin", ENV['GITTEST_REMOTE_SSH_URL']) + + @repo.fetch("origin", { + credentials: ssh_key_credential + }) end - if Rugged.features.include?(:ssh) && ssh_creds? - def test_fetch_over_ssh_with_credentials - @repo.remotes.create("origin", ENV['GITTEST_REMOTE_SSH_URL']) + def test_fetch_over_ssh_with_credentials_from_agent + skip unless Rugged.features.include?(:ssh) && ssh_creds? - @repo.fetch("origin", { - credentials: ssh_key_credential - }) - end + @repo.remotes.create("origin", ENV['GITTEST_REMOTE_SSH_URL']) - def test_fetch_over_ssh_with_credentials_from_agent - @repo.remotes.create("origin", ENV['GITTEST_REMOTE_SSH_URL']) + @repo.fetch("origin", { + credentials: ssh_key_credential_from_agent + }) + end - @repo.fetch("origin", { - credentials: ssh_key_credential_from_agent - }) - end + def test_fetch_over_ssh_with_credentials_callback + skip unless Rugged.features.include?(:ssh) && ssh_creds? - def test_fetch_over_ssh_with_credentials_callback - @repo.remotes.create("origin", ENV['GITTEST_REMOTE_SSH_URL']) + @repo.remotes.create("origin", ENV['GITTEST_REMOTE_SSH_URL']) - @repo.fetch("origin", - credentials: lambda { |url, username, allowed_types| - return ssh_key_credential - } - ) - end + @repo.fetch("origin", + credentials: lambda { |url, username, allowed_types| + return ssh_key_credential + } + ) end end diff --git a/test/online/ls_test.rb b/test/online/ls_test.rb index dffd4cfcc..97d92cbf9 100644 --- a/test/online/ls_test.rb +++ b/test/online/ls_test.rb @@ -5,43 +5,43 @@ def setup @repo = FixtureRepo.from_libgit2("push_src") end - if Rugged.features.include?(:https) - def test_ls_over_https - remote = @repo.remotes.create("origin", "https://github.com/libgit2/TestGitRepository.git") - - assert_equal [ - { :local? => false, :oid => "49322bb17d3acc9146f98c97d078513228bbf3c0", :loid => nil, :name => "HEAD" }, - { :local? => false, :oid => "0966a434eb1a025db6b71485ab63a3bfbea520b6", :loid => nil, :name => "refs/heads/first-merge" }, - { :local? => false, :oid => "49322bb17d3acc9146f98c97d078513228bbf3c0", :loid => nil, :name => "refs/heads/master" }, - { :local? => false, :oid => "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1", :loid => nil, :name => "refs/heads/no-parent" }, - { :local? => false, :oid => "d96c4e80345534eccee5ac7b07fc7603b56124cb", :loid => nil, :name => "refs/tags/annotated_tag" }, - { :local? => false, :oid => "c070ad8c08840c8116da865b2d65593a6bb9cd2a", :loid => nil, :name => "refs/tags/annotated_tag^{}" }, - { :local? => false, :oid => "55a1a760df4b86a02094a904dfa511deb5655905", :loid => nil, :name => "refs/tags/blob" }, - { :local? => false, :oid => "8f50ba15d49353813cc6e20298002c0d17b0a9ee", :loid => nil, :name => "refs/tags/commit_tree" }, - { :local? => false, :oid => "6e0c7bdb9b4ed93212491ee778ca1c65047cab4e", :loid => nil, :name => "refs/tags/nearly-dangling"} - ], remote.ls.to_a - end + def test_ls_over_https + skip unless Rugged.features.include?(:https) + + remote = @repo.remotes.create("origin", "https://github.com/libgit2/TestGitRepository.git") + + assert_equal [ + { :local? => false, :oid => "49322bb17d3acc9146f98c97d078513228bbf3c0", :loid => nil, :name => "HEAD" }, + { :local? => false, :oid => "0966a434eb1a025db6b71485ab63a3bfbea520b6", :loid => nil, :name => "refs/heads/first-merge" }, + { :local? => false, :oid => "49322bb17d3acc9146f98c97d078513228bbf3c0", :loid => nil, :name => "refs/heads/master" }, + { :local? => false, :oid => "42e4e7c5e507e113ebbb7801b16b52cf867b7ce1", :loid => nil, :name => "refs/heads/no-parent" }, + { :local? => false, :oid => "d96c4e80345534eccee5ac7b07fc7603b56124cb", :loid => nil, :name => "refs/tags/annotated_tag" }, + { :local? => false, :oid => "c070ad8c08840c8116da865b2d65593a6bb9cd2a", :loid => nil, :name => "refs/tags/annotated_tag^{}" }, + { :local? => false, :oid => "55a1a760df4b86a02094a904dfa511deb5655905", :loid => nil, :name => "refs/tags/blob" }, + { :local? => false, :oid => "8f50ba15d49353813cc6e20298002c0d17b0a9ee", :loid => nil, :name => "refs/tags/commit_tree" }, + { :local? => false, :oid => "6e0c7bdb9b4ed93212491ee778ca1c65047cab4e", :loid => nil, :name => "refs/tags/nearly-dangling"} + ], remote.ls.to_a end - if git_creds? - def test_ls_over_git - remote = @repo.remotes.create("origin", ENV['GITTEST_REMOTE_GIT_URL']) - remote.push(["refs/heads/b1:refs/heads/b1"]) + def test_ls_over_git + skip unless git_creds? - assert_equal [ - { :local? => false, :oid => "a78705c3b2725f931d3ee05348d83cc26700f247", :loid => nil, :name => "refs/heads/b1" } - ], remote.ls.to_a - end + remote = @repo.remotes.create("origin", ENV['GITTEST_REMOTE_GIT_URL']) + remote.push(["refs/heads/b1:refs/heads/b1"]) + + assert_equal [ + { :local? => false, :oid => "a78705c3b2725f931d3ee05348d83cc26700f247", :loid => nil, :name => "refs/heads/b1" } + ], remote.ls.to_a end - if Rugged.features.include?(:ssh) && ssh_creds? - def test_ls_over_ssh_with_credentials - remote = @repo.remotes.create("origin", ENV['GITTEST_REMOTE_SSH_URL']) - remote.push(["refs/heads/b1:refs/heads/b1"], credentials: ssh_key_credential) + def test_ls_over_ssh_with_credentials + skip unless Rugged.features.include?(:ssh) && ssh_creds? + + remote = @repo.remotes.create("origin", ENV['GITTEST_REMOTE_SSH_URL']) + remote.push(["refs/heads/b1:refs/heads/b1"], credentials: ssh_key_credential) - assert_equal [ - { :local? => false, :oid => "a78705c3b2725f931d3ee05348d83cc26700f247", :loid => nil, :name => "refs/heads/b1" } - ], remote.ls(credentials: ssh_key_credential).to_a - end + assert_equal [ + { :local? => false, :oid => "a78705c3b2725f931d3ee05348d83cc26700f247", :loid => nil, :name => "refs/heads/b1" } + ], remote.ls(credentials: ssh_key_credential).to_a end end diff --git a/test/online/push_test.rb b/test/online/push_test.rb index 541beaaf9..f244f9b46 100644 --- a/test/online/push_test.rb +++ b/test/online/push_test.rb @@ -2,56 +2,57 @@ class OnlineGitPushTest < Rugged::OnlineTestCase def setup + skip unless git_creds? @repo = FixtureRepo.from_libgit2("push_src") @remote = @repo.remotes.create("test", ENV['GITTEST_REMOTE_GIT_URL']) @target_repo = Rugged::Repository.new(ENV['GITTEST_REMOTE_REPO_PATH']) end - if git_creds? - def test_push_branches - @remote.push([ - "refs/heads/b1:refs/heads/b1", - "refs/heads/b2:refs/heads/b2", - "refs/heads/b3:refs/heads/b3", - "refs/heads/b4:refs/heads/b4", - "refs/heads/b5:refs/heads/b5" - ]) - - assert_equal @repo.references["refs/heads/b1"].target_id, @target_repo.references["refs/heads/b1"].target_id - assert_equal @repo.references["refs/heads/b2"].target_id, @target_repo.references["refs/heads/b2"].target_id - assert_equal @repo.references["refs/heads/b3"].target_id, @target_repo.references["refs/heads/b3"].target_id - assert_equal @repo.references["refs/heads/b4"].target_id, @target_repo.references["refs/heads/b4"].target_id - assert_equal @repo.references["refs/heads/b5"].target_id, @target_repo.references["refs/heads/b5"].target_id - end + def test_push_branches + skip unless git_creds? + + @remote.push([ + "refs/heads/b1:refs/heads/b1", + "refs/heads/b2:refs/heads/b2", + "refs/heads/b3:refs/heads/b3", + "refs/heads/b4:refs/heads/b4", + "refs/heads/b5:refs/heads/b5" + ]) + + assert_equal @repo.references["refs/heads/b1"].target_id, @target_repo.references["refs/heads/b1"].target_id + assert_equal @repo.references["refs/heads/b2"].target_id, @target_repo.references["refs/heads/b2"].target_id + assert_equal @repo.references["refs/heads/b3"].target_id, @target_repo.references["refs/heads/b3"].target_id + assert_equal @repo.references["refs/heads/b4"].target_id, @target_repo.references["refs/heads/b4"].target_id + assert_equal @repo.references["refs/heads/b5"].target_id, @target_repo.references["refs/heads/b5"].target_id end end -if Rugged.features.include?(:ssh) - class OnlineSshPushTest < Rugged::OnlineTestCase - def setup - @repo = FixtureRepo.from_libgit2("push_src") - @remote = @repo.remotes.create("test", ENV['GITTEST_REMOTE_SSH_URL']) - @target_repo = Rugged::Repository.new(ENV['GITTEST_REMOTE_REPO_PATH']) - end - - if ssh_creds? - def test_push_branches - @remote.push([ - "refs/heads/b1:refs/heads/b1", - "refs/heads/b2:refs/heads/b2", - "refs/heads/b3:refs/heads/b3", - "refs/heads/b4:refs/heads/b4", - "refs/heads/b5:refs/heads/b5" - ], { - credentials: ssh_key_credential - }) - - assert_equal @repo.references["refs/heads/b1"].target_id, @target_repo.references["refs/heads/b1"].target_id - assert_equal @repo.references["refs/heads/b2"].target_id, @target_repo.references["refs/heads/b2"].target_id - assert_equal @repo.references["refs/heads/b3"].target_id, @target_repo.references["refs/heads/b3"].target_id - assert_equal @repo.references["refs/heads/b4"].target_id, @target_repo.references["refs/heads/b4"].target_id - assert_equal @repo.references["refs/heads/b5"].target_id, @target_repo.references["refs/heads/b5"].target_id - end - end +class OnlineSshPushTest < Rugged::OnlineTestCase + def setup + skip unless Rugged.features.include?(:ssh) + + @repo = FixtureRepo.from_libgit2("push_src") + @remote = @repo.remotes.create("test", ENV['GITTEST_REMOTE_SSH_URL']) + @target_repo = Rugged::Repository.new(ENV['GITTEST_REMOTE_REPO_PATH']) + end + + def test_push_branches + skip unless ssh_creds? + + @remote.push([ + "refs/heads/b1:refs/heads/b1", + "refs/heads/b2:refs/heads/b2", + "refs/heads/b3:refs/heads/b3", + "refs/heads/b4:refs/heads/b4", + "refs/heads/b5:refs/heads/b5" + ], { + credentials: ssh_key_credential + }) + + assert_equal @repo.references["refs/heads/b1"].target_id, @target_repo.references["refs/heads/b1"].target_id + assert_equal @repo.references["refs/heads/b2"].target_id, @target_repo.references["refs/heads/b2"].target_id + assert_equal @repo.references["refs/heads/b3"].target_id, @target_repo.references["refs/heads/b3"].target_id + assert_equal @repo.references["refs/heads/b4"].target_id, @target_repo.references["refs/heads/b4"].target_id + assert_equal @repo.references["refs/heads/b5"].target_id, @target_repo.references["refs/heads/b5"].target_id end end diff --git a/test/remote_test.rb b/test/remote_test.rb index fac134dec..52bb7fa34 100644 --- a/test/remote_test.rb +++ b/test/remote_test.rb @@ -1,7 +1,7 @@ require "test_helper" require 'net/http' -class RemoteNetworkTest < Rugged::TestCase +class RemoteNetworkTest < Rugged::OnlineTestCase def setup @repo = FixtureRepo.from_rugged("testrepo.git") end diff --git a/test/test_helper.rb b/test/test_helper.rb index e66a9cad5..cb7bf9f9e 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -158,11 +158,11 @@ def before_setup end end - def self.ssh_creds? + def ssh_creds? %w{URL USER KEY PUBKEY PASSPHRASE}.all? { |key| ENV["GITTEST_REMOTE_SSH_#{key}"] } end - def self.git_creds? + def git_creds? ENV['GITTEST_REMOTE_GIT_URL'] end