From 8c00dbb503b36a1f9c7e69dcaf3ca202879b55ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Wed, 10 Oct 2018 17:58:18 -0300 Subject: [PATCH 1/9] Add unit tests for Blob from gitlab-ce + setup code --- Makefile | 8 +- ruby/lib/gitlab/git/blob.rb | 19 +- ruby/spec/lib/gitlab/git/blob_spec.rb | 296 ++++++++++++++++++++++++ ruby/spec/spec_helper.rb | 2 + ruby/spec/support/generate-seed-repo-rb | 164 +++++++++++++ ruby/spec/support/helpers/seed_repo.rb | 154 ++++++++++++ ruby/spec/test_repo_helper.rb | 23 +- 7 files changed, 641 insertions(+), 25 deletions(-) create mode 100644 ruby/spec/lib/gitlab/git/blob_spec.rb create mode 100755 ruby/spec/support/generate-seed-repo-rb create mode 100644 ruby/spec/support/helpers/seed_repo.rb diff --git a/Makefile b/Makefile index 4afaee5bc65..681f89a14f1 100644 --- a/Makefile +++ b/Makefile @@ -7,6 +7,7 @@ BIN_BUILD_DIR := $(TARGET_DIR)/bin PKG_BUILD_DIR := $(TARGET_DIR)/src/$(PKG) export TEST_REPO_STORAGE_PATH := $(BUILD_DIR)/internal/testhelper/testdata/data TEST_REPO := $(TEST_REPO_STORAGE_PATH)/gitlab-test.git +GIT_TEST_REPO := $(TEST_REPO_STORAGE_PATH)/gitlab-git-test.git INSTALL_DEST_DIR := $(DESTDIR)$(PREFIX)/bin/ COVERAGE_DIR := $(TARGET_DIR)/cover ASSEMBLY_ROOT := $(TARGET_DIR)/assembly @@ -119,8 +120,11 @@ $(TEST_REPO): # Git notes aren't fetched by default with git clone git -C $@ fetch origin refs/notes/*:refs/notes/* +$(GIT_TEST_REPO): + git clone --bare https://gitlab.com/gitlab-org/gitlab-git-test.git $@ + .PHONY: prepare-tests -prepare-tests: $(TARGET_SETUP) $(TEST_REPO) .ruby-bundle +prepare-tests: $(TARGET_SETUP) $(TEST_REPO) $(GIT_TEST_REPO) .ruby-bundle .PHONY: test test: test-go rspec @@ -177,7 +181,7 @@ codeclimate-report: .PHONY: clean clean: - rm -rf $(TARGET_DIR) $(TEST_REPO) $(TEST_REPO_STORAGE_PATH) ./internal/service/ssh/gitaly-*-pack .ruby-bundle + rm -rf $(TARGET_DIR) $(TEST_REPO_STORAGE_PATH) ./ruby/tmp/repositories ./internal/service/ssh/gitaly-*-pack .ruby-bundle .PHONY: cover cover: prepare-tests $(GOCOVMERGE) diff --git a/ruby/lib/gitlab/git/blob.rb b/ruby/lib/gitlab/git/blob.rb index 5c164fc357b..7e0b8376334 100644 --- a/ruby/lib/gitlab/git/blob.rb +++ b/ruby/lib/gitlab/git/blob.rb @@ -6,8 +6,7 @@ module Gitlab # This number is the maximum amount of data that we want to display to # the user. We load as much as we can for encoding detection - # (Linguist) and LFS pointer parsing. All other cases where we need full - # blob data should use load_all_data!. + # (Linguist) and LFS pointer parsing. MAX_DATA_DISPLAY_SIZE = 10.megabytes # These limits are used as a heuristic to ignore files which can't be LFS @@ -159,22 +158,6 @@ module Gitlab encode! @data end - # Load all blob data (not just the first MAX_DATA_DISPLAY_SIZE bytes) into - # memory as a Ruby string. - def load_all_data!(repository) - return if @data == '' # don't mess with submodule blobs - - # Even if we return early, recalculate wether this blob is binary in - # case a blob was initialized as text but the full data isn't - @binary = nil - - return if @loaded_all_data - - @data = repository.gitaly_blob_client.get_blob(oid: id, limit: -1).data - @loaded_all_data = true - @loaded_size = @data.bytesize - end - def name encode! @name end diff --git a/ruby/spec/lib/gitlab/git/blob_spec.rb b/ruby/spec/lib/gitlab/git/blob_spec.rb new file mode 100644 index 00000000000..41bb41a1893 --- /dev/null +++ b/ruby/spec/lib/gitlab/git/blob_spec.rb @@ -0,0 +1,296 @@ +require "spec_helper" + +describe Gitlab::Git::Blob, :seed_helper do + include TestRepo + + let(:repository) { gitlab_git_from_gitaly(git_test_repo_read_only) } + let(:rugged) do + Rugged::Repository.new(GIT_TEST_REPO_PATH) + end + + describe 'initialize' do + let(:blob) { Gitlab::Git::Blob.new(name: 'test') } + + it 'handles nil data' do + expect(blob.name).to eq('test') + expect(blob.size).to eq(nil) + expect(blob.loaded_size).to eq(nil) + end + end + + describe '.find' do + context 'nil path' do + let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, nil) } + + it { expect(blob).to eq(nil) } + end + + context 'utf-8 branch' do + let(:blob) { Gitlab::Git::Blob.find(repository, 'Ääh-test-utf-8', "files/ruby/popen.rb")} + + it { expect(blob.id).to eq(SeedRepo::RubyBlob::ID) } + end + + context 'blank path' do + let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, '') } + + it { expect(blob).to eq(nil) } + end + + context 'file in subdir' do + let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "files/ruby/popen.rb") } + + it { expect(blob.id).to eq(SeedRepo::RubyBlob::ID) } + it { expect(blob.name).to eq(SeedRepo::RubyBlob::NAME) } + it { expect(blob.path).to eq("files/ruby/popen.rb") } + it { expect(blob.commit_id).to eq(SeedRepo::Commit::ID) } + it { expect(blob.data[0..10]).to eq(SeedRepo::RubyBlob::CONTENT[0..10]) } + it { expect(blob.size).to eq(669) } + it { expect(blob.mode).to eq("100644") } + end + + context 'file in root' do + let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, ".gitignore") } + + it { expect(blob.id).to eq("dfaa3f97ca337e20154a98ac9d0be76ddd1fcc82") } + it { expect(blob.name).to eq(".gitignore") } + it { expect(blob.path).to eq(".gitignore") } + it { expect(blob.commit_id).to eq(SeedRepo::Commit::ID) } + it { expect(blob.data[0..10]).to eq("*.rbc\n*.sas") } + it { expect(blob.size).to eq(241) } + it { expect(blob.mode).to eq("100644") } + it { expect(blob).not_to be_binary } + end + + context 'file in root with leading slash' do + let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "/.gitignore") } + + it { expect(blob.id).to eq("dfaa3f97ca337e20154a98ac9d0be76ddd1fcc82") } + it { expect(blob.name).to eq(".gitignore") } + it { expect(blob.path).to eq(".gitignore") } + it { expect(blob.commit_id).to eq(SeedRepo::Commit::ID) } + it { expect(blob.data[0..10]).to eq("*.rbc\n*.sas") } + it { expect(blob.size).to eq(241) } + it { expect(blob.mode).to eq("100644") } + end + + context 'non-exist file' do + let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "missing.rb") } + + it { expect(blob).to be_nil } + end + + context 'six submodule' do + let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, 'six') } + + it { expect(blob.id).to eq('409f37c4f05865e4fb208c771485f211a22c4c2d') } + it { expect(blob.data).to eq('') } + + it 'does not mark the blob as binary' do + expect(blob).not_to be_binary + end + end + + context 'large file' do + let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, 'files/images/6049019_460s.jpg') } + let(:blob_size) { 111803 } + let(:stub_limit) { 1000 } + + before do + stub_const('Gitlab::Git::Blob::MAX_DATA_DISPLAY_SIZE', stub_limit) + end + + it { expect(blob.size).to eq(blob_size) } + it { expect(blob.data.length).to eq(stub_limit) } + + it 'check that this test is sane' do + # It only makes sense to test limiting if the blob is larger than the limit. + expect(blob.size).to be > Gitlab::Git::Blob::MAX_DATA_DISPLAY_SIZE + end + + it 'marks the blob as binary' do + expect(Gitlab::Git::Blob).to receive(:new) + .with(hash_including(binary: true)) + .and_call_original + + expect(blob).to be_binary + end + end + end + + describe 'encoding' do + context 'file with russian text' do + let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "encoding/russian.rb") } + + it { expect(blob.name).to eq("russian.rb") } + it { expect(blob.data.lines.first).to eq("Хороший файл") } + it { expect(blob.size).to eq(23) } + it { expect(blob.truncated?).to be_falsey } + # Run it twice since data is encoded after the first run + it { expect(blob.truncated?).to be_falsey } + it { expect(blob.mode).to eq("100755") } + end + + context 'file with Chinese text' do + let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "encoding/テスト.txt") } + + it { expect(blob.name).to eq("テスト.txt") } + it { expect(blob.data).to include("これはテスト") } + it { expect(blob.size).to eq(340) } + it { expect(blob.mode).to eq("100755") } + it { expect(blob.truncated?).to be_falsey } + end + + context 'file with ISO-8859 text' do + let(:blob) { Gitlab::Git::Blob.find(repository, SeedRepo::LastCommit::ID, "encoding/iso8859.txt") } + + it { expect(blob.name).to eq("iso8859.txt") } + it { expect(blob.loaded_size).to eq(4) } + it { expect(blob.size).to eq(4) } + it { expect(blob.mode).to eq("100644") } + it { expect(blob.truncated?).to be_falsey } + end + end + + describe 'mode' do + context 'file regular' do + let(:blob) do + Gitlab::Git::Blob.find( + repository, + 'fa1b1e6c004a68b7d8763b86455da9e6b23e36d6', + 'files/ruby/regex.rb' + ) + end + + it { expect(blob.name).to eq('regex.rb') } + it { expect(blob.path).to eq('files/ruby/regex.rb') } + it { expect(blob.size).to eq(1200) } + it { expect(blob.mode).to eq("100644") } + end + + context 'file binary' do + let(:blob) do + Gitlab::Git::Blob.find( + repository, + 'fa1b1e6c004a68b7d8763b86455da9e6b23e36d6', + 'files/executables/ls' + ) + end + + it { expect(blob.name).to eq('ls') } + it { expect(blob.path).to eq('files/executables/ls') } + it { expect(blob.size).to eq(110080) } + it { expect(blob.mode).to eq("100755") } + end + + context 'file symlink to regular' do + let(:blob) do + Gitlab::Git::Blob.find( + repository, + 'fa1b1e6c004a68b7d8763b86455da9e6b23e36d6', + 'files/links/ruby-style-guide.md' + ) + end + + it { expect(blob.name).to eq('ruby-style-guide.md') } + it { expect(blob.path).to eq('files/links/ruby-style-guide.md') } + it { expect(blob.size).to eq(31) } + it { expect(blob.mode).to eq("120000") } + end + + context 'file symlink to binary' do + let(:blob) do + Gitlab::Git::Blob.find( + repository, + 'fa1b1e6c004a68b7d8763b86455da9e6b23e36d6', + 'files/links/touch' + ) + end + + it { expect(blob.name).to eq('touch') } + it { expect(blob.path).to eq('files/links/touch') } + it { expect(blob.size).to eq(20) } + it { expect(blob.mode).to eq("120000") } + end + end + + describe 'lfs_pointers' do + context 'file a valid lfs pointer' do + let(:blob) do + Gitlab::Git::Blob.find( + repository, + '33bcff41c232a11727ac6d660bd4b0c2ba86d63d', + 'files/lfs/image.jpg' + ) + end + + it { expect(blob.lfs_pointer?).to eq(true) } + it { expect(blob.lfs_oid).to eq("4206f951d2691c78aac4c0ce9f2b23580b2c92cdcc4336e1028742c0274938e0") } + it { expect(blob.lfs_size).to eq(19548) } + it { expect(blob.id).to eq("f4d76af13003d1106be7ac8c5a2a3d37ddf32c2a") } + it { expect(blob.name).to eq("image.jpg") } + it { expect(blob.path).to eq("files/lfs/image.jpg") } + it { expect(blob.size).to eq(130) } + it { expect(blob.mode).to eq("100644") } + end + + describe 'file an invalid lfs pointer' do + context 'with correct version header but incorrect size and oid' do + let(:blob) do + Gitlab::Git::Blob.find( + repository, + '33bcff41c232a11727ac6d660bd4b0c2ba86d63d', + 'files/lfs/archive-invalid.tar' + ) + end + + it { expect(blob.lfs_pointer?).to eq(false) } + it { expect(blob.lfs_oid).to eq(nil) } + it { expect(blob.lfs_size).to eq(nil) } + it { expect(blob.id).to eq("f8a898db217a5a85ed8b3d25b34c1df1d1094c46") } + it { expect(blob.name).to eq("archive-invalid.tar") } + it { expect(blob.path).to eq("files/lfs/archive-invalid.tar") } + it { expect(blob.size).to eq(43) } + it { expect(blob.mode).to eq("100644") } + end + + context 'with correct version header and size but incorrect size and oid' do + let(:blob) do + Gitlab::Git::Blob.find( + repository, + '33bcff41c232a11727ac6d660bd4b0c2ba86d63d', + 'files/lfs/picture-invalid.png' + ) + end + + it { expect(blob.lfs_pointer?).to eq(false) } + it { expect(blob.lfs_oid).to eq(nil) } + it { expect(blob.lfs_size).to eq(1575078) } + it { expect(blob.id).to eq("5ae35296e1f95c1ef9feda1241477ed29a448572") } + it { expect(blob.name).to eq("picture-invalid.png") } + it { expect(blob.path).to eq("files/lfs/picture-invalid.png") } + it { expect(blob.size).to eq(57) } + it { expect(blob.mode).to eq("100644") } + end + + context 'with correct version header and size but invalid size and oid' do + let(:blob) do + Gitlab::Git::Blob.find( + repository, + '33bcff41c232a11727ac6d660bd4b0c2ba86d63d', + 'files/lfs/file-invalid.zip' + ) + end + + it { expect(blob.lfs_pointer?).to eq(false) } + it { expect(blob.lfs_oid).to eq(nil) } + it { expect(blob.lfs_size).to eq(nil) } + it { expect(blob.id).to eq("d831981bd876732b85a1bcc6cc01210c9f36248f") } + it { expect(blob.name).to eq("file-invalid.zip") } + it { expect(blob.path).to eq("files/lfs/file-invalid.zip") } + it { expect(blob.size).to eq(60) } + it { expect(blob.mode).to eq("100644") } + end + end + end +end diff --git a/ruby/spec/spec_helper.rb b/ruby/spec/spec_helper.rb index 75652b01a36..54aa9038db2 100644 --- a/ruby/spec/spec_helper.rb +++ b/ruby/spec/spec_helper.rb @@ -3,4 +3,6 @@ require_relative '../lib/gitlab/git.rb' require_relative 'support/sentry.rb' require 'test_repo_helper' +Dir[File.join(__dir__, 'support/helpers/*.rb')].each { |f| require f } + ENV['GITALY_RUBY_GIT_BIN_PATH'] ||= 'git' diff --git a/ruby/spec/support/generate-seed-repo-rb b/ruby/spec/support/generate-seed-repo-rb new file mode 100755 index 00000000000..25fbd364c51 --- /dev/null +++ b/ruby/spec/support/generate-seed-repo-rb @@ -0,0 +1,164 @@ +#!/usr/bin/env ruby +# +# # generate-seed-repo-rb +# +# This script generates the seed_repo.rb file used by lib/gitlab/git +# tests. The seed_repo.rb file needs to be updated anytime there is a +# Git push to https://gitlab.com/gitlab-org/gitlab-git-test. +# +# Usage: +# +# ./spec/support/generate-seed-repo-rb > spec/support/helpers/seed_repo.rb +# +# + +require 'erb' +require 'tempfile' + +SOURCE = File.expand_path('../../../internal/testhelper/testdata/data/gitlab-git-test.git', __dir__).freeze +SCRIPT_NAME = 'generate-seed-repo-rb'.freeze +REPO_NAME = 'gitlab-git-test.git'.freeze + +def main + Dir.mktmpdir do |dir| + unless system(*%W[git clone --bare #{SOURCE} #{REPO_NAME}], chdir: dir) + abort "git clone failed" + end + + repo = File.join(dir, REPO_NAME) + erb = ERB.new(DATA.read) + erb.run(binding) + end +end + +def capture!(cmd, dir) + output = IO.popen(cmd, 'r', chdir: dir) { |io| io.read } + raise "command failed with #{$?}: #{cmd.join(' ')}" unless $?.success? + + output.chomp +end + +main + +__END__ +# This file is generated by <%= SCRIPT_NAME %>. Do not edit this file manually. +# +# Seed repo: +<%= capture!(%w{git log --format=#\ %H\ %s}, repo) %> + +module SeedRepo + module BigCommit + ID = "913c66a37b4a45b9769037c55c2d238bd0942d2e".freeze + PARENT_ID = "cfe32cf61b73a0d5e9f13e774abde7ff789b1660".freeze + MESSAGE = "Files, encoding and much more".freeze + AUTHOR_FULL_NAME = "Dmitriy Zaporozhets".freeze + FILES_COUNT = 2 + end + + module Commit + ID = "570e7b2abdd848b95f2f578043fc23bd6f6fd24d".freeze + PARENT_ID = "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9".freeze + MESSAGE = "Change some files\n\nSigned-off-by: Dmitriy Zaporozhets \n".freeze + AUTHOR_FULL_NAME = "Dmitriy Zaporozhets".freeze + FILES = ["files/ruby/popen.rb", "files/ruby/regex.rb"].freeze + FILES_COUNT = 2 + C_FILE_PATH = "files/ruby".freeze + C_FILES = ["popen.rb", "regex.rb", "version_info.rb"].freeze + BLOB_FILE = %{%h3= @key.title\n%hr\n%pre= @key.key\n.actions\n = link_to 'Remove', @key, :confirm => 'Are you sure?', :method => :delete, :class => \"btn danger delete-key\"\n\n\n}.freeze + BLOB_FILE_PATH = "app/views/keys/show.html.haml".freeze + end + + module EmptyCommit + ID = "b0e52af38d7ea43cf41d8a6f2471351ac036d6c9".freeze + PARENT_ID = "40f4a7a617393735a95a0bb67b08385bc1e7c66d".freeze + MESSAGE = "Empty commit".freeze + AUTHOR_FULL_NAME = "Rémy Coutable".freeze + FILES = [].freeze + FILES_COUNT = FILES.count + end + + module EncodingCommit + ID = "40f4a7a617393735a95a0bb67b08385bc1e7c66d".freeze + PARENT_ID = "66028349a123e695b589e09a36634d976edcc5e8".freeze + MESSAGE = "Add ISO-8859-encoded file".freeze + AUTHOR_FULL_NAME = "Stan Hu".freeze + FILES = ["encoding/iso8859.txt"].freeze + FILES_COUNT = FILES.count + end + + module FirstCommit + ID = "1a0b36b3cdad1d2ee32457c102a8c0b7056fa863".freeze + PARENT_ID = nil + MESSAGE = "Initial commit".freeze + AUTHOR_FULL_NAME = "Dmitriy Zaporozhets".freeze + FILES = ["LICENSE", ".gitignore", "README.md"].freeze + FILES_COUNT = 3 + end + + module LastCommit + ID = <%= capture!(%w[git show -s --format=%H HEAD], repo).inspect %>.freeze + PARENT_ID = <%= capture!(%w[git show -s --format=%P HEAD], repo).split.last.inspect %>.freeze + MESSAGE = <%= capture!(%w[git show -s --format=%s HEAD], repo).inspect %>.freeze + AUTHOR_FULL_NAME = <%= capture!(%w[git show -s --format=%an HEAD], repo).inspect %>.freeze + FILES = <%= + parents = capture!(%w[git show -s --format=%P HEAD], repo).split + merge_base = parents.size > 1 ? capture!(%w[git merge-base] + parents, repo) : parents.first + capture!( %W[git diff --name-only #{merge_base}..HEAD --], repo).split("\n").inspect + %>.freeze + FILES_COUNT = FILES.count + end + + module Repo + HEAD = "master".freeze + BRANCHES = %w[ +<%= capture!(%W[git for-each-ref --format=#{' ' * 3}%(refname:strip=2) refs/heads/], repo) %> + ].freeze + TAGS = %w[ +<%= capture!(%W[git for-each-ref --format=#{' ' * 3}%(refname:strip=2) refs/tags/], repo) %> + ].freeze + end + + module RubyBlob + ID = "7e3e39ebb9b2bf433b4ad17313770fbe4051649c".freeze + NAME = "popen.rb".freeze + CONTENT = <<-eos.freeze +require 'fileutils' +require 'open3' + +module Popen + extend self + + def popen(cmd, path=nil) + unless cmd.is_a?(Array) + raise RuntimeError, "System commands must be given as an array of strings" + end + + path ||= Dir.pwd + + vars = { + "PWD" => path + } + + options = { + chdir: path + } + + unless File.directory?(path) + FileUtils.mkdir_p(path) + end + + @cmd_output = "" + @cmd_status = 0 + + Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr| + @cmd_output << stdout.read + @cmd_output << stderr.read + @cmd_status = wait_thr.value.exitstatus + end + + return @cmd_output, @cmd_status + end +end + eos + end +end diff --git a/ruby/spec/support/helpers/seed_repo.rb b/ruby/spec/support/helpers/seed_repo.rb new file mode 100644 index 00000000000..0249c6210b8 --- /dev/null +++ b/ruby/spec/support/helpers/seed_repo.rb @@ -0,0 +1,154 @@ +# This file is generated by generate-seed-repo-rb. Do not edit this file manually. +# +# Seed repo: +# 4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6 Merge branch 'master' into 'master' +# 0e1b353b348f8477bdbec1ef47087171c5032cd9 adds an executable with different permissions +# 0e50ec4d3c7ce42ab74dda1d422cb2cbffe1e326 Merge branch 'lfs_pointers' into 'master' +# 33bcff41c232a11727ac6d660bd4b0c2ba86d63d Add valid and invalid lfs pointers +# 732401c65e924df81435deb12891ef570167d2e2 Update year in license file +# b0e52af38d7ea43cf41d8a6f2471351ac036d6c9 Empty commit +# 40f4a7a617393735a95a0bb67b08385bc1e7c66d Add ISO-8859-encoded file +# 66028349a123e695b589e09a36634d976edcc5e8 Merge branch 'add-comments-to-gitmodules' into 'master' +# de5714f34c4e34f1d50b9a61a2e6c9132fe2b5fd Add comments to the end of .gitmodules to test parsing +# fa1b1e6c004a68b7d8763b86455da9e6b23e36d6 Merge branch 'add-files' into 'master' +# eb49186cfa5c4338011f5f590fac11bd66c5c631 Add submodules nested deeper than the root +# 18d9c205d0d22fdf62bc2f899443b83aafbf941f Add executables and links files +# 5937ac0a7beb003549fc5fd26fc247adbce4a52e Add submodule from gitlab.com +# 570e7b2abdd848b95f2f578043fc23bd6f6fd24d Change some files +# 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 More submodules +# d14d6c0abdd253381df51a723d58691b2ee1ab08 Remove ds_store files +# c1acaa58bbcbc3eafe538cb8274ba387047b69f8 Ignore DS files +# ae73cb07c9eeaf35924a10f713b364d32b2dd34f Binary file added +# 874797c3a73b60d2187ed6e2fcabd289ff75171e Ruby files modified +# 2f63565e7aac07bcdadb654e253078b727143ec4 Modified image +# 33f3729a45c02fc67d00adb1b8bca394b0e761d9 Image added +# 913c66a37b4a45b9769037c55c2d238bd0942d2e Files, encoding and much more +# cfe32cf61b73a0d5e9f13e774abde7ff789b1660 Add submodule +# 6d394385cf567f80a8fd85055db1ab4c5295806f Added contributing guide +# 1a0b36b3cdad1d2ee32457c102a8c0b7056fa863 Initial commit + +module SeedRepo + module BigCommit + ID = "913c66a37b4a45b9769037c55c2d238bd0942d2e".freeze + PARENT_ID = "cfe32cf61b73a0d5e9f13e774abde7ff789b1660".freeze + MESSAGE = "Files, encoding and much more".freeze + AUTHOR_FULL_NAME = "Dmitriy Zaporozhets".freeze + FILES_COUNT = 2 + end + + module Commit + ID = "570e7b2abdd848b95f2f578043fc23bd6f6fd24d".freeze + PARENT_ID = "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9".freeze + MESSAGE = "Change some files\n\nSigned-off-by: Dmitriy Zaporozhets \n".freeze + AUTHOR_FULL_NAME = "Dmitriy Zaporozhets".freeze + FILES = ["files/ruby/popen.rb", "files/ruby/regex.rb"].freeze + FILES_COUNT = 2 + C_FILE_PATH = "files/ruby".freeze + C_FILES = ["popen.rb", "regex.rb", "version_info.rb"].freeze + BLOB_FILE = %{%h3= @key.title\n%hr\n%pre= @key.key\n.actions\n = link_to 'Remove', @key, :confirm => 'Are you sure?', :method => :delete, :class => \"btn danger delete-key\"\n\n\n}.freeze + BLOB_FILE_PATH = "app/views/keys/show.html.haml".freeze + end + + module EmptyCommit + ID = "b0e52af38d7ea43cf41d8a6f2471351ac036d6c9".freeze + PARENT_ID = "40f4a7a617393735a95a0bb67b08385bc1e7c66d".freeze + MESSAGE = "Empty commit".freeze + AUTHOR_FULL_NAME = "Rémy Coutable".freeze + FILES = [].freeze + FILES_COUNT = FILES.count + end + + module EncodingCommit + ID = "40f4a7a617393735a95a0bb67b08385bc1e7c66d".freeze + PARENT_ID = "66028349a123e695b589e09a36634d976edcc5e8".freeze + MESSAGE = "Add ISO-8859-encoded file".freeze + AUTHOR_FULL_NAME = "Stan Hu".freeze + FILES = ["encoding/iso8859.txt"].freeze + FILES_COUNT = FILES.count + end + + module FirstCommit + ID = "1a0b36b3cdad1d2ee32457c102a8c0b7056fa863".freeze + PARENT_ID = nil + MESSAGE = "Initial commit".freeze + AUTHOR_FULL_NAME = "Dmitriy Zaporozhets".freeze + FILES = ["LICENSE", ".gitignore", "README.md"].freeze + FILES_COUNT = 3 + end + + module LastCommit + ID = "4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6".freeze + PARENT_ID = "0e1b353b348f8477bdbec1ef47087171c5032cd9".freeze + MESSAGE = "Merge branch 'master' into 'master'".freeze + AUTHOR_FULL_NAME = "Stan Hu".freeze + FILES = ["bin/executable"].freeze + FILES_COUNT = FILES.count + end + + module Repo + HEAD = "master".freeze + BRANCHES = %w[ + feature + fix + fix-blob-path + fix-existing-submodule-dir + fix-mode + gitattributes + gitattributes-updated + master + merge-test + rd-add-file-larger-than-1-mb + Ääh-test-utf-8 + ].freeze + TAGS = %w[ + v1.0.0 + v1.1.0 + v1.2.0 + v1.2.1 + ].freeze + end + + module RubyBlob + ID = "7e3e39ebb9b2bf433b4ad17313770fbe4051649c".freeze + NAME = "popen.rb".freeze + CONTENT = <<-eos.freeze +require 'fileutils' +require 'open3' + +module Popen + extend self + + def popen(cmd, path=nil) + unless cmd.is_a?(Array) + raise RuntimeError, "System commands must be given as an array of strings" + end + + path ||= Dir.pwd + + vars = { + "PWD" => path + } + + options = { + chdir: path + } + + unless File.directory?(path) + FileUtils.mkdir_p(path) + end + + @cmd_output = "" + @cmd_status = 0 + + Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr| + @cmd_output << stdout.read + @cmd_output << stderr.read + @cmd_status = wait_thr.value.exitstatus + end + + return @cmd_output, @cmd_status + end +end + eos + end +end diff --git a/ruby/spec/test_repo_helper.rb b/ruby/spec/test_repo_helper.rb index a2792610d1b..59595b177ff 100644 --- a/ruby/spec/test_repo_helper.rb +++ b/ruby/spec/test_repo_helper.rb @@ -7,14 +7,27 @@ DEFAULT_STORAGE_DIR = File.expand_path('../../tmp/repositories', __FILE__) DEFAULT_STORAGE_NAME = 'default'.freeze TEST_REPO_PATH = File.join(DEFAULT_STORAGE_DIR, 'gitlab-test.git') TEST_REPO_ORIGIN = '../internal/testhelper/testdata/data/gitlab-test.git'.freeze +GIT_TEST_REPO_PATH = File.join(DEFAULT_STORAGE_DIR, 'gitlab-git-test.git') +GIT_TEST_REPO_ORIGIN = '../internal/testhelper/testdata/data/gitlab-git-test.git'.freeze module TestRepo def self.prepare_test_repository FileUtils.rm_rf(Dir["#{DEFAULT_STORAGE_DIR}/mutable-*"]) - return if File.directory?(TEST_REPO_PATH) FileUtils.mkdir_p(DEFAULT_STORAGE_DIR) - clone_new_repo!(TEST_REPO_PATH) + + { + TEST_REPO_ORIGIN => TEST_REPO_PATH, + GIT_TEST_REPO_ORIGIN => GIT_TEST_REPO_PATH + }.each do |origin, path| + next if File.directory?(path) + + clone_new_repo!(origin, path) + end + end + + def git_test_repo_read_only + Gitaly::Repository.new(storage_name: DEFAULT_STORAGE_NAME, relative_path: File.basename(GIT_TEST_REPO_PATH)) end def test_repo_read_only @@ -23,7 +36,7 @@ module TestRepo def new_mutable_test_repo relative_path = "mutable-#{SecureRandom.hex(6)}.git" - TestRepo.clone_new_repo!(File.join(DEFAULT_STORAGE_DIR, relative_path)) + TestRepo.clone_new_repo!(TEST_REPO_ORIGIN, File.join(DEFAULT_STORAGE_DIR, relative_path)) Gitaly::Repository.new(storage_name: DEFAULT_STORAGE_NAME, relative_path: relative_path) end @@ -53,8 +66,8 @@ module TestRepo ) end - def self.clone_new_repo!(destination) - return if system(*%W[git clone --quiet --bare #{TEST_REPO_ORIGIN} #{destination}]) + def self.clone_new_repo!(origin, destination) + return if system(*%W[git clone --quiet --bare #{origin} #{destination}]) abort "Failed to clone test repo. Try running 'make prepare-tests' and try again." end -- GitLab From 5c576f034f9a771b236116f4cc6c51cde86d5980 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Thu, 11 Oct 2018 17:16:29 -0300 Subject: [PATCH 2/9] Add Gitlab::Git::Branch unit specs --- ruby/Gemfile | 1 + ruby/Gemfile.lock | 2 + ruby/spec/lib/gitlab/git/branch_spec.rb | 66 +++++++++++++++++++++++++ ruby/spec/spec_helper.rb | 1 + 4 files changed, 70 insertions(+) create mode 100644 ruby/spec/lib/gitlab/git/branch_spec.rb diff --git a/ruby/Gemfile b/ruby/Gemfile index aced9c0e4cb..e11aac6e719 100644 --- a/ruby/Gemfile +++ b/ruby/Gemfile @@ -25,4 +25,5 @@ gem 'google-protobuf', '= 3.5.1' group :development, :test do gem 'gitlab-styles', '~> 2.0.0', require: false gem 'rspec', require: false + gem 'timecop' end diff --git a/ruby/Gemfile.lock b/ruby/Gemfile.lock index 45621905bcc..887f9107171 100644 --- a/ruby/Gemfile.lock +++ b/ruby/Gemfile.lock @@ -137,6 +137,7 @@ GEM multi_json (~> 1.10) stringex (2.8.4) thread_safe (0.3.6) + timecop (0.9.1) tzinfo (1.2.2) thread_safe (~> 0.1) unicode-display_width (1.3.0) @@ -161,6 +162,7 @@ DEPENDENCIES rspec rugged (~> 0.27) sentry-raven (~> 2.7.2) + timecop BUNDLED WITH 1.16.6 diff --git a/ruby/spec/lib/gitlab/git/branch_spec.rb b/ruby/spec/lib/gitlab/git/branch_spec.rb new file mode 100644 index 00000000000..34453423877 --- /dev/null +++ b/ruby/spec/lib/gitlab/git/branch_spec.rb @@ -0,0 +1,66 @@ +require "spec_helper" + +describe Gitlab::Git::Branch, :seed_helper do + include TestRepo + + let(:repository) { gitlab_git_from_gitaly(git_test_repo_read_only) } + let(:rugged) do + Rugged::Repository.new(GIT_TEST_REPO_PATH) + end + + subject { repository.branches } + + it { is_expected.to be_kind_of Array } + + describe '.find' do + subject { described_class.find(repository, branch) } + + before do + allow(repository).to receive(:find_branch).with(branch) + .and_call_original + end + + context 'when finding branch via branch name' do + let(:branch) { 'master' } + + it 'returns a branch object' do + expect(subject).to be_a(described_class) + expect(subject.name).to eq(branch) + + expect(repository).to have_received(:find_branch).with(branch) + end + end + + context 'when the branch is already a branch' do + let(:commit) { repository.commit('master') } + let(:branch) { described_class.new(repository, 'master', commit.sha, commit) } + + it 'returns a branch object' do + expect(subject).to be_a(described_class) + expect(subject).to eq(branch) + + expect(repository).not_to have_received(:find_branch).with(branch) + end + end + end + + describe '#size' do + subject { super().size } + it { is_expected.to eq(SeedRepo::Repo::BRANCHES.size) } + end + + describe 'master branch' do + let(:branch) do + repository.branches.find { |branch| branch.name == 'master' } + end + + it { expect(branch.dereferenced_target.sha).to eq(SeedRepo::LastCommit::ID) } + end + + it { expect(repository.branches.size).to eq(SeedRepo::Repo::BRANCHES.size) } + + def create_commit + params[:message].delete!("\r") + Rugged::Commit.create(rugged, params.merge(committer: committer.merge(time: Time.now))) + end +end diff --git a/ruby/spec/spec_helper.rb b/ruby/spec/spec_helper.rb index 54aa9038db2..be9412e5a69 100644 --- a/ruby/spec/spec_helper.rb +++ b/ruby/spec/spec_helper.rb @@ -1,6 +1,7 @@ require_relative '../lib/gitaly_server.rb' require_relative '../lib/gitlab/git.rb' require_relative 'support/sentry.rb' +require 'timecop' require 'test_repo_helper' Dir[File.join(__dir__, 'support/helpers/*.rb')].each { |f| require f } -- GitLab From e1ddfa18a538a15945f9515f4157b175ef2706bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Thu, 11 Oct 2018 18:10:51 -0300 Subject: [PATCH 3/9] Add Gitlab::Git::Commit unit specs --- ruby/Gemfile | 1 + ruby/Gemfile.lock | 3 + ruby/lib/gitlab/git/commit.rb | 35 +-- ruby/spec/factories/gitaly/commit.rb | 17 ++ ruby/spec/factories/gitaly/commit_author.rb | 9 + ruby/spec/factories/sequences.rb | 4 + ruby/spec/lib/gitlab/git/commit_spec.rb | 234 ++++++++++++++++++++ ruby/spec/spec_helper.rb | 9 + ruby/spec/test_repo_helper.rb | 21 +- 9 files changed, 297 insertions(+), 36 deletions(-) create mode 100644 ruby/spec/factories/gitaly/commit.rb create mode 100644 ruby/spec/factories/gitaly/commit_author.rb create mode 100644 ruby/spec/factories/sequences.rb create mode 100644 ruby/spec/lib/gitlab/git/commit_spec.rb diff --git a/ruby/Gemfile b/ruby/Gemfile index e11aac6e719..3aa5fd4cecc 100644 --- a/ruby/Gemfile +++ b/ruby/Gemfile @@ -26,4 +26,5 @@ group :development, :test do gem 'gitlab-styles', '~> 2.0.0', require: false gem 'rspec', require: false gem 'timecop' + gem 'factory_bot' end diff --git a/ruby/Gemfile.lock b/ruby/Gemfile.lock index 887f9107171..87f575f96b1 100644 --- a/ruby/Gemfile.lock +++ b/ruby/Gemfile.lock @@ -14,6 +14,8 @@ GEM crass (1.0.4) diff-lcs (1.3) escape_utils (1.2.1) + factory_bot (4.11.1) + activesupport (>= 3.0.0) faraday (0.12.2) multipart-post (>= 1.2, < 3) gemojione (3.3.0) @@ -148,6 +150,7 @@ PLATFORMS DEPENDENCIES activesupport (~> 5.0.2) bundler (>= 1.16.5) + factory_bot faraday (~> 0.12) gitaly-proto (~> 0.116.0) github-linguist (~> 6.1) diff --git a/ruby/lib/gitlab/git/commit.rb b/ruby/lib/gitlab/git/commit.rb index ad1871f1a0b..e7382d5143f 100644 --- a/ruby/lib/gitlab/git/commit.rb +++ b/ruby/lib/gitlab/git/commit.rb @@ -161,17 +161,6 @@ module Gitlab Gitlab::Git::CommitStats.new(@repository, self) end - # Get ref names collection - # - # Ex. - # commit.ref_names(repo) - # - def ref_names(repo) - refs(repo).map do |ref| - ref.sub(%r{^refs/(heads|remotes|tags)/}, "") - end - end - def message encode! @message end @@ -293,32 +282,10 @@ module Gitlab ) end - # Get a collection of Gitlab::Git::Ref objects for this commit. - # - # Ex. - # commit.ref(repo) - # - def refs(repo) - repo.refs_hash[id] - end - def message_from_gitaly_body return @raw_commit.subject.dup if @raw_commit.body_size.zero? - return @raw_commit.body.dup if full_body_fetched_from_gitaly? - - if @raw_commit.body_size > MAX_COMMIT_MESSAGE_DISPLAY_SIZE - "#{@raw_commit.subject}\n\n--commit message is too big".strip - else - fetch_body_from_gitaly - end - end - - def full_body_fetched_from_gitaly? - @raw_commit.body.bytesize == @raw_commit.body_size - end - def fetch_body_from_gitaly - self.class.get_message(@repository, id) + @raw_commit.body.dup end end end diff --git a/ruby/spec/factories/gitaly/commit.rb b/ruby/spec/factories/gitaly/commit.rb new file mode 100644 index 00000000000..5034c3d0e48 --- /dev/null +++ b/ruby/spec/factories/gitaly/commit.rb @@ -0,0 +1,17 @@ +FactoryBot.define do + sequence(:gitaly_commit_id) { Digest::SHA1.hexdigest(Time.now.to_f.to_s) } + + factory :gitaly_commit, class: Gitaly::GitCommit do + skip_create + + id { generate(:gitaly_commit_id) } + parent_ids do + ids = [generate(:gitaly_commit_id), generate(:gitaly_commit_id)] + Google::Protobuf::RepeatedField.new(:string, ids) + end + subject { "My commit" } + body { subject + "\nMy body" } + author { build(:gitaly_commit_author) } + committer { build(:gitaly_commit_author) } + end +end diff --git a/ruby/spec/factories/gitaly/commit_author.rb b/ruby/spec/factories/gitaly/commit_author.rb new file mode 100644 index 00000000000..aaf634ce08b --- /dev/null +++ b/ruby/spec/factories/gitaly/commit_author.rb @@ -0,0 +1,9 @@ +FactoryBot.define do + factory :gitaly_commit_author, class: Gitaly::CommitAuthor do + skip_create + + name { generate(:name) } + email { generate(:email) } + date { Google::Protobuf::Timestamp.new(seconds: Time.now.to_i) } + end +end diff --git a/ruby/spec/factories/sequences.rb b/ruby/spec/factories/sequences.rb new file mode 100644 index 00000000000..16a05b1ca06 --- /dev/null +++ b/ruby/spec/factories/sequences.rb @@ -0,0 +1,4 @@ +FactoryBot.define do + sequence(:name) { |n| "John Doe#{n}" } + sequence(:email) { |n| "user#{n}@example.org" } +end diff --git a/ruby/spec/lib/gitlab/git/commit_spec.rb b/ruby/spec/lib/gitlab/git/commit_spec.rb new file mode 100644 index 00000000000..97b02c2083c --- /dev/null +++ b/ruby/spec/lib/gitlab/git/commit_spec.rb @@ -0,0 +1,234 @@ +require "spec_helper" + +describe Gitlab::Git::Commit, :seed_helper do + include TestRepo + + let(:repository) { gitlab_git_from_gitaly(git_test_repo_read_only) } + let(:rugged_repo) do + Rugged::Repository.new(GIT_TEST_REPO_PATH) + end + let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) } + let(:rugged_commit) { rugged_repo.lookup(SeedRepo::Commit::ID) } + + describe "Commit info" do + before do + @committer = { + email: 'mike@smith.com', + name: "Mike Smith", + time: Time.now + } + + @author = { + email: 'john@smith.com', + name: "John Smith", + time: Time.now + } + + @parents = [rugged_repo.head.target] + @gitlab_parents = @parents.map { |c| described_class.find(repository, c.oid) } + @tree = @parents.first.tree + + sha = Rugged::Commit.create( + rugged_repo, + author: @author, + committer: @committer, + tree: @tree, + parents: @parents, + message: "Refactoring specs", + update_ref: "HEAD" + ) + + @raw_commit = rugged_repo.lookup(sha) + @commit = described_class.find(repository, sha) + end + + it { expect(@commit.short_id).to eq(@raw_commit.oid[0..10]) } + it { expect(@commit.id).to eq(@raw_commit.oid) } + it { expect(@commit.sha).to eq(@raw_commit.oid) } + it { expect(@commit.safe_message).to eq(@raw_commit.message) } + it { expect(@commit.created_at).to eq(@raw_commit.author[:time]) } + it { expect(@commit.date).to eq(@raw_commit.committer[:time]) } + it { expect(@commit.author_email).to eq(@author[:email]) } + it { expect(@commit.author_name).to eq(@author[:name]) } + it { expect(@commit.committer_name).to eq(@committer[:name]) } + it { expect(@commit.committer_email).to eq(@committer[:email]) } + it { expect(@commit.different_committer?).to be_truthy } + it { expect(@commit.parents).to eq(@gitlab_parents) } + it { expect(@commit.parent_id).to eq(@parents.first.oid) } + it { expect(@commit.no_commit_message).to eq("--no commit message") } + + after do + # Erase the new commit so other tests get the original repo + rugged_repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID) + end + end + + describe "Commit info from gitaly commit" do + let(:subject) { "My commit".force_encoding('ASCII-8BIT') } + let(:body) { subject + "My body".force_encoding('ASCII-8BIT') } + let(:body_size) { body.length } + let(:gitaly_commit) { build(:gitaly_commit, subject: subject, body: body, body_size: body_size) } + let(:id) { gitaly_commit.id } + let(:committer) { gitaly_commit.committer } + let(:author) { gitaly_commit.author } + let(:commit) { described_class.new(repository, gitaly_commit) } + + it { expect(commit.short_id).to eq(id[0..10]) } + it { expect(commit.id).to eq(id) } + it { expect(commit.sha).to eq(id) } + it { expect(commit.safe_message).to eq(body) } + it { expect(commit.created_at).to eq(Time.at(committer.date.seconds)) } + it { expect(commit.author_email).to eq(author.email) } + it { expect(commit.author_name).to eq(author.name) } + it { expect(commit.committer_name).to eq(committer.name) } + it { expect(commit.committer_email).to eq(committer.email) } + it { expect(commit.parent_ids).to eq(gitaly_commit.parent_ids) } + + context 'body_size != body.size' do + let(:body) { "".force_encoding('ASCII-8BIT') } + + context 'zero body_size' do + it { expect(commit.safe_message).to eq(subject) } + end + end + end + + context 'Class methods' do + describe '.find' do + it "returns an array of parent ids" do + expect(described_class.find(repository, SeedRepo::Commit::ID).parent_ids).to be_an(Array) + end + + it "should return valid commit for tag" do + expect(described_class.find(repository, 'v1.0.0').id).to eq('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') + end + + it "should return nil for non-commit ids" do + blob = Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "files/ruby/popen.rb") + expect(described_class.find(repository, blob.id)).to be_nil + end + + it "should return nil for parent of non-commit object" do + blob = Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "files/ruby/popen.rb") + expect(described_class.find(repository, "#{blob.id}^")).to be_nil + end + + it "should return nil for nonexisting ids" do + expect(described_class.find(repository, "+123_4532530XYZ")).to be_nil + end + + context 'with broken repo' do + let(:repository) { gitlab_git_from_gitaly(new_broken_test_repo) } + + it 'returns nil' do + expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_nil + end + end + end + + describe '.shas_with_signatures' do + let(:signed_shas) { %w[5937ac0a7beb003549fc5fd26fc247adbce4a52e 570e7b2abdd848b95f2f578043fc23bd6f6fd24d] } + let(:unsigned_shas) { %w[19e2e9b4ef76b422ce1154af39a91323ccc57434 c642fe9b8b9f28f9225d7ea953fe14e74748d53b] } + let(:first_signed_shas) { %w[5937ac0a7beb003549fc5fd26fc247adbce4a52e c642fe9b8b9f28f9225d7ea953fe14e74748d53b] } + + it 'has 2 signed shas' do + ret = described_class.shas_with_signatures(repository, signed_shas) + expect(ret).to eq(signed_shas) + end + + it 'has 0 signed shas' do + ret = described_class.shas_with_signatures(repository, unsigned_shas) + expect(ret).to eq([]) + end + + it 'has 1 signed sha' do + ret = described_class.shas_with_signatures(repository, first_signed_shas) + expect(ret).to contain_exactly(first_signed_shas.first) + end + end + end + + describe 'move this test to gitaly-ruby' do + describe '#init_from_rugged' do + let(:gitlab_commit) { described_class.new(repository, rugged_commit) } + subject { gitlab_commit } + + describe '#id' do + subject { super().id } + it { is_expected.to eq(SeedRepo::Commit::ID) } + end + end + end + + describe '#init_from_hash' do + let(:commit) { described_class.new(repository, sample_commit_hash) } + subject { commit } + + describe '#id' do + subject { super().id } + it { is_expected.to eq(sample_commit_hash[:id])} + end + + describe '#message' do + subject { super().message } + it { is_expected.to eq(sample_commit_hash[:message])} + end + end + + shared_examples '#stats' do + subject { commit.stats } + + describe '#additions' do + subject { super().additions } + it { is_expected.to eq(11) } + end + + describe '#deletions' do + subject { super().deletions } + it { is_expected.to eq(6) } + end + + describe '#total' do + subject { super().total } + it { is_expected.to eq(17) } + end + end + + describe '#stats with gitaly on' do + it_should_behave_like '#stats' + end + + describe '#stats with gitaly disabled', :skip_gitaly_mock do + it_should_behave_like '#stats' + end + + describe '#has_zero_stats?' do + it { expect(commit.has_zero_stats?).to eq(false) } + end + + describe '#to_hash' do + let(:hash) { commit.to_hash } + subject { hash } + + it { is_expected.to be_kind_of Hash } + + describe '#keys' do + subject { super().keys.sort } + it { is_expected.to match(sample_commit_hash.keys.sort) } + end + end + + def sample_commit_hash + { + author_email: "dmitriy.zaporozhets@gmail.com", + author_name: "Dmitriy Zaporozhets", + authored_date: "2012-02-27 20:51:12 +0200", + committed_date: "2012-02-27 20:51:12 +0200", + committer_email: "dmitriy.zaporozhets@gmail.com", + committer_name: "Dmitriy Zaporozhets", + id: SeedRepo::Commit::ID, + message: "tree css fixes", + parent_ids: ["874797c3a73b60d2187ed6e2fcabd289ff75171e"] + } + end +end diff --git a/ruby/spec/spec_helper.rb b/ruby/spec/spec_helper.rb index be9412e5a69..818c3c44c48 100644 --- a/ruby/spec/spec_helper.rb +++ b/ruby/spec/spec_helper.rb @@ -3,7 +3,16 @@ require_relative '../lib/gitlab/git.rb' require_relative 'support/sentry.rb' require 'timecop' require 'test_repo_helper' +require 'factory_bot' Dir[File.join(__dir__, 'support/helpers/*.rb')].each { |f| require f } ENV['GITALY_RUBY_GIT_BIN_PATH'] ||= 'git' + +RSpec.configure do |config| + config.include FactoryBot::Syntax::Methods + + config.before(:suite) do + FactoryBot.find_definitions + end +end diff --git a/ruby/spec/test_repo_helper.rb b/ruby/spec/test_repo_helper.rb index 59595b177ff..3181bfd1d57 100644 --- a/ruby/spec/test_repo_helper.rb +++ b/ruby/spec/test_repo_helper.rb @@ -35,13 +35,24 @@ module TestRepo end def new_mutable_test_repo - relative_path = "mutable-#{SecureRandom.hex(6)}.git" + relative_path = random_repository_relative_path(:mutable) TestRepo.clone_new_repo!(TEST_REPO_ORIGIN, File.join(DEFAULT_STORAGE_DIR, relative_path)) Gitaly::Repository.new(storage_name: DEFAULT_STORAGE_NAME, relative_path: relative_path) end + def new_broken_test_repo + relative_path = random_repository_relative_path(:broken) + repo_path = File.join(DEFAULT_STORAGE_DIR, relative_path) + TestRepo.clone_new_repo!(TEST_REPO_ORIGIN, repo_path) + + refs_path = File.join(repo_path, 'refs') + FileUtils.rm_r(refs_path) + + Gitaly::Repository.new(storage_name: DEFAULT_STORAGE_NAME, relative_path: relative_path) + end + def new_empty_test_repo - relative_path = "mutable-#{SecureRandom.hex(6)}.git" + relative_path = random_repository_relative_path(:mutable) TestRepo.init_new_repo!(File.join(DEFAULT_STORAGE_DIR, relative_path)) Gitaly::Repository.new(storage_name: DEFAULT_STORAGE_NAME, relative_path: relative_path) end @@ -76,6 +87,12 @@ module TestRepo abort "Failed to init test repo." end + + private + + def random_repository_relative_path(prefix) + "#{prefix}-#{SecureRandom.hex(6)}.git" + end end TestRepo.prepare_test_repository -- GitLab From d483f47d02906914be8a093f34ed62ab04584ffa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Thu, 11 Oct 2018 19:46:11 -0300 Subject: [PATCH 4/9] Add Gitlab::Git::RemoteRepository unit specs --- ruby/Gemfile | 1 + ruby/Gemfile.lock | 35 ++++++ .../lib/gitlab/git/remote_repository_spec.rb | 105 ++++++++++++++++++ ruby/spec/spec_helper.rb | 1 + ruby/spec/test_repo_helper.rb | 6 + 5 files changed, 148 insertions(+) create mode 100644 ruby/spec/lib/gitlab/git/remote_repository_spec.rb diff --git a/ruby/Gemfile b/ruby/Gemfile index 3aa5fd4cecc..3b11b970f99 100644 --- a/ruby/Gemfile +++ b/ruby/Gemfile @@ -25,6 +25,7 @@ gem 'google-protobuf', '= 3.5.1' group :development, :test do gem 'gitlab-styles', '~> 2.0.0', require: false gem 'rspec', require: false + gem 'rspec-parameterized', require: false gem 'timecop' gem 'factory_bot' end diff --git a/ruby/Gemfile.lock b/ruby/Gemfile.lock index 87f575f96b1..9d877699d42 100644 --- a/ruby/Gemfile.lock +++ b/ruby/Gemfile.lock @@ -1,18 +1,30 @@ GEM remote: https://rubygems.org/ specs: + abstract_type (0.0.7) activesupport (5.0.6) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (~> 0.7) minitest (~> 5.1) tzinfo (~> 1.1) + adamantium (0.2.0) + ice_nine (~> 0.11.0) + memoizable (~> 0.4.0) addressable (2.5.2) public_suffix (>= 2.0.2, < 4.0) ast (2.3.0) + binding_of_caller (0.8.0) + debug_inspector (>= 0.0.1) charlock_holmes (0.7.6) + coderay (1.1.2) + concord (0.1.5) + adamantium (~> 0.2.0) + equalizer (~> 0.0.9) concurrent-ruby (1.0.5) crass (1.0.4) + debug_inspector (0.0.3) diff-lcs (1.3) + equalizer (0.0.11) escape_utils (1.2.1) factory_bot (4.11.1) activesupport (>= 3.0.0) @@ -68,6 +80,7 @@ GEM googleapis-common-protos-types (~> 1.0.0) googleauth (>= 0.5.1, < 0.7) i18n (0.8.1) + ice_nine (0.11.2) json (2.1.0) jwt (2.1.0) licensee (8.9.2) @@ -77,6 +90,8 @@ GEM little-plugger (~> 1.1) multi_json (~> 1.10) memoist (0.16.0) + memoizable (0.4.2) + thread_safe (~> 0.3, >= 0.3.1) mime-types (3.2.2) mime-types-data (~> 3.2015) mime-types-data (3.2018.0812) @@ -94,6 +109,11 @@ GEM ast (~> 2.2) posix-spawn (0.3.13) powerpack (0.1.1) + proc_to_ast (0.1.0) + coderay + parser + unparser + procto (0.0.3) public_suffix (3.0.2) rainbow (2.2.2) rake @@ -112,6 +132,12 @@ GEM rspec-mocks (3.7.0) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.7.0) + rspec-parameterized (0.4.0) + binding_of_caller + parser + proc_to_ast + rspec (>= 2.13, < 4) + unparser rspec-support (3.7.1) rubocop (0.50.0) parallel (~> 1.10) @@ -143,6 +169,14 @@ GEM tzinfo (1.2.2) thread_safe (~> 0.1) unicode-display_width (1.3.0) + unparser (0.2.8) + abstract_type (~> 0.0.7) + adamantium (~> 0.2.0) + concord (~> 0.1.5) + diff-lcs (~> 1.3) + equalizer (~> 0.0.9) + parser (>= 2.3.1.2, < 2.6) + procto (~> 0.0.2) PLATFORMS ruby @@ -163,6 +197,7 @@ DEPENDENCIES licensee (~> 8.9.0) rdoc (~> 4.2) rspec + rspec-parameterized rugged (~> 0.27) sentry-raven (~> 2.7.2) timecop diff --git a/ruby/spec/lib/gitlab/git/remote_repository_spec.rb b/ruby/spec/lib/gitlab/git/remote_repository_spec.rb new file mode 100644 index 00000000000..93d88f6445c --- /dev/null +++ b/ruby/spec/lib/gitlab/git/remote_repository_spec.rb @@ -0,0 +1,105 @@ +require 'spec_helper' + +describe Gitlab::Git::RemoteRepository, :seed_helper do + include TestRepo + + let(:repository) { gitlab_git_from_gitaly(git_test_repo_read_only) } + let(:non_existing_gitaly_repo) do + Gitaly::Repository.new(storage_name: DEFAULT_STORAGE_NAME, relative_path: 'does-not-exist.git') + end + + subject { described_class.new(repository) } + + describe '#empty?' do + using RSpec::Parameterized::TableSyntax + + where(:repository, :result) do + repository | false + gitlab_git_from_gitaly(non_existing_gitaly_repo) | true + end + + with_them do + it { expect(subject.empty?).to eq(result) } + end + end + + describe '#commit_id' do + it 'returns an OID if the revision exists' do + expect(subject.commit_id('v1.0.0')).to eq('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') + end + + it 'is nil when the revision does not exist' do + expect(subject.commit_id('does-not-exist')).to be_nil + end + end + + describe '#branch_exists?' do + using RSpec::Parameterized::TableSyntax + + where(:branch, :result) do + 'master' | true + 'does-not-exist' | false + end + + with_them do + it { expect(subject.branch_exists?(branch)).to eq(result) } + end + end + + describe '#same_repository?' do + using RSpec::Parameterized::TableSyntax + + where(:other_repository, :result) do + repository | true + repository_from_relative_path(repository.relative_path) | true + repository_from_relative_path('wrong/relative-path.git') | false + end + + with_them do + it { expect(subject.same_repository?(other_repository)).to eq(result) } + end + end + + describe '#fetch_env' do + let(:remote_repository) { described_class.new(repository) } + + let(:gitaly_client) { double(:gitaly_client) } + let(:address) { 'fake-address' } + let(:token) { 'fake-token' } + + subject { remote_repository.fetch_env } + + before do + ENV['GITALY_RUBY_GITALY_BIN_DIR'] = __dir__ + + allow(remote_repository).to receive(:gitaly_client).and_return(gitaly_client) + + expect(gitaly_client).to receive(:address).with(repository.storage).and_return(address) + expect(gitaly_client).to receive(:token).with(repository.storage).and_return(token) + end + + it { expect(subject).to be_a(Hash) } + it { expect(subject['GITALY_ADDRESS']).to eq(address) } + it { expect(subject['GITALY_TOKEN']).to eq(token) } + it { expect(subject['GITALY_WD']).to eq(Dir.pwd) } + + it 'creates a plausible GIT_SSH_COMMAND' do + git_ssh_command = subject['GIT_SSH_COMMAND'] + + expect(git_ssh_command).to start_with('/') + expect(git_ssh_command).to end_with('/gitaly-ssh upload-pack') + end + + it 'creates a plausible GITALY_PAYLOAD' do + req = Gitaly::SSHUploadPackRequest.decode_json(subject['GITALY_PAYLOAD']) + + expect(remote_repository.gitaly_repository).to eq(req.repository) + end + + context 'when the token is blank' do + let(:token) { '' } + + it { expect(subject.keys).not_to include('GITALY_TOKEN') } + end + end +end diff --git a/ruby/spec/spec_helper.rb b/ruby/spec/spec_helper.rb index 818c3c44c48..5e83fd5a002 100644 --- a/ruby/spec/spec_helper.rb +++ b/ruby/spec/spec_helper.rb @@ -3,6 +3,7 @@ require_relative '../lib/gitlab/git.rb' require_relative 'support/sentry.rb' require 'timecop' require 'test_repo_helper' +require 'rspec-parameterized' require 'factory_bot' Dir[File.join(__dir__, 'support/helpers/*.rb')].each { |f| require f } diff --git a/ruby/spec/test_repo_helper.rb b/ruby/spec/test_repo_helper.rb index 3181bfd1d57..e78f8884cab 100644 --- a/ruby/spec/test_repo_helper.rb +++ b/ruby/spec/test_repo_helper.rb @@ -77,6 +77,12 @@ module TestRepo ) end + def repository_from_relative_path(relative_path) + gitlab_git_from_gitaly( + Gitaly::Repository.new(storage_name: DEFAULT_STORAGE_NAME, relative_path: relative_path) + ) + end + def self.clone_new_repo!(origin, destination) return if system(*%W[git clone --quiet --bare #{origin} #{destination}]) abort "Failed to clone test repo. Try running 'make prepare-tests' and try again." -- GitLab From 85a8edf0fe840a21056befaf0d221a5a7dd9de5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Thu, 11 Oct 2018 22:37:25 -0300 Subject: [PATCH 5/9] Expand Gitlab::Git::Repository unit specs with examples from rails --- ruby/spec/gitaly/ref_service_spec.rb | 2 +- ruby/spec/gitaly/repository_service_spec.rb | 2 +- .../lib/gitlab/git/remote_repository_spec.rb | 2 - ruby/spec/lib/gitlab/git/repository_spec.rb | 695 +++++++++++++++++- ruby/spec/lib/gitlab/git/wiki_spec.rb | 1 - ruby/spec/spec_helper.rb | 4 + .../support/helpers/gitlab_shell_helper.rb | 13 + .../helpers}/integration_helper.rb | 18 +- ruby/spec/test_repo_helper.rb | 6 + 9 files changed, 726 insertions(+), 17 deletions(-) create mode 100644 ruby/spec/support/helpers/gitlab_shell_helper.rb rename ruby/spec/{ => support/helpers}/integration_helper.rb (75%) diff --git a/ruby/spec/gitaly/ref_service_spec.rb b/ruby/spec/gitaly/ref_service_spec.rb index 96a64863987..c3ba9d30cab 100644 --- a/ruby/spec/gitaly/ref_service_spec.rb +++ b/ruby/spec/gitaly/ref_service_spec.rb @@ -1,5 +1,5 @@ -require 'integration_helper' require 'securerandom' +require 'spec_helper' describe Gitaly::RefService do include IntegrationClient diff --git a/ruby/spec/gitaly/repository_service_spec.rb b/ruby/spec/gitaly/repository_service_spec.rb index 402a367bad5..5485f34de02 100644 --- a/ruby/spec/gitaly/repository_service_spec.rb +++ b/ruby/spec/gitaly/repository_service_spec.rb @@ -1,4 +1,4 @@ -require 'integration_helper' +require 'spec_helper' describe Gitaly::RepositoryService do include IntegrationClient diff --git a/ruby/spec/lib/gitlab/git/remote_repository_spec.rb b/ruby/spec/lib/gitlab/git/remote_repository_spec.rb index 93d88f6445c..2788f482b7f 100644 --- a/ruby/spec/lib/gitlab/git/remote_repository_spec.rb +++ b/ruby/spec/lib/gitlab/git/remote_repository_spec.rb @@ -70,8 +70,6 @@ describe Gitlab::Git::RemoteRepository, :seed_helper do subject { remote_repository.fetch_env } before do - ENV['GITALY_RUBY_GITALY_BIN_DIR'] = __dir__ - allow(remote_repository).to receive(:gitaly_client).and_return(gitaly_client) expect(gitaly_client).to receive(:address).with(repository.storage).and_return(address) diff --git a/ruby/spec/lib/gitlab/git/repository_spec.rb b/ruby/spec/lib/gitlab/git/repository_spec.rb index f7464b8377c..8401a871119 100644 --- a/ruby/spec/lib/gitlab/git/repository_spec.rb +++ b/ruby/spec/lib/gitlab/git/repository_spec.rb @@ -2,8 +2,636 @@ require 'spec_helper' describe Gitlab::Git::Repository do include TestRepo + include Gitlab::EncodingHelper + using RSpec::Parameterized::TableSyntax - let(:repository) { gitlab_git_from_gitaly(test_repo_read_only) } + shared_examples 'wrapping gRPC errors' do |gitaly_client_class, gitaly_client_method| + it 'wraps gRPC not found error' do + expect_any_instance_of(gitaly_client_class).to receive(gitaly_client_method) + .and_raise(GRPC::NotFound) + expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository) + end + + it 'wraps gRPC unknown error' do + expect_any_instance_of(gitaly_client_class).to receive(gitaly_client_method) + .and_raise(GRPC::Unknown) + expect { subject }.to raise_error(Gitlab::Git::CommandError) + end + end + + let(:mutable_repository) { gitlab_git_from_gitaly(new_mutable_git_test_repo) } + let(:repository) { gitlab_git_from_gitaly(git_test_repo_read_only) } + let(:repository_path) { repository.path } + let(:repository_rugged) { Rugged::Repository.new(repository_path) } + let(:storage_path) { DEFAULT_STORAGE_DIR } + let(:user) { Gitlab::Git::User.new('johndone', 'John Doe', 'johndoe@mail.com', 'user-1') } + + describe '.create_hooks' do + let(:repo_path) { File.join(storage_path, 'hook-test.git') } + let(:hooks_dir) { File.join(repo_path, 'hooks') } + let(:target_hooks_dir) { Gitlab.config.gitlab_shell.hooks_path } + let(:existing_target) { File.join(repo_path, 'foobar') } + + before do + GitlabShellHelper.setup_gitlab_shell + + FileUtils.rm_rf(repo_path) + FileUtils.mkdir_p(repo_path) + end + + context 'hooks is a directory' do + let(:existing_file) { File.join(hooks_dir, 'my-file') } + + before do + FileUtils.mkdir_p(hooks_dir) + FileUtils.touch(existing_file) + described_class.create_hooks(repo_path, target_hooks_dir) + end + + it { expect(File.readlink(hooks_dir)).to eq(target_hooks_dir) } + it { expect(Dir[File.join(repo_path, "hooks.old.*/my-file")].count).to eq(1) } + end + + context 'hooks is a valid symlink' do + before do + FileUtils.mkdir_p existing_target + File.symlink(existing_target, hooks_dir) + described_class.create_hooks(repo_path, target_hooks_dir) + end + + it { expect(File.readlink(hooks_dir)).to eq(target_hooks_dir) } + end + + context 'hooks is a broken symlink' do + before do + FileUtils.rm_f(existing_target) + File.symlink(existing_target, hooks_dir) + described_class.create_hooks(repo_path, target_hooks_dir) + end + + it { expect(File.readlink(hooks_dir)).to eq(target_hooks_dir) } + end + end + + describe "Respond to" do + subject { repository } + + it { is_expected.to respond_to(:root_ref) } + it { is_expected.to respond_to(:tags) } + end + + describe '#root_ref' do + it 'calls #discover_default_branch' do + expect(repository).to receive(:discover_default_branch) + repository.root_ref + end + end + + describe '#branch_names' do + subject { repository.branch_names } + + it 'has SeedRepo::Repo::BRANCHES.size elements' do + expect(subject.size).to eq(SeedRepo::Repo::BRANCHES.size) + end + + it { is_expected.to include("master") } + it { is_expected.not_to include("branch-from-space") } + end + + describe '#tag_names' do + subject { repository.tag_names } + + it { is_expected.to be_kind_of Array } + + it 'has SeedRepo::Repo::TAGS.size elements' do + expect(subject.size).to eq(SeedRepo::Repo::TAGS.size) + end + + describe '#last' do + subject { super().last } + it { is_expected.to eq("v1.2.1") } + end + it { is_expected.to include("v1.0.0") } + it { is_expected.not_to include("v5.0.0") } + end + + describe '#empty?' do + it { expect(repository).not_to be_empty } + end + + describe "#delete_branch" do + let(:repository) { mutable_repository } + + it "removes the branch from the repo" do + branch_name = "to-be-deleted-soon" + + create_branch(repository, branch_name) + expect(repository_rugged.branches[branch_name]).not_to be_nil + + repository.delete_branch(branch_name) + expect(repository_rugged.branches[branch_name]).to be_nil + end + + context "when branch does not exist" do + it "raises a DeleteBranchError exception" do + expect { repository.delete_branch("this-branch-does-not-exist") }.to raise_error(Gitlab::Git::Repository::DeleteBranchError) + end + end + end + + describe '#delete_refs' do + let(:repository) { mutable_repository } + + it 'deletes the ref' do + repository.delete_refs('refs/heads/feature') + + expect(repository_rugged.references['refs/heads/feature']).to be_nil + end + + it 'deletes all refs' do + refs = %w[refs/heads/wip refs/tags/v1.1.0] + repository.delete_refs(*refs) + + refs.each do |ref| + expect(repository_rugged.references[ref]).to be_nil + end + end + + it 'does not fail when deleting an empty list of refs' do + expect { repository.delete_refs(*[]) }.not_to raise_error + end + + it 'raises an error if it failed' do + expect { repository.delete_refs('refs\heads\fix') }.to raise_error(Gitlab::Git::Repository::GitError) + end + end + + describe '#fetch_repository_as_mirror' do + let(:new_repository) do + gitlab_git_from_gitaly(new_empty_test_repo) + end + let(:repository) { mutable_repository } + let(:remote_repository) { Gitlab::Git::RemoteRepository.new(repository) } + let(:fake_env) { {} } + + before do + + end + + subject { new_repository.fetch_repository_as_mirror(remote_repository) } + + it 'fetches a repository as a mirror remote' do + expect(new_repository).to receive(:add_remote).with(anything, Gitlab::Git::Repository::GITALY_INTERNAL_URL, mirror_refmap: :all_refs) + expect(remote_repository).to receive(:fetch_env).and_return(fake_env) + expect(new_repository).to receive(:fetch_remote).with(anything, env: fake_env) + expect(new_repository).to receive(:remove_remote).with(anything) + + subject + end + end + + describe '#raw_changes_between' do + let(:old_rev) { } + let(:new_rev) { } + let(:changes) { repository.raw_changes_between(old_rev, new_rev) } + + context 'initial commit' do + let(:old_rev) { Gitlab::Git::BLANK_SHA } + let(:new_rev) { '1a0b36b3cdad1d2ee32457c102a8c0b7056fa863' } + + it 'returns the changes' do + expect(changes).to be_present + expect(changes.size).to eq(3) + end + end + + context 'with an invalid rev' do + let(:old_rev) { 'foo' } + let(:new_rev) { 'bar' } + + it 'returns an error' do + expect { changes }.to raise_error(Gitlab::Git::Repository::GitError) + end + end + + context 'with valid revs' do + let(:old_rev) { 'fa1b1e6c004a68b7d8763b86455da9e6b23e36d6' } + let(:new_rev) { '4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6' } + + it 'returns the changes' do + expect(changes.size).to eq(9) + expect(changes.first.operation).to eq(:modified) + expect(changes.first.new_path).to eq('.gitmodules') + expect(changes.last.operation).to eq(:added) + expect(changes.last.new_path).to eq('files/lfs/picture-invalid.png') + end + end + end + + describe '#merge_base' do + where(:from, :to, :result) do + '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' | '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' + '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' + '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | 'foobar' | nil + 'foobar' | '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | nil + end + + with_them do + it { expect(repository.merge_base(from, to)).to eq(result) } + end + end + + describe '#find_branch' do + it 'should return a Branch for master' do + branch = repository.find_branch('master') + + expect(branch).to be_a_kind_of(Gitlab::Git::Branch) + expect(branch.name).to eq('master') + end + + it 'should handle non-existent branch' do + branch = repository.find_branch('this-is-garbage') + + expect(branch).to eq(nil) + end + end + + describe '#branches' do + subject { repository.branches } + + context 'with local and remote branches' do + let(:repository) { mutable_repository } + + before do + create_remote_branch('joe', 'remote_branch', 'master') + create_branch(repository, 'local_branch', 'master') + end + + it 'returns the local and remote branches' do + expect(subject.any? { |b| b.name == 'joe/remote_branch' }).to eq(true) + expect(subject.any? { |b| b.name == 'local_branch' }).to eq(true) + end + end + end + + describe '#branch_exists?' do + it 'returns true for an existing branch' do + expect(repository.branch_exists?('master')).to eq(true) + end + + it 'returns false for a non-existing branch' do + expect(repository.branch_exists?('kittens')).to eq(false) + end + + it 'returns false when using an invalid branch name' do + expect(repository.branch_exists?('.bla')).to eq(false) + end + end + + describe '#local_branches' do + let(:repository) { mutable_repository } + + before do + create_remote_branch('joe', 'remote_branch', 'master') + create_branch(repository, 'local_branch', 'master') + end + + it 'returns the local branches' do + expect(repository.local_branches.any? { |branch| branch.name == 'remote_branch' }).to eq(false) + expect(repository.local_branches.any? { |branch| branch.name == 'local_branch' }).to eq(true) + end + end + + describe '#fetch_source_branch!' do + let(:local_ref) { 'refs/merge-requests/1/head' } + let(:repository) { mutable_repository } + let(:source_repository) { repository } + + context 'when the branch exists' do + context 'when the commit does not exist locally' do + let(:source_branch) { 'new-branch-for-fetch-source-branch' } + let(:source_path) { File.join(DEFAULT_STORAGE_DIR, source_repository.relative_path) } + let(:source_rugged) { Rugged::Repository.new(source_path) } + let(:new_oid) { new_commit_edit_old_file(source_rugged).oid } + + before do + source_rugged.branches.create(source_branch, new_oid) + end + + it 'writes the ref' do + expect(repository.fetch_source_branch!(source_repository, source_branch, local_ref)).to eq(true) + expect(repository.commit(local_ref).sha).to eq(new_oid) + end + end + + context 'when the commit exists locally' do + let(:source_branch) { 'master' } + let(:expected_oid) { SeedRepo::LastCommit::ID } + + it 'writes the ref' do + # Sanity check: the commit should already exist + expect(repository.commit(expected_oid)).not_to be_nil + + expect(repository.fetch_source_branch!(source_repository, source_branch, local_ref)).to eq(true) + expect(repository.commit(local_ref).sha).to eq(expected_oid) + end + end + end + + context 'when the branch does not exist' do + let(:source_branch) { 'definitely-not-master' } + + it 'does not write the ref' do + expect(repository.fetch_source_branch!(source_repository, source_branch, local_ref)).to eq(false) + expect(repository.commit(local_ref)).to be_nil + end + end + end + + describe '#rm_branch' do + let(:repository) { mutable_repository } + let(:branch_name) { "to-be-deleted-soon" } + + before do + # TODO: project.add_developer(user) + create_branch(repository, branch_name) + end + + it "removes the branch from the repo" do + repository.rm_branch(branch_name, user: user) + + expect(repository_rugged.branches[branch_name]).to be_nil + end + end + + describe '#write_ref' do + context 'validations' do + using RSpec::Parameterized::TableSyntax + + where(:ref_path, :ref) do + 'foo bar' | '123' + 'foobar' | "12\x003" + end + + with_them do + it 'raises ArgumentError' do + expect { repository.write_ref(ref_path, ref) }.to raise_error(ArgumentError) + end + end + end + end + + describe '#write_config' do + before do + repository_rugged.config["gitlab.fullpath"] = repository_path + end + + context 'is given a path' do + it 'writes it to disk' do + repository.write_config(full_path: "not-the/real-path.git") + + config = File.read(File.join(repository_path, "config")) + + expect(config).to include("[gitlab]") + expect(config).to include("fullpath = not-the/real-path.git") + end + end + + context 'it is given an empty path' do + it 'does not write it to disk' do + repository.write_config(full_path: "") + + config = File.read(File.join(repository_path, "config")) + + expect(config).to include("[gitlab]") + expect(config).to include("fullpath = #{repository_path}") + end + end + end + + describe '#merge' do + let(:repository) { mutable_repository } + let(:source_sha) { '913c66a37b4a45b9769037c55c2d238bd0942d2e' } + let(:target_branch) { 'test-merge-target-branch' } + + before do + create_branch(repository, target_branch, '6d394385cf567f80a8fd85055db1ab4c5295806f') + end + + it 'can perform a merge' do + merge_commit_id = nil + result = repository.merge(user, source_sha, target_branch, 'Test merge') do |commit_id| + merge_commit_id = commit_id + end + + expect(result.newrev).to eq(merge_commit_id) + expect(result.repo_created).to eq(false) + expect(result.branch_created).to eq(false) + end + + it 'returns nil if there was a concurrent branch update' do + concurrent_update_id = '33f3729a45c02fc67d00adb1b8bca394b0e761d9' + result = repository.merge(user, source_sha, target_branch, 'Test merge') do + # This ref update should make the merge fail + repository.write_ref(Gitlab::Git::BRANCH_REF_PREFIX + target_branch, concurrent_update_id) + end + + # This 'nil' signals that the merge was not applied + expect(result).to be_nil + + # Our concurrent ref update should not have been undone + expect(repository.find_branch(target_branch).target).to eq(concurrent_update_id) + end + end + + describe '#ff_merge' do + let(:repository) { mutable_repository } + let(:branch_head) { '6d394385cf567f80a8fd85055db1ab4c5295806f' } + let(:source_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' } + let(:target_branch) { 'test-ff-target-branch' } + + before do + create_branch(repository, target_branch, branch_head) + end + + subject { repository.ff_merge(user, source_sha, target_branch) } + + it 'performs a ff_merge' do + expect(subject.newrev).to eq(source_sha) + expect(subject.repo_created).to be(false) + expect(subject.branch_created).to be(false) + + expect(repository.commit(target_branch).id).to eq(source_sha) + end + + context 'with a non-existing target branch' do + subject { repository.ff_merge(user, source_sha, 'this-isnt-real') } + + it 'throws an ArgumentError' do + expect { subject }.to raise_error(ArgumentError) + end + end + + context 'with a non-existing source commit' do + let(:source_sha) { 'f001' } + + it 'throws an ArgumentError' do + expect { subject }.to raise_error(ArgumentError) + end + end + + context 'when the source sha is not a descendant of the branch head' do + let(:source_sha) { '1a0b36b3cdad1d2ee32457c102a8c0b7056fa863' } + + it "doesn't perform the ff_merge" do + expect { subject }.to raise_error(Gitlab::Git::CommitError) + + expect(repository.commit(target_branch).id).to eq(branch_head) + end + end + end + + describe '#delete_all_refs_except' do + let(:repository) { mutable_repository } + + before do + repository.write_ref("refs/delete/a", "0b4bc9a49b562e85de7cc9e834518ea6828729b9") + repository.write_ref("refs/also-delete/b", "12d65c8dd2b2676fa3ac47d955accc085a37a9c1") + repository.write_ref("refs/keep/c", "6473c90867124755509e100d0d35ebdc85a0b6ae") + repository.write_ref("refs/also-keep/d", "0b4bc9a49b562e85de7cc9e834518ea6828729b9") + end + + it 'deletes all refs except those with the specified prefixes' do + repository.delete_all_refs_except(%w(refs/keep refs/also-keep refs/heads)) + expect(repository_rugged.references.exist?("refs/delete/a")).to be(false) + expect(repository_rugged.references.exist?("refs/also-delete/b")).to be(false) + expect(repository_rugged.references.exist?("refs/keep/c")).to be(true) + expect(repository_rugged.references.exist?("refs/also-keep/d")).to be(true) + expect(repository_rugged.references.exist?("refs/heads/master")).to be(true) + end + end + + describe 'remotes' do + let(:repository) { mutable_repository } + let(:remote_name) { 'my-remote' } + let(:url) { 'http://my-repo.git' } + + describe '#add_remote' do + let(:mirror_refmap) { '+refs/*:refs/*' } + + it 'added the remote' do + begin + repository_rugged.remotes.delete(remote_name) + rescue Rugged::ConfigError + end + + repository.add_remote(remote_name, url, mirror_refmap: mirror_refmap) + + expect(repository_rugged.remotes[remote_name]).not_to be_nil + expect(repository_rugged.config["remote.#{remote_name}.mirror"]).to eq('true') + expect(repository_rugged.config["remote.#{remote_name}.prune"]).to eq('true') + expect(repository_rugged.config["remote.#{remote_name}.fetch"]).to eq(mirror_refmap) + end + end + + describe '#remove_remote' do + it 'removes the remote' do + repository_rugged.remotes.create(remote_name, url) + + repository.remove_remote(remote_name) + + expect(repository_rugged.remotes[remote_name]).to be_nil + end + end + end + + describe '#squash' do + let(:repository) { mutable_repository } + let(:squash_id) { '1' } + let(:branch_name) { 'fix' } + let(:start_sha) { '4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6' } + let(:end_sha) { '12d65c8dd2b2676fa3ac47d955accc085a37a9c1' } + + subject do + opts = { + branch: branch_name, + start_sha: start_sha, + end_sha: end_sha, + author: user, + message: 'Squash commit message' + } + + repository.squash(user, squash_id, opts) + end + + describe 'sparse checkout' do + let(:expected_files) { %w(files files/js files/js/application.js) } + + it 'checks out only the files in the diff' do + allow(repository).to receive(:with_worktree).and_wrap_original do |m, *args| + m.call(*args) do + worktree_path = args[0] + files_pattern = File.join(worktree_path, '**', '*') + expected = expected_files.map do |path| + File.expand_path(path, worktree_path) + end + + expect(Dir[files_pattern]).to eq(expected) + end + end + + subject + end + + context 'when the diff contains a rename' do + let(:end_sha) { new_commit_move_file(repository_rugged).oid } + + after do + # Erase our commits so other tests get the original repo + repository_rugged.references.update('refs/heads/master', SeedRepo::LastCommit::ID) + end + + it 'does not include the renamed file in the sparse checkout' do + allow(repository).to receive(:with_worktree).and_wrap_original do |m, *args| + m.call(*args) do + worktree_path = args[0] + files_pattern = File.join(worktree_path, '**', '*') + + expect(Dir[files_pattern]).not_to include('CHANGELOG') + expect(Dir[files_pattern]).not_to include('encoding/CHANGELOG') + end + end + + subject + end + end + end + + describe 'with an ASCII-8BIT diff' do + let(:diff) { "diff --git a/README.md b/README.md\nindex faaf198..43c5edf 100644\n--- a/README.md\n+++ b/README.md\n@@ -1,4 +1,4 @@\n-testme\n+✓ testme\n ======\n \n Sample repo for testing gitlab features\n" } + + it 'applies a ASCII-8BIT diff' do + allow(repository).to receive(:run_git!).and_call_original + allow(repository).to receive(:run_git!).with(%W(diff --binary #{start_sha}...#{end_sha})).and_return(diff.force_encoding('ASCII-8BIT')) + + expect(subject).to match(/\h{40}/) + end + end + + describe 'with trailing whitespace in an invalid patch' do + let(:diff) { "diff --git a/README.md b/README.md\nindex faaf198..43c5edf 100644\n--- a/README.md\n+++ b/README.md\n@@ -1,4 +1,4 @@\n-testme\n+ \n ====== \n \n Sample repo for testing gitlab features\n" } + + it 'does not include whitespace warnings in the error' do + allow(repository).to receive(:run_git!).and_call_original + allow(repository).to receive(:run_git!).with(%W(diff --binary #{start_sha}...#{end_sha})).and_return(diff.force_encoding('ASCII-8BIT')) + + expect { subject }.to raise_error do |error| + expect(error).to be_a(described_class::GitError) + expect(error.message).not_to include('trailing whitespace') + end + end + end + end describe '#from_gitaly_with_block' do let(:call_metadata) do @@ -112,4 +740,69 @@ describe Gitlab::Git::Repository do end end end + + def create_remote_branch(remote_name, branch_name, source_branch_name) + source_branch = repository.branches.find { |branch| branch.name == source_branch_name } + repository_rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", source_branch.dereferenced_target.sha) + end + + # Build the options hash that's passed to Rugged::Commit#create + def commit_options(repo, index, message) + options = {} + options[:tree] = index.write_tree(repo) + options[:author] = { + email: "test@example.com", + name: "Test Author", + time: Time.gm(2014, "mar", 3, 20, 15, 1) + } + options[:committer] = { + email: "test@example.com", + name: "Test Author", + time: Time.gm(2014, "mar", 3, 20, 15, 1) + } + options[:message] ||= message + options[:parents] = repo.empty? ? [] : [repo.head.target].compact + options[:update_ref] = "HEAD" + + options + end + + # Writes a new commit to the repo and returns a Rugged::Commit. Replaces the + # contents of CHANGELOG with a single new line of text. + def new_commit_edit_old_file(repo) + oid = repo.write("I replaced the changelog with this text", :blob) + index = repo.index + index.read_tree(repo.head.target.tree) + index.add(path: "CHANGELOG", oid: oid, mode: 0100644) + + options = commit_options( + repo, + index, + "Edit CHANGELOG in its original location" + ) + + sha = Rugged::Commit.create(repo, options) + repo.lookup(sha) + end + + # Writes a new commit to the repo and returns a Rugged::Commit. Moves the + # CHANGELOG file to the encoding/ directory. + def new_commit_move_file(repo) + blob_oid = repo.head.target.tree.detect { |i| i[:name] == "CHANGELOG" }[:oid] + file_content = repo.lookup(blob_oid).content + oid = repo.write(file_content, :blob) + index = repo.index + index.read_tree(repo.head.target.tree) + index.add(path: "encoding/CHANGELOG", oid: oid, mode: 0100644) + index.remove("CHANGELOG") + + options = commit_options(repo, index, "Move CHANGELOG to encoding/") + + sha = Rugged::Commit.create(repo, options) + repo.lookup(sha) + end + + def create_branch(repository, branch_name, start_point = 'HEAD') + repository.rugged.branches.create(branch_name, start_point) + end end diff --git a/ruby/spec/lib/gitlab/git/wiki_spec.rb b/ruby/spec/lib/gitlab/git/wiki_spec.rb index baf07c0168d..08292df97d5 100644 --- a/ruby/spec/lib/gitlab/git/wiki_spec.rb +++ b/ruby/spec/lib/gitlab/git/wiki_spec.rb @@ -4,7 +4,6 @@ describe Gitlab::Git::Wiki do include TestRepo let(:repository) { gitlab_git_from_gitaly(new_empty_test_repo) } - let(:user) { project.owner } subject { described_class.new(repository) } diff --git a/ruby/spec/spec_helper.rb b/ruby/spec/spec_helper.rb index 5e83fd5a002..39b848e9756 100644 --- a/ruby/spec/spec_helper.rb +++ b/ruby/spec/spec_helper.rb @@ -6,9 +6,13 @@ require 'test_repo_helper' require 'rspec-parameterized' require 'factory_bot' +# Load these helpers first, since they're required by other helpers +require File.join(__dir__, 'support/helpers/gitlab_shell_helper.rb') + Dir[File.join(__dir__, 'support/helpers/*.rb')].each { |f| require f } ENV['GITALY_RUBY_GIT_BIN_PATH'] ||= 'git' +ENV['GITALY_RUBY_GITALY_BIN_DIR'] = __dir__ RSpec.configure do |config| config.include FactoryBot::Syntax::Methods diff --git a/ruby/spec/support/helpers/gitlab_shell_helper.rb b/ruby/spec/support/helpers/gitlab_shell_helper.rb new file mode 100644 index 00000000000..d2fb8da4330 --- /dev/null +++ b/ruby/spec/support/helpers/gitlab_shell_helper.rb @@ -0,0 +1,13 @@ +require 'spec_helper' + +GITALY_RUBY_DIR = File.expand_path('../../../..', __FILE__) +TMP_DIR = File.join(GITALY_RUBY_DIR, 'tmp') +GITLAB_SHELL_DIR = File.join(TMP_DIR, 'gitlab-shell') + +module GitlabShellHelper + def self.setup_gitlab_shell + ENV['GITALY_RUBY_GITLAB_SHELL_PATH'] = GITLAB_SHELL_DIR + + FileUtils.mkdir_p([TMP_DIR, File.join(GITLAB_SHELL_DIR, 'hooks')]) + end +end diff --git a/ruby/spec/integration_helper.rb b/ruby/spec/support/helpers/integration_helper.rb similarity index 75% rename from ruby/spec/integration_helper.rb rename to ruby/spec/support/helpers/integration_helper.rb index 9b630051268..f77f0fcdfac 100644 --- a/ruby/spec/integration_helper.rb +++ b/ruby/spec/support/helpers/integration_helper.rb @@ -4,13 +4,11 @@ require 'gitaly' require 'spec_helper' SOCKET_PATH = 'gitaly.socket'.freeze -GITALY_RUBY_DIR = File.expand_path('../..', __FILE__) -TMP_DIR = File.expand_path('../../tmp', __FILE__) module IntegrationClient def gitaly_stub(service) klass = Gitaly.const_get(service).const_get(:Stub) - klass.new("unix:tmp/#{SOCKET_PATH}", :this_channel_is_insecure) + klass.new("unix:#{File.join(TMP_DIR, SOCKET_PATH)}", :this_channel_is_insecure) end def gitaly_repo(storage, relative_path) @@ -19,18 +17,16 @@ module IntegrationClient end def start_gitaly - build_dir = File.expand_path('../../../_build', __FILE__) - gitlab_shell_dir = File.join(TMP_DIR, 'gitlab-shell') - - FileUtils.mkdir_p([TMP_DIR, File.join(gitlab_shell_dir, 'hooks')]) + build_dir = File.expand_path(File.join(GITALY_RUBY_DIR, '../_build')) + GitlabShellHelper.setup_gitlab_shell config_toml = <<~CONFIG socket_path = "#{SOCKET_PATH}" bin_dir = "#{build_dir}/bin" - + [gitlab-shell] - dir = "#{gitlab_shell_dir}" - + dir = "#{GITLAB_SHELL_DIR}" + [gitaly-ruby] dir = "#{GITALY_RUBY_DIR}" @@ -47,7 +43,7 @@ def start_gitaly gitaly_pid = spawn(File.join(build_dir, 'bin/gitaly'), config_path, options) at_exit { Process.kill('KILL', gitaly_pid) } - wait_ready!(File.join('tmp', SOCKET_PATH)) + wait_ready!(File.join(TMP_DIR, SOCKET_PATH)) end def wait_ready!(socket) diff --git a/ruby/spec/test_repo_helper.rb b/ruby/spec/test_repo_helper.rb index e78f8884cab..642d7c4fb30 100644 --- a/ruby/spec/test_repo_helper.rb +++ b/ruby/spec/test_repo_helper.rb @@ -40,6 +40,12 @@ module TestRepo Gitaly::Repository.new(storage_name: DEFAULT_STORAGE_NAME, relative_path: relative_path) end + def new_mutable_git_test_repo + relative_path = random_repository_relative_path(:mutable) + TestRepo.clone_new_repo!(GIT_TEST_REPO_ORIGIN, File.join(DEFAULT_STORAGE_DIR, relative_path)) + Gitaly::Repository.new(storage_name: DEFAULT_STORAGE_NAME, relative_path: relative_path) + end + def new_broken_test_repo relative_path = random_repository_relative_path(:broken) repo_path = File.join(DEFAULT_STORAGE_DIR, relative_path) -- GitLab From 5b85a26399aabce57d655f75c10e810c52b86911 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Sat, 13 Oct 2018 13:02:37 -0300 Subject: [PATCH 6/9] Add tag spec examples and remove unused code --- ruby/lib/gitlab/git/tag.rb | 41 +------------------ ruby/spec/lib/gitlab/git/blob_spec.rb | 2 +- ruby/spec/lib/gitlab/git/branch_spec.rb | 2 +- ruby/spec/lib/gitlab/git/commit_spec.rb | 2 +- .../lib/gitlab/git/remote_repository_spec.rb | 2 +- ruby/spec/lib/gitlab/git/repository_spec.rb | 22 ++++++++++ 6 files changed, 27 insertions(+), 44 deletions(-) diff --git a/ruby/lib/gitlab/git/tag.rb b/ruby/lib/gitlab/git/tag.rb index 706b12b5fc4..1cb150f84fb 100644 --- a/ruby/lib/gitlab/git/tag.rb +++ b/ruby/lib/gitlab/git/tag.rb @@ -7,33 +7,10 @@ module Gitlab attr_reader :object_sha, :repository - MAX_TAG_MESSAGE_DISPLAY_SIZE = 10.megabytes SERIALIZE_KEYS = %i[name target target_commit message].freeze attr_accessor *SERIALIZE_KEYS # rubocop:disable Lint/AmbiguousOperator - class << self - def get_message(repository, tag_id) - BatchLoader.for({ repository: repository, tag_id: tag_id }).batch do |items, loader| - items_by_repo = items.group_by { |i| i[:repository] } - - items_by_repo.each do |repo, items| - tag_ids = items.map { |i| i[:tag_id] } - - messages = get_messages(repository, tag_ids) - - messages.each do |id, message| - loader.call({ repository: repository, tag_id: id }, message) - end - end - end - end - - def get_messages(repository, tag_ids) - repository.gitaly_ref_client.get_tag_messages(tag_ids) - end - end - def initialize(repository, raw_tag) @repository = repository @raw_tag = raw_tag @@ -59,7 +36,7 @@ module Gitlab def init_from_gitaly @name = encode!(@raw_tag.name.dup) @target = @raw_tag.id - @message = message_from_gitaly_tag + @message = @raw_tag.message.dup if @raw_tag.target_commit.present? @target_commit = Gitlab::Git::Commit.decorate(repository, @raw_tag.target_commit) @@ -69,22 +46,6 @@ module Gitlab def message encode! @message end - - private - - def message_from_gitaly_tag - return @raw_tag.message.dup if full_message_fetched_from_gitaly? - - if @raw_tag.message_size > MAX_TAG_MESSAGE_DISPLAY_SIZE - '--tag message is too big' - else - self.class.get_message(@repository, target) - end - end - - def full_message_fetched_from_gitaly? - @raw_tag.message.bytesize == @raw_tag.message_size - end end end end diff --git a/ruby/spec/lib/gitlab/git/blob_spec.rb b/ruby/spec/lib/gitlab/git/blob_spec.rb index 41bb41a1893..165cdf3ae47 100644 --- a/ruby/spec/lib/gitlab/git/blob_spec.rb +++ b/ruby/spec/lib/gitlab/git/blob_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -describe Gitlab::Git::Blob, :seed_helper do +describe Gitlab::Git::Blob do include TestRepo let(:repository) { gitlab_git_from_gitaly(git_test_repo_read_only) } diff --git a/ruby/spec/lib/gitlab/git/branch_spec.rb b/ruby/spec/lib/gitlab/git/branch_spec.rb index 34453423877..c32f72f2247 100644 --- a/ruby/spec/lib/gitlab/git/branch_spec.rb +++ b/ruby/spec/lib/gitlab/git/branch_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -describe Gitlab::Git::Branch, :seed_helper do +describe Gitlab::Git::Branch do include TestRepo let(:repository) { gitlab_git_from_gitaly(git_test_repo_read_only) } diff --git a/ruby/spec/lib/gitlab/git/commit_spec.rb b/ruby/spec/lib/gitlab/git/commit_spec.rb index 97b02c2083c..ce3c19f8a74 100644 --- a/ruby/spec/lib/gitlab/git/commit_spec.rb +++ b/ruby/spec/lib/gitlab/git/commit_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -describe Gitlab::Git::Commit, :seed_helper do +describe Gitlab::Git::Commit do include TestRepo let(:repository) { gitlab_git_from_gitaly(git_test_repo_read_only) } diff --git a/ruby/spec/lib/gitlab/git/remote_repository_spec.rb b/ruby/spec/lib/gitlab/git/remote_repository_spec.rb index 2788f482b7f..df946859426 100644 --- a/ruby/spec/lib/gitlab/git/remote_repository_spec.rb +++ b/ruby/spec/lib/gitlab/git/remote_repository_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Git::RemoteRepository, :seed_helper do +describe Gitlab::Git::RemoteRepository do include TestRepo let(:repository) { gitlab_git_from_gitaly(git_test_repo_read_only) } diff --git a/ruby/spec/lib/gitlab/git/repository_spec.rb b/ruby/spec/lib/gitlab/git/repository_spec.rb index 8401a871119..4a4caa6adb2 100644 --- a/ruby/spec/lib/gitlab/git/repository_spec.rb +++ b/ruby/spec/lib/gitlab/git/repository_spec.rb @@ -98,6 +98,28 @@ describe Gitlab::Git::Repository do it { is_expected.not_to include("branch-from-space") } end + describe '#tags' do + describe 'first tag' do + let(:tag) { repository.tags.first } + + it { expect(tag.name).to eq("v1.0.0") } + it { expect(tag.target).to eq("f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8") } + it { expect(tag.dereferenced_target.sha).to eq("6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9") } + it { expect(tag.message).to eq("Release") } + end + + describe 'last tag' do + let(:tag) { repository.tags.last } + + it { expect(tag.name).to eq("v1.2.1") } + it { expect(tag.target).to eq("2ac1f24e253e08135507d0830508febaaccf02ee") } + it { expect(tag.dereferenced_target.sha).to eq("fa1b1e6c004a68b7d8763b86455da9e6b23e36d6") } + it { expect(tag.message).to eq("Version 1.2.1") } + end + + it { expect(repository.tags.size).to eq(SeedRepo::Repo::TAGS.size) } + end + describe '#tag_names' do subject { repository.tag_names } -- GitLab From 78fad676686f6c9e6ad8f3723a8250dc594f0dd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Mon, 15 Oct 2018 00:00:48 -0300 Subject: [PATCH 7/9] Add user spec examples and remove unused code --- ruby/lib/gitlab/git/operation_service.rb | 5 +--- ruby/lib/gitlab/git/repository.rb | 2 -- ruby/lib/gitlab/git/user.rb | 8 ----- ruby/spec/lib/gitlab/git/user_spec.rb | 37 ++++++++++++++++++++++++ 4 files changed, 38 insertions(+), 14 deletions(-) create mode 100644 ruby/spec/lib/gitlab/git/user_spec.rb diff --git a/ruby/lib/gitlab/git/operation_service.rb b/ruby/lib/gitlab/git/operation_service.rb index 99ee18166d3..eaa9ffea7b9 100644 --- a/ruby/lib/gitlab/git/operation_service.rb +++ b/ruby/lib/gitlab/git/operation_service.rb @@ -21,10 +21,7 @@ module Gitlab attr_reader :user, :repository def initialize(user, new_repository) - if user - user = Gitlab::Git::User.from_gitlab(user) unless user.respond_to?(:gl_id) - @user = user - end + @user = user # Refactoring aid Gitlab::Git.check_namespace!(new_repository) diff --git a/ruby/lib/gitlab/git/repository.rb b/ruby/lib/gitlab/git/repository.rb index 6e0351d45db..a89d10f56af 100644 --- a/ruby/lib/gitlab/git/repository.rb +++ b/ruby/lib/gitlab/git/repository.rb @@ -336,8 +336,6 @@ module Gitlab target_object = Ref.dereference_object(lookup(target)) raise InvalidRef.new("target not found: #{target}") unless target_object - user = Gitlab::Git::User.from_gitlab(user) unless user.respond_to?(:gl_id) - options = nil # Use nil, not the empty hash. Rugged cares about this. if message options = { diff --git a/ruby/lib/gitlab/git/user.rb b/ruby/lib/gitlab/git/user.rb index e573cd0e143..02b095d9413 100644 --- a/ruby/lib/gitlab/git/user.rb +++ b/ruby/lib/gitlab/git/user.rb @@ -3,10 +3,6 @@ module Gitlab class User attr_reader :username, :name, :email, :gl_id - def self.from_gitlab(gitlab_user) - new(gitlab_user.username, gitlab_user.name, gitlab_user.email, Gitlab::GlId.gl_id(gitlab_user)) - end - def self.from_gitaly(gitaly_user) new( gitaly_user.gl_username, @@ -26,10 +22,6 @@ module Gitlab def ==(other) [username, name, email, gl_id] == [other.username, other.name, other.email, other.gl_id] end - - def to_gitaly - Gitaly::User.new(gl_username: username, gl_id: gl_id, name: name.b, email: email.b) - end end end end diff --git a/ruby/spec/lib/gitlab/git/user_spec.rb b/ruby/spec/lib/gitlab/git/user_spec.rb new file mode 100644 index 00000000000..ae3f8d6cc03 --- /dev/null +++ b/ruby/spec/lib/gitlab/git/user_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Gitlab::Git::User do + let(:username) { 'janedoe' } + let(:name) { 'Jane Doé' } + let(:email) { 'janedoé@example.com' } + let(:gl_id) { 'user-123' } + let(:user) do + described_class.new(username, name, email, gl_id) + end + + subject { described_class.new(username, name, email, gl_id) } + + describe '.from_gitaly' do + let(:gitaly_user) do + Gitaly::User.new(gl_username: username, name: name.b, email: email.b, gl_id: gl_id) + end + + subject { described_class.from_gitaly(gitaly_user) } + + it { expect(subject).to eq(user) } + end + + describe '#==' do + def eq_other(username, name, email, gl_id) + eq(described_class.new(username, name, email, gl_id)) + end + + it { expect(subject).to eq_other(username, name, email, gl_id) } + + it { expect(subject).not_to eq_other(nil, nil, nil, nil) } + it { expect(subject).not_to eq_other(username + 'x', name, email, gl_id) } + it { expect(subject).not_to eq_other(username, name + 'x', email, gl_id) } + it { expect(subject).not_to eq_other(username, name, email + 'x', gl_id) } + it { expect(subject).not_to eq_other(username, name, email, gl_id + 'x') } + end +end -- GitLab From 8f2b01a8a943f8fe06fe1c7b6938b152a930423f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Mon, 15 Oct 2018 00:01:26 -0300 Subject: [PATCH 8/9] Add LfsChanges and RawDiff spec examples --- ruby/spec/lib/gitlab/git/lfs_changes_spec.rb | 21 ++++++ .../lib/gitlab/git/raw_diff_change_spec.rb | 68 +++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 ruby/spec/lib/gitlab/git/lfs_changes_spec.rb create mode 100644 ruby/spec/lib/gitlab/git/raw_diff_change_spec.rb diff --git a/ruby/spec/lib/gitlab/git/lfs_changes_spec.rb b/ruby/spec/lib/gitlab/git/lfs_changes_spec.rb new file mode 100644 index 00000000000..3a5c673beb6 --- /dev/null +++ b/ruby/spec/lib/gitlab/git/lfs_changes_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Gitlab::Git::LfsChanges do + include TestRepo + + let(:repository) { gitlab_git_from_gitaly(new_mutable_test_repo) } + let(:newrev) { '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51' } + let(:blob_object_id) { '0c304a93cb8430108629bbbcaa27db3343299bc0' } + + subject { described_class.new(repository, newrev) } + + describe '#new_pointers' do + it 'filters new objects to find lfs pointers' do + expect(subject.new_pointers(not_in: []).first.id).to eq(blob_object_id) + end + + it 'limits new_objects using object_limit' do + expect(subject.new_pointers(object_limit: 1)).to eq([]) + end + end +end diff --git a/ruby/spec/lib/gitlab/git/raw_diff_change_spec.rb b/ruby/spec/lib/gitlab/git/raw_diff_change_spec.rb new file mode 100644 index 00000000000..c3a1cd5e33a --- /dev/null +++ b/ruby/spec/lib/gitlab/git/raw_diff_change_spec.rb @@ -0,0 +1,68 @@ +require 'spec_helper' + +describe Gitlab::Git::RawDiffChange do + let(:raw_change) { } + let(:old_mode) { 0o100644 } + let(:new_mode) { 0o100644 } + let(:change) { described_class.new(raw_change, old_mode, new_mode) } + + context 'bad input' do + let(:raw_change) { 'foo' } + + it 'does not set most of the attrs' do + expect(change.blob_id).to eq('foo') + expect(change.operation).to eq(:unknown) + expect(change.old_path).to be_blank + expect(change.new_path).to be_blank + expect(change.blob_size).to eq(0) + end + end + + context 'adding a file' do + let(:raw_change) { '93e123ac8a3e6a0b600953d7598af629dec7b735 59 A bar/branch-test.txt' } + + it 'initialize the proper attrs' do + expect(change.operation).to eq(:added) + expect(change.old_path).to be_blank + expect(change.new_path).to eq('bar/branch-test.txt') + expect(change.blob_id).to be_present + expect(change.blob_size).to be_present + end + end + + context 'renaming a file' do + let(:raw_change) { "85bc2f9753afd5f4fc5d7c75f74f8d526f26b4f3 107 R060\tfiles/js/commit.js.coffee\tfiles/js/commit.coffee" } + + it 'initialize the proper attrs' do + expect(change.operation).to eq(:renamed) + expect(change.old_path).to eq('files/js/commit.js.coffee') + expect(change.new_path).to eq('files/js/commit.coffee') + expect(change.blob_id).to be_present + expect(change.blob_size).to be_present + end + end + + context 'modifying a file' do + let(:raw_change) { 'c60514b6d3d6bf4bec1030f70026e34dfbd69ad5 824 M README.md' } + + it 'initialize the proper attrs' do + expect(change.operation).to eq(:modified) + expect(change.old_path).to eq('README.md') + expect(change.new_path).to eq('README.md') + expect(change.blob_id).to be_present + expect(change.blob_size).to be_present + end + end + + context 'deleting a file' do + let(:raw_change) { '60d7a906c2fd9e4509aeb1187b98d0ea7ce827c9 15364 D files/.DS_Store' } + + it 'initialize the proper attrs' do + expect(change.operation).to eq(:deleted) + expect(change.old_path).to eq('files/.DS_Store') + expect(change.new_path).to be_nil + expect(change.blob_id).to be_present + expect(change.blob_size).to be_present + end + end +end -- GitLab From f7dfc8ce21561b3c4149a593b600b1b4143b1976 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Mon, 15 Oct 2018 00:57:03 -0300 Subject: [PATCH 9/9] Add changelog for Gitlab::Git spec import from gitlab-ce Had to fix the changelog file from another branch that used this branch's name --- changelogs/unreleased/gitlab-git-unit-tests.yml | 4 ++-- changelogs/unreleased/loc-fixes.yml | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/loc-fixes.yml diff --git a/changelogs/unreleased/gitlab-git-unit-tests.yml b/changelogs/unreleased/gitlab-git-unit-tests.yml index 6bfca1c8535..e32a3df3212 100644 --- a/changelogs/unreleased/gitlab-git-unit-tests.yml +++ b/changelogs/unreleased/gitlab-git-unit-tests.yml @@ -1,5 +1,5 @@ --- -title: Force english output on git commands -merge_request: 898 +title: Port unit tests for Gitlab::Git from gitlab-ce +merge_request: 914 author: type: other diff --git a/changelogs/unreleased/loc-fixes.yml b/changelogs/unreleased/loc-fixes.yml new file mode 100644 index 00000000000..6bfca1c8535 --- /dev/null +++ b/changelogs/unreleased/loc-fixes.yml @@ -0,0 +1,5 @@ +--- +title: Force english output on git commands +merge_request: 898 +author: +type: other -- GitLab