diff options
Diffstat (limited to 'spec/models')
25 files changed, 309 insertions, 122 deletions
diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index c4486a32082..4e71597521d 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' RSpec.describe AbuseReport, type: :model do subject { create(:abuse_report) } - let(:user) { create(:user) } + let(:user) { create(:admin) } it { expect(subject).to be_valid } diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index b950fcdd81a..01ca1584ed2 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -29,6 +29,40 @@ describe ApplicationSetting, models: true do it { is_expected.not_to allow_value(['test']).for(:disabled_oauth_sign_in_sources) } end + describe 'default_artifacts_expire_in' do + it 'sets an error if it cannot parse' do + setting.update(default_artifacts_expire_in: 'a') + + expect_invalid + end + + it 'sets an error if it is blank' do + setting.update(default_artifacts_expire_in: ' ') + + expect_invalid + end + + it 'sets the value if it is valid' do + setting.update(default_artifacts_expire_in: '30 days') + + expect(setting).to be_valid + expect(setting.default_artifacts_expire_in).to eq('30 days') + end + + it 'sets the value if it is 0' do + setting.update(default_artifacts_expire_in: '0') + + expect(setting).to be_valid + expect(setting.default_artifacts_expire_in).to eq('0') + end + + def expect_invalid + expect(setting).to be_invalid + expect(setting.errors.messages) + .to have_key(:default_artifacts_expire_in) + end + end + it { is_expected.to validate_presence_of(:max_attachment_size) } it do @@ -62,9 +96,9 @@ describe ApplicationSetting, models: true do describe 'inclusion' do it { is_expected.to allow_value('custom1').for(:repository_storages) } - it { is_expected.to allow_value(['custom2', 'custom3']).for(:repository_storages) } + it { is_expected.to allow_value(%w(custom2 custom3)).for(:repository_storages) } it { is_expected.not_to allow_value('alternative').for(:repository_storages) } - it { is_expected.not_to allow_value(['alternative', 'custom1']).for(:repository_storages) } + it { is_expected.not_to allow_value(%w(alternative custom1)).for(:repository_storages) } end describe 'presence' do @@ -83,7 +117,7 @@ describe ApplicationSetting, models: true do describe '#repository_storage' do it 'returns the first storage' do - setting.repository_storages = ['good', 'bad'] + setting.repository_storages = %w(good bad) expect(setting.repository_storage).to eq('good') end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 2dfca8bcfce..b963ca4e542 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -162,11 +162,17 @@ describe Ci::Build, :models do is_expected.to be_nil end - it 'when resseting value' do + it 'when resetting value' do build.artifacts_expire_in = nil is_expected.to be_nil end + + it 'when setting to 0' do + build.artifacts_expire_in = '0' + + is_expected.to be_nil + end end describe '#commit' do @@ -1239,8 +1245,8 @@ describe Ci::Build, :models do { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true }, { key: 'CI_PROJECT_ID', value: project.id.to_s, public: true }, { key: 'CI_PROJECT_NAME', value: project.path, public: true }, - { key: 'CI_PROJECT_PATH', value: project.path_with_namespace, public: true }, - { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.path, public: true }, + { key: 'CI_PROJECT_PATH', value: project.full_path, public: true }, + { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true }, { key: 'CI_PROJECT_URL', value: project.web_url, public: true }, { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true } ] @@ -1262,8 +1268,8 @@ describe Ci::Build, :models do context 'when build has user' do let(:user_variables) do - [ { key: 'GITLAB_USER_ID', value: user.id.to_s, public: true }, - { key: 'GITLAB_USER_EMAIL', value: user.email, public: true } ] + [{ key: 'GITLAB_USER_ID', value: user.id.to_s, public: true }, + { key: 'GITLAB_USER_EMAIL', value: user.email, public: true }] end before do @@ -1420,7 +1426,7 @@ describe Ci::Build, :models do end context 'when runner is assigned to build' do - let(:runner) { create(:ci_runner, description: 'description', tag_list: ['docker', 'linux']) } + let(:runner) { create(:ci_runner, description: 'description', tag_list: %w(docker linux)) } before do build.update(runner: runner) diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 10c2bfbb400..c2fc8c02bb3 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -168,9 +168,9 @@ describe Ci::Pipeline, models: true do end it 'returns list of stages with correct statuses' do - expect(statuses).to eq([['build', 'failed'], - ['test', 'success'], - ['deploy', 'running']]) + expect(statuses).to eq([%w(build failed), + %w(test success), + %w(deploy running)]) end context 'when commit status is retried' do @@ -183,9 +183,9 @@ describe Ci::Pipeline, models: true do end it 'ignores the previous state' do - expect(statuses).to eq([['build', 'success'], - ['test', 'success'], - ['deploy', 'running']]) + expect(statuses).to eq([%w(build success), + %w(test success), + %w(deploy running)]) end end end @@ -199,7 +199,7 @@ describe Ci::Pipeline, models: true do describe '#stages_name' do it 'returns a valid names of stages' do - expect(pipeline.stages_name).to eq(['build', 'test', 'deploy']) + expect(pipeline.stages_name).to eq(%w(build test deploy)) end end end @@ -767,7 +767,7 @@ describe Ci::Pipeline, models: true do end it 'cancels created builds' do - expect(latest_status).to eq ['canceled', 'canceled'] + expect(latest_status).to eq %w(canceled canceled) end end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index f8513ac8b1c..76ce558eea0 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -113,7 +113,7 @@ describe Ci::Runner, models: true do context 'when runner has tags' do before do - runner.tag_list = ['bb', 'cc'] + runner.tag_list = %w(bb cc) end shared_examples 'tagged build picker' do @@ -169,7 +169,7 @@ describe Ci::Runner, models: true do context 'when having runner tags' do before do - runner.tag_list = ['bb', 'cc'] + runner.tag_list = %w(bb cc) end it 'cannot handle it for builds without matching tags' do @@ -189,7 +189,7 @@ describe Ci::Runner, models: true do context 'when having runner tags' do before do - runner.tag_list = ['bb', 'cc'] + runner.tag_list = %w(bb cc) build.tag_list = ['bb'] end @@ -212,7 +212,7 @@ describe Ci::Runner, models: true do context 'when having runner tags' do before do - runner.tag_list = ['bb', 'cc'] + runner.tag_list = %w(bb cc) build.tag_list = ['bb'] end diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 2e3702f7520..6151d53cd91 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' describe CacheMarkdownField do - CacheMarkdownField::CACHING_CLASSES << "ThingWithMarkdownFields" + caching_classes = CacheMarkdownField::CACHING_CLASSES + CacheMarkdownField::CACHING_CLASSES = ["ThingWithMarkdownFields"].freeze # The minimum necessary ActiveModel to test this concern class ThingWithMarkdownFields @@ -54,7 +55,7 @@ describe CacheMarkdownField do end end - CacheMarkdownField::CACHING_CLASSES.delete("ThingWithMarkdownFields") + CacheMarkdownField::CACHING_CLASSES = caching_classes def thing_subclass(new_attr) Class.new(ThingWithMarkdownFields) { add_attr(new_attr) } diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index e008ec28fa4..677e60e1282 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -86,7 +86,7 @@ describe Group, 'Routable' do let(:nested_group) { create(:group, parent: group) } it { expect(group.full_path).to eq(group.path) } - it { expect(nested_group.full_path).to eq("#{group.path}/#{nested_group.path}") } + it { expect(nested_group.full_path).to eq("#{group.full_path}/#{nested_group.path}") } end describe '#full_name' do @@ -102,7 +102,7 @@ describe Project, 'Routable' do describe '#full_path' do let(:project) { build_stubbed(:empty_project) } - it { expect(project.full_path).to eq "#{project.namespace.path}/#{project.path}" } + it { expect(project.full_path).to eq "#{project.namespace.full_path}/#{project.path}" } end describe '#full_name' do diff --git a/spec/models/concerns/uniquify_spec.rb b/spec/models/concerns/uniquify_spec.rb new file mode 100644 index 00000000000..83187d732e4 --- /dev/null +++ b/spec/models/concerns/uniquify_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe Uniquify, models: true do + let(:uniquify) { described_class.new } + + describe "#string" do + it 'returns the given string if it does not exist' do + result = uniquify.string('test_string') { |s| false } + + expect(result).to eq('test_string') + end + + it 'returns the given string with a counter attached if the string exists' do + result = uniquify.string('test_string') { |s| s == 'test_string' } + + expect(result).to eq('test_string1') + end + + it 'increments the counter for each candidate string that also exists' do + result = uniquify.string('test_string') { |s| s == 'test_string' || s == 'test_string1' } + + expect(result).to eq('test_string2') + end + + it 'allows passing in a base function that defines the location of the counter' do + result = uniquify.string(-> (counter) { "test_#{counter}_string" }) do |s| + s == 'test__string' + end + + expect(result).to eq('test_1_string') + end + end +end diff --git a/spec/models/cycle_analytics/production_spec.rb b/spec/models/cycle_analytics/production_spec.rb index b9fe492fe2c..e6a826a9418 100644 --- a/spec/models/cycle_analytics/production_spec.rb +++ b/spec/models/cycle_analytics/production_spec.rb @@ -21,13 +21,12 @@ describe 'CycleAnalytics#production', feature: true do ["production deploy happens after merge request is merged (along with other changes)", lambda do |context, data| # Make other changes on master - sha = context.project.repository.commit_file( + sha = context.project.repository.create_file( context.user, context.random_git_name, 'content', message: 'commit message', - branch_name: 'master', - update: false) + branch_name: 'master') context.project.repository.commit(sha) context.deploy_master diff --git a/spec/models/cycle_analytics/staging_spec.rb b/spec/models/cycle_analytics/staging_spec.rb index 9a024d533a1..3a02ed81adb 100644 --- a/spec/models/cycle_analytics/staging_spec.rb +++ b/spec/models/cycle_analytics/staging_spec.rb @@ -18,7 +18,7 @@ describe 'CycleAnalytics#staging', feature: true do start_time_conditions: [["merge request that closes issue is merged", -> (context, data) do context.merge_merge_requests_closing_issue(data[:issue]) - end ]], + end]], end_time_conditions: [["merge request that closes issue is deployed to production", -> (context, data) do context.deploy_master @@ -26,13 +26,12 @@ describe 'CycleAnalytics#staging', feature: true do ["production deploy happens after merge request is merged (along with other changes)", lambda do |context, data| # Make other changes on master - sha = context.project.repository.commit_file( + sha = context.project.repository.create_file( context.user, context.random_git_name, 'content', message: 'commit message', - branch_name: 'master', - update: false) + branch_name: 'master') context.project.repository.commit(sha) context.deploy_master diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index f0ed0c679d5..dce18f008f8 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -76,7 +76,8 @@ describe Environment, models: true do end describe '#update_merge_request_metrics?' do - { 'production' => true, + { + 'production' => true, 'production/eu' => true, 'production/www.gitlab.com' => true, 'productioneu' => false, @@ -311,7 +312,7 @@ describe Environment, models: true do end describe '#generate_slug' do - SUFFIX = "-[a-z0-9]{6}" + SUFFIX = "-[a-z0-9]{6}".freeze { "staging-12345678901234567" => "staging-123456789" + SUFFIX, "9-staging-123456789012345" => "env-9-staging-123" + SUFFIX, diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index e4be0aba7a6..87ea2e70680 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -89,8 +89,8 @@ describe ProjectMember, models: true do @user_1 = create :user @user_2 = create :user - @project_1.team << [ @user_1, :developer ] - @project_2.team << [ @user_2, :reporter ] + @project_1.team << [@user_1, :developer] + @project_2.team << [@user_2, :reporter] @status = @project_2.team.import(@project_1) end @@ -137,8 +137,8 @@ describe ProjectMember, models: true do @user_1 = create :user @user_2 = create :user - @project_1.team << [ @user_1, :developer] - @project_2.team << [ @user_2, :reporter] + @project_1.team << [@user_1, :developer] + @project_2.team << [@user_2, :reporter] ProjectMember.truncate_teams([@project_1.id, @project_2.id]) end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 6d599e148a2..0a10ee01506 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -109,7 +109,7 @@ describe MergeRequestDiff, models: true do { id: 'sha2' } ] - expect(subject.commits_sha).to eq(['sha1', 'sha2']) + expect(subject.commits_sha).to eq(%w(sha1 sha2)) end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index a01741a9971..fa1b0396bcf 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -209,6 +209,50 @@ describe MergeRequest, models: true do end end + describe '#diff_size' do + let(:merge_request) do + build(:merge_request, source_branch: 'expand-collapse-files', target_branch: 'master') + end + + context 'when there are MR diffs' do + before do + merge_request.save + end + + it 'returns the correct count' do + expect(merge_request.diff_size).to eq(105) + end + + it 'does not perform highlighting' do + expect(Gitlab::Diff::Highlight).not_to receive(:new) + + merge_request.diff_size + end + end + + context 'when there are no MR diffs' do + before do + merge_request.compare = CompareService.new( + merge_request.source_project, + merge_request.source_branch + ).execute( + merge_request.target_project, + merge_request.target_branch + ) + end + + it 'returns the correct count' do + expect(merge_request.diff_size).to eq(105) + end + + it 'does not perform highlighting' do + expect(Gitlab::Diff::Highlight).not_to receive(:new) + + merge_request.diff_size + end + end + end + describe "#related_notes" do let!(:merge_request) { create(:merge_request) } diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 35d932f1c64..3f9c4289de9 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -36,7 +36,7 @@ describe Namespace, models: true do end describe '#to_param' do - it { expect(namespace.to_param).to eq(namespace.path) } + it { expect(namespace.to_param).to eq(namespace.full_path) } end describe '#human_name' do @@ -151,7 +151,7 @@ describe Namespace, models: true do describe :rm_dir do let!(:project) { create(:empty_project, namespace: namespace) } - let!(:path) { File.join(Gitlab.config.repositories.storages.default, namespace.path) } + let!(:path) { File.join(Gitlab.config.repositories.storages.default, namespace.full_path) } it "removes its dirs when deleted" do namespace.destroy diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb index 497a626a418..4014d6129ee 100644 --- a/spec/models/project_services/bamboo_service_spec.rb +++ b/spec/models/project_services/bamboo_service_spec.rb @@ -181,7 +181,7 @@ describe BambooService, models: true, caching: true do end it 'sets commit status to "pending" when response has no results' do - stub_request(body: %Q({"results":{"results":{"size":"0"}}})) + stub_request(body: %q({"results":{"results":{"size":"0"}}})) is_expected.to eq('pending') end diff --git a/spec/models/project_services/buildkite_service_spec.rb b/spec/models/project_services/buildkite_service_spec.rb index dbd23ff5491..05b602d8106 100644 --- a/spec/models/project_services/buildkite_service_spec.rb +++ b/spec/models/project_services/buildkite_service_spec.rb @@ -92,7 +92,7 @@ describe BuildkiteService, models: true, caching: true do end it 'passes through build status untouched when status is 200' do - stub_request(body: %Q({"status":"Great Success"})) + stub_request(body: %q({"status":"Great Success"})) is_expected.to eq('Great Success') end @@ -101,7 +101,7 @@ describe BuildkiteService, models: true, caching: true do end def stub_request(status: 200, body: nil) - body ||= %Q({"status":"success"}) + body ||= %q({"status":"success"}) buildkite_full_url = 'https://gitlab.buildkite.com/status/secret-sauce-status-token.json?commit=123' WebMock.stub_request(:get, buildkite_full_url).to_return( diff --git a/spec/models/project_services/drone_ci_service_spec.rb b/spec/models/project_services/drone_ci_service_spec.rb index f9307d6de7b..044737c6026 100644 --- a/spec/models/project_services/drone_ci_service_spec.rb +++ b/spec/models/project_services/drone_ci_service_spec.rb @@ -28,7 +28,7 @@ describe DroneCiService, models: true, caching: true do shared_context :drone_ci_service do let(:drone) { DroneCiService.new } let(:project) { create(:project, :repository, name: 'project') } - let(:path) { "#{project.namespace.path}/#{project.path}" } + let(:path) { project.full_path } let(:drone_url) { 'http://drone.example.com' } let(:sha) { '2ab7834c' } let(:branch) { 'dev' } @@ -50,7 +50,7 @@ describe DroneCiService, models: true, caching: true do end def stub_request(status: 200, body: nil) - body ||= %Q({"status":"success"}) + body ||= %q({"status":"success"}) WebMock.stub_request(:get, commit_status_path).to_return( status: status, @@ -95,12 +95,12 @@ describe DroneCiService, models: true, caching: true do is_expected.to eq(:error) end - { "killed" => :canceled, + { + "killed" => :canceled, "failure" => :failed, "error" => :failed, - "success" => "success", + "success" => "success" }.each do |drone_status, our_status| - it "sets commit status to #{our_status.inspect} when returned status is #{drone_status.inspect}" do stub_request(body: %Q({"status":"#{drone_status}"})) diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb index b9fb6f3f6f4..d5a16226d9d 100644 --- a/spec/models/project_services/irker_service_spec.rb +++ b/spec/models/project_services/irker_service_spec.rb @@ -59,8 +59,8 @@ describe IrkerService, models: true do conn = @irker_server.accept conn.readlines.each do |line| - msg = JSON.load(line.chomp("\n")) - expect(msg.keys).to match_array(['to', 'privmsg']) + msg = JSON.parse(line.chomp("\n")) + expect(msg.keys).to match_array(%w(to privmsg)) expect(msg['to']).to match_array(["irc://chat.freenode.net/#commits", "irc://test.net/#test"]) end diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb index 9052479d35e..24356447fed 100644 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -171,7 +171,7 @@ describe KubernetesService, models: true, caching: true do context 'with invalid pods' do it 'returns no terminals' do - stub_reactive_cache(service, pods: [ { "bad" => "pod" } ]) + stub_reactive_cache(service, pods: [{ "bad" => "pod" }]) is_expected.to be_empty end @@ -184,7 +184,7 @@ describe KubernetesService, models: true, caching: true do before do stub_reactive_cache( service, - pods: [ pod, pod, kube_pod(app: "should-be-filtered-out") ] + pods: [pod, pod, kube_pod(app: "should-be-filtered-out")] ) end diff --git a/spec/models/project_services/mattermost_slash_commands_service_spec.rb b/spec/models/project_services/mattermost_slash_commands_service_spec.rb index 98f3d420c8a..598ff232c82 100644 --- a/spec/models/project_services/mattermost_slash_commands_service_spec.rb +++ b/spec/models/project_services/mattermost_slash_commands_service_spec.rb @@ -36,7 +36,8 @@ describe MattermostSlashCommandsService, :models do description: "Perform common operations on: #{project.name_with_namespace}", display_name: "GitLab / #{project.name_with_namespace}", method: 'P', - username: 'GitLab' }.to_json). + username: 'GitLab' + }.to_json). to_return( status: 200, headers: { 'Content-Type' => 'application/json' }, diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb index a1edd083aa1..77b18e1c7d0 100644 --- a/spec/models/project_services/teamcity_service_spec.rb +++ b/spec/models/project_services/teamcity_service_spec.rb @@ -143,7 +143,7 @@ describe TeamcityService, models: true, caching: true do end it 'returns a build URL when teamcity_url has no trailing slash' do - stub_request(body: %Q({"build":{"id":"666"}})) + stub_request(body: %q({"build":{"id":"666"}})) is_expected.to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo') end @@ -152,7 +152,7 @@ describe TeamcityService, models: true, caching: true do let(:teamcity_url) { 'http://gitlab.com/teamcity/' } it 'returns a build URL' do - stub_request(body: %Q({"build":{"id":"666"}})) + stub_request(body: %q({"build":{"id":"666"}})) is_expected.to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo') end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b0087a9e15d..ee4f4092062 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -402,7 +402,7 @@ describe Project, models: true do let(:project) { create(:empty_project, path: "somewhere") } it 'returns the full web URL for this repo' do - expect(project.web_url).to eq("#{Gitlab.config.gitlab.url}/#{project.namespace.path}/somewhere") + expect(project.web_url).to eq("#{Gitlab.config.gitlab.url}/#{project.namespace.full_path}/somewhere") end end @@ -803,7 +803,7 @@ describe Project, models: true do end let(:avatar_path) do - "/#{project.namespace.name}/#{project.path}/avatar" + "/#{project.full_path}/avatar" end it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" } @@ -1148,16 +1148,14 @@ describe Project, models: true do end it 'renames a repository' do - ns = project.namespace_dir - expect(gitlab_shell).to receive(:mv_repository). ordered. - with(project.repository_storage_path, "#{ns}/foo", "#{ns}/#{project.path}"). + with(project.repository_storage_path, "#{project.namespace.full_path}/foo", "#{project.full_path}"). and_return(true) expect(gitlab_shell).to receive(:mv_repository). ordered. - with(project.repository_storage_path, "#{ns}/foo.wiki", "#{ns}/#{project.path}.wiki"). + with(project.repository_storage_path, "#{project.namespace.full_path}/foo.wiki", "#{project.full_path}.wiki"). and_return(true) expect_any_instance_of(SystemHooksService). @@ -1166,7 +1164,7 @@ describe Project, models: true do expect_any_instance_of(Gitlab::UploadsTransfer). to receive(:rename_project). - with('foo', project.path, ns) + with('foo', project.path, project.namespace.full_path) expect(project).to receive(:expire_caches_before_rename) @@ -1538,7 +1536,7 @@ describe Project, models: true do it 'schedules a RepositoryForkWorker job' do expect(RepositoryForkWorker).to receive(:perform_async). with(project.id, forked_from_project.repository_storage_path, - forked_from_project.path_with_namespace, project.namespace.path) + forked_from_project.path_with_namespace, project.namespace.full_path) project.add_import_job end @@ -1752,7 +1750,7 @@ describe Project, models: true do describe 'inside_path' do let!(:project1) { create(:empty_project) } let!(:project2) { create(:empty_project) } - let!(:path) { project1.namespace.path } + let!(:path) { project1.namespace.full_path } it { expect(Project.inside_path(path)).to eq([project1]) } end @@ -1767,7 +1765,7 @@ describe Project, models: true do end before do - project.repository.commit_file(User.last, '.gitlab/route-map.yml', route_map, message: 'Add .gitlab/route-map.yml', branch_name: 'master', update: false) + project.repository.create_file(User.last, '.gitlab/route-map.yml', route_map, message: 'Add .gitlab/route-map.yml', branch_name: 'master') end context 'when there is a .gitlab/route-map.yml at the commit' do @@ -1896,4 +1894,25 @@ describe Project, models: true do end end end + + describe '#http_url_to_repo' do + let(:project) { create :empty_project } + + context 'when no user is given' do + it 'returns the url to the repo without a username' do + url = project.http_url_to_repo + + expect(url).to eq(project.http_url_to_repo) + expect(url).not_to include('@') + end + end + + context 'when user is given' do + it 'returns the url to the repo with the username' do + user = build_stubbed(:user) + + expect(project.http_url_to_repo(user)).to match(%r{https?:\/\/#{user.username}@}) + end + end + end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 838fd3754b2..ae203fada12 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -291,10 +291,10 @@ describe Repository, models: true do end end - describe "#commit_dir" do + describe "#create_dir" do it "commits a change that creates a new directory" do expect do - repository.commit_dir(user, 'newdir', + repository.create_dir(user, 'newdir', message: 'Create newdir', branch_name: 'master') end.to change { repository.commits('master').count }.by(1) @@ -307,7 +307,7 @@ describe Repository, models: true do it "creates a fork and commit to the forked project" do expect do - repository.commit_dir(user, 'newdir', + repository.create_dir(user, 'newdir', message: 'Create newdir', branch_name: 'patch', start_branch_name: 'master', start_project: forked_project) end.to change { repository.commits('master').count }.by(0) @@ -323,7 +323,7 @@ describe Repository, models: true do context "when an author is specified" do it "uses the given email/name to set the commit's author" do expect do - repository.commit_dir(user, 'newdir', + repository.create_dir(user, 'newdir', message: 'Add newdir', branch_name: 'master', author_email: author_email, author_name: author_name) @@ -337,25 +337,23 @@ describe Repository, models: true do end end - describe "#commit_file" do - it 'commits change to a file successfully' do + describe "#create_file" do + it 'commits new file successfully' do expect do - repository.commit_file(user, 'CHANGELOG', 'Changelog!', - message: 'Updates file content', - branch_name: 'master', - update: true) + repository.create_file(user, 'NEWCHANGELOG', 'Changelog!', + message: 'Create changelog', + branch_name: 'master') end.to change { repository.commits('master').count }.by(1) - blob = repository.blob_at('master', 'CHANGELOG') + blob = repository.blob_at('master', 'NEWCHANGELOG') expect(blob.data).to eq('Changelog!') end it 'respects the autocrlf setting' do - repository.commit_file(user, 'hello.txt', "Hello,\r\nWorld", + repository.create_file(user, 'hello.txt', "Hello,\r\nWorld", message: 'Add hello world', - branch_name: 'master', - update: true) + branch_name: 'master') blob = repository.blob_at('master', 'hello.txt') @@ -365,10 +363,9 @@ describe Repository, models: true do context "when an author is specified" do it "uses the given email/name to set the commit's author" do expect do - repository.commit_file(user, 'README', 'README!', + repository.create_file(user, 'NEWREADME', 'README!', message: 'Add README', branch_name: 'master', - update: true, author_email: author_email, author_name: author_name) end.to change { repository.commits('master').count }.by(1) @@ -382,6 +379,18 @@ describe Repository, models: true do end describe "#update_file" do + it 'updates file successfully' do + expect do + repository.update_file(user, 'CHANGELOG', 'Changelog!', + message: 'Update changelog', + branch_name: 'master') + end.to change { repository.commits('master').count }.by(1) + + blob = repository.blob_at('master', 'CHANGELOG') + + expect(blob.data).to eq('Changelog!') + end + it 'updates filename successfully' do expect do repository.update_file(user, 'NEWLICENSE', 'Copyright!', @@ -398,9 +407,6 @@ describe Repository, models: true do context "when an author is specified" do it "uses the given email/name to set the commit's author" do - repository.commit_file(user, 'README', 'README!', - message: 'Add README', branch_name: 'master', update: true) - expect do repository.update_file(user, 'README', 'Updated README!', branch_name: 'master', @@ -418,13 +424,10 @@ describe Repository, models: true do end end - describe "#remove_file" do + describe "#delete_file" do it 'removes file successfully' do - repository.commit_file(user, 'README', 'README!', - message: 'Add README', branch_name: 'master', update: true) - expect do - repository.remove_file(user, 'README', + repository.delete_file(user, 'README', message: 'Remove README', branch_name: 'master') end.to change { repository.commits('master').count }.by(1) @@ -433,11 +436,8 @@ describe Repository, models: true do context "when an author is specified" do it "uses the given email/name to set the commit's author" do - repository.commit_file(user, 'README', 'README!', - message: 'Add README', branch_name: 'master', update: true) - expect do - repository.remove_file(user, 'README', + repository.delete_file(user, 'README', message: 'Remove README', branch_name: 'master', author_email: author_email, author_name: author_name) end.to change { repository.commits('master').count }.by(1) @@ -587,14 +587,14 @@ describe Repository, models: true do describe "#license_blob", caching: true do before do - repository.remove_file( + repository.delete_file( user, 'LICENSE', message: 'Remove LICENSE', branch_name: 'master') end it 'handles when HEAD points to non-existent ref' do - repository.commit_file( + repository.create_file( user, 'LICENSE', 'Copyright!', - message: 'Add LICENSE', branch_name: 'master', update: false) + message: 'Add LICENSE', branch_name: 'master') allow(repository).to receive(:file_on_head). and_raise(Rugged::ReferenceError) @@ -603,27 +603,27 @@ describe Repository, models: true do end it 'looks in the root_ref only' do - repository.remove_file(user, 'LICENSE', + repository.delete_file(user, 'LICENSE', message: 'Remove LICENSE', branch_name: 'markdown') - repository.commit_file(user, 'LICENSE', + repository.create_file(user, 'LICENSE', Licensee::License.new('mit').content, - message: 'Add LICENSE', branch_name: 'markdown', update: false) + message: 'Add LICENSE', branch_name: 'markdown') expect(repository.license_blob).to be_nil end it 'detects license file with no recognizable open-source license content' do - repository.commit_file(user, 'LICENSE', 'Copyright!', - message: 'Add LICENSE', branch_name: 'master', update: false) + repository.create_file(user, 'LICENSE', 'Copyright!', + message: 'Add LICENSE', branch_name: 'master') expect(repository.license_blob.name).to eq('LICENSE') end %w[LICENSE LICENCE LiCensE LICENSE.md LICENSE.foo COPYING COPYING.md].each do |filename| it "detects '#{filename}'" do - repository.commit_file(user, filename, + repository.create_file(user, filename, Licensee::License.new('mit').content, - message: "Add #{filename}", branch_name: 'master', update: false) + message: "Add #{filename}", branch_name: 'master') expect(repository.license_blob.name).to eq(filename) end @@ -632,7 +632,7 @@ describe Repository, models: true do describe '#license_key', caching: true do before do - repository.remove_file(user, 'LICENSE', + repository.delete_file(user, 'LICENSE', message: 'Remove LICENSE', branch_name: 'master') end @@ -647,16 +647,16 @@ describe Repository, models: true do end it 'detects license file with no recognizable open-source license content' do - repository.commit_file(user, 'LICENSE', 'Copyright!', - message: 'Add LICENSE', branch_name: 'master', update: false) + repository.create_file(user, 'LICENSE', 'Copyright!', + message: 'Add LICENSE', branch_name: 'master') expect(repository.license_key).to be_nil end it 'returns the license key' do - repository.commit_file(user, 'LICENSE', + repository.create_file(user, 'LICENSE', Licensee::License.new('mit').content, - message: 'Add LICENSE', branch_name: 'master', update: false) + message: 'Add LICENSE', branch_name: 'master') expect(repository.license_key).to eq('mit') end @@ -913,10 +913,9 @@ describe Repository, models: true do expect(empty_repository).to receive(:expire_emptiness_caches) expect(empty_repository).to receive(:expire_branches_cache) - empty_repository.commit_file(user, 'CHANGELOG', 'Changelog!', + empty_repository.create_file(user, 'CHANGELOG', 'Changelog!', message: 'Updates file content', - branch_name: 'master', - update: false) + branch_name: 'master') end end end @@ -1382,13 +1381,13 @@ describe Repository, models: true do describe '#branch_count' do it 'returns the number of branches' do - expect(repository.branch_count).to be_an_instance_of(Fixnum) + expect(repository.branch_count).to be_an(Integer) end end describe '#tag_count' do it 'returns the number of tags' do - expect(repository.tag_count).to be_an_instance_of(Fixnum) + expect(repository.tag_count).to be_an(Integer) end end @@ -1738,7 +1737,7 @@ describe Repository, models: true do context 'with an existing repository' do it 'returns the commit count' do - expect(repository.commit_count).to be_an_instance_of(Fixnum) + expect(repository.commit_count).to be_an(Integer) end end end @@ -1796,7 +1795,7 @@ describe Repository, models: true do describe '#gitlab_ci_yml_for' do before do - repository.commit_file(User.last, '.gitlab-ci.yml', 'CONTENT', message: 'Add .gitlab-ci.yml', branch_name: 'master', update: false) + repository.create_file(User.last, '.gitlab-ci.yml', 'CONTENT', message: 'Add .gitlab-ci.yml', branch_name: 'master') end context 'when there is a .gitlab-ci.yml at the commit' do @@ -1814,7 +1813,7 @@ describe Repository, models: true do describe '#route_map_for' do before do - repository.commit_file(User.last, '.gitlab/route-map.yml', 'CONTENT', message: 'Add .gitlab/route-map.yml', branch_name: 'master', update: false) + repository.create_file(User.last, '.gitlab/route-map.yml', 'CONTENT', message: 'Add .gitlab/route-map.yml', branch_name: 'master') end context 'when there is a .gitlab/route-map.yml at the commit' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 584a4facd94..6356f8b6c92 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -22,7 +22,7 @@ describe User, models: true do it { is_expected.to have_many(:deploy_keys).dependent(:destroy) } it { is_expected.to have_many(:events).dependent(:destroy) } it { is_expected.to have_many(:recent_events).class_name('Event') } - it { is_expected.to have_many(:issues).dependent(:destroy) } + it { is_expected.to have_many(:issues).dependent(:restrict_with_exception) } it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:assigned_issues).dependent(:nullify) } it { is_expected.to have_many(:merge_requests).dependent(:destroy) } @@ -208,6 +208,22 @@ describe User, models: true do end end end + + describe 'ghost users' do + it 'does not allow a non-blocked ghost user' do + user = build(:user, :ghost) + user.state = 'active' + + expect(user).to be_invalid + end + + it 'allows a blocked ghost user' do + user = build(:user, :ghost) + user.state = 'blocked' + + expect(user).to be_valid + end + end end describe "scopes" do @@ -693,9 +709,7 @@ describe User, models: true do end describe '.search_with_secondary_emails' do - def search_with_secondary_emails(query) - described_class.search_with_secondary_emails(query) - end + delegate :search_with_secondary_emails, to: :described_class let!(:user) { create(:user) } let!(:email) { create(:email) } @@ -1492,4 +1506,41 @@ describe User, models: true do expect(user.admin).to be true end end + + describe '.ghost' do + it "creates a ghost user if one isn't already present" do + ghost = User.ghost + + expect(ghost).to be_ghost + expect(ghost).to be_persisted + end + + it "does not create a second ghost user if one is already present" do + expect do + User.ghost + User.ghost + end.to change { User.count }.by(1) + expect(User.ghost).to eq(User.ghost) + end + + context "when a regular user exists with the username 'ghost'" do + it "creates a ghost user with a non-conflicting username" do + create(:user, username: 'ghost') + ghost = User.ghost + + expect(ghost).to be_persisted + expect(ghost.username).to eq('ghost1') + end + end + + context "when a regular user exists with the email 'ghost@example.com'" do + it "creates a ghost user with a non-conflicting email" do + create(:user, email: 'ghost@example.com') + ghost = User.ghost + + expect(ghost).to be_persisted + expect(ghost.email).to eq('ghost1@example.com') + end + end + end end |