diff options
Diffstat (limited to 'spec')
40 files changed, 831 insertions, 172 deletions
diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 7211acc53dc..4a737587899 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -69,18 +69,11 @@ describe Projects::JobsController do Ci::Build::AVAILABLE_STATUSES.each do |status| create_build(status, status) end - - RequestStore.begin! - end - - after do - RequestStore.end! - RequestStore.clear! end - it "verifies number of queries" do + it 'verifies number of queries', :request_store do recorded = ActiveRecord::QueryRecorder.new { get_index } - expect(recorded.count).to be_within(5).of(8) + expect(recorded.count).to be_within(5).of(7) end def create_build(name, status) diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 0a64fe2beda..6e1c91738db 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -119,10 +119,8 @@ describe Projects::MergeRequestsController do end end - context 'number of queries' do + context 'number of queries', :request_store do it 'verifies number of queries' do - RequestStore.begin! - # pre-create objects merge_request @@ -130,9 +128,6 @@ describe Projects::MergeRequestsController do expect(recorded.count).to be_within(5).of(30) expect(recorded.cached_count).to eq(0) - - RequestStore.end! - RequestStore.clear! end end end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index c880da1e36a..734532668d3 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -5,9 +5,12 @@ describe Projects::PipelinesController do let(:user) { create(:user) } let(:project) { create(:empty_project, :public) } + let(:feature) { ProjectFeature::DISABLED } before do project.add_developer(user) + project.project_feature.update( + builds_access_level: feature) sign_in(user) end @@ -49,21 +52,14 @@ describe Projects::PipelinesController do expect(json_response['details']).to have_key 'stages' end - context 'when the pipeline has multiple stages and groups' do + context 'when the pipeline has multiple stages and groups', :request_store do before do - RequestStore.begin! - create_build('build', 0, 'build') create_build('test', 1, 'rspec 0') create_build('deploy', 2, 'production') create_build('post deploy', 3, 'pages 0') end - after do - RequestStore.end! - RequestStore.clear! - end - let(:project) { create(:project) } let(:pipeline) do create(:ci_empty_pipeline, project: project, user: user, sha: project.commit.id) @@ -160,16 +156,26 @@ describe Projects::PipelinesController do format: :json end - it 'retries a pipeline without returning any content' do - expect(response).to have_http_status(:no_content) - expect(build.reload).to be_retried + context 'when builds are enabled' do + let(:feature) { ProjectFeature::ENABLED } + + it 'retries a pipeline without returning any content' do + expect(response).to have_http_status(:no_content) + expect(build.reload).to be_retried + end + end + + context 'when builds are disabled' do + it 'fails to retry pipeline' do + expect(response).to have_http_status(:not_found) + end end end describe 'POST cancel.json' do let!(:pipeline) { create(:ci_pipeline, project: project) } let!(:build) { create(:ci_build, :running, pipeline: pipeline) } - + before do post :cancel, namespace_id: project.namespace, project_id: project, @@ -177,9 +183,19 @@ describe Projects::PipelinesController do format: :json end - it 'cancels a pipeline without returning any content' do - expect(response).to have_http_status(:no_content) - expect(pipeline.reload).to be_canceled + context 'when builds are enabled' do + let(:feature) { ProjectFeature::ENABLED } + + it 'cancels a pipeline without returning any content' do + expect(response).to have_http_status(:no_content) + expect(pipeline.reload).to be_canceled + end + end + + context 'when builds are disabled' do + it 'fails to retry pipeline' do + expect(response).to have_http_status(:not_found) + end end end end diff --git a/spec/features/help_pages_spec.rb b/spec/features/help_pages_spec.rb index e0b2404e60a..31014f5cad2 100644 --- a/spec/features/help_pages_spec.rb +++ b/spec/features/help_pages_spec.rb @@ -34,7 +34,7 @@ describe 'Help Pages', feature: true do end end - context 'in a production environment with version check enabled', js: true do + context 'in a production environment with version check enabled', :js do before do allow(Rails.env).to receive(:production?) { true } allow(current_application_settings).to receive(:version_check_enabled) { true } @@ -44,18 +44,12 @@ describe 'Help Pages', feature: true do visit help_path end - it 'should display a version check image' do - expect(find('.js-version-status-badge')).to be_visible + it 'has a version check image' do + expect(find('.js-version-status-badge', visible: false)['src']).to end_with('/version-check-url') end - it 'should have a src url' do - expect(find('.js-version-status-badge')['src']).to match(/\/version-check-url/) - end - - it 'should hide the version check image if the image request fails' do - # We use '--load-images=no' with poltergeist so we must trigger manually - execute_script("$('.js-version-status-badge').trigger('error');") - + it 'hides the version check image if the image request fails' do + # We use '--load-images=yes' with poltergeist so the image fails to load expect(find('.js-version-status-badge', visible: false)).not_to be_visible end end diff --git a/spec/features/projects/artifacts/file_spec.rb b/spec/features/projects/artifacts/file_spec.rb index 25c4f3c87a2..860373e531b 100644 --- a/spec/features/projects/artifacts/file_spec.rb +++ b/spec/features/projects/artifacts/file_spec.rb @@ -39,6 +39,7 @@ feature 'Artifact file', :js, feature: true do context 'JPG file' do before do + page.driver.browser.url_blacklist = [] visit_file('rails_sample.jpg') wait_for_requests diff --git a/spec/features/projects/features_visibility_spec.rb b/spec/features/projects/features_visibility_spec.rb index c49648f54bd..d76b5e4ef1b 100644 --- a/spec/features/projects/features_visibility_spec.rb +++ b/spec/features/projects/features_visibility_spec.rb @@ -68,9 +68,12 @@ describe 'Edit Project Settings', feature: true do end describe 'project features visibility pages' do + let(:pipeline) { create(:ci_empty_pipeline, project: project) } + let(:job) { create(:ci_build, pipeline: pipeline) } + let(:tools) do { - builds: namespace_project_pipelines_path(project.namespace, project), + builds: namespace_project_job_path(project.namespace, project, job), issues: namespace_project_issues_path(project.namespace, project), wiki: namespace_project_wiki_path(project.namespace, project, :home), snippets: namespace_project_snippets_path(project.namespace, project), diff --git a/spec/features/projects/pipeline_schedules_spec.rb b/spec/features/projects/pipeline_schedules_spec.rb index 317949d6b56..2d43f7a10bc 100644 --- a/spec/features/projects/pipeline_schedules_spec.rb +++ b/spec/features/projects/pipeline_schedules_spec.rb @@ -127,7 +127,7 @@ feature 'Pipeline Schedules', :feature do end it 'shows the pipeline schedule with default ref' do - page.within('.git-revision-dropdown-toggle') do + page.within('.js-target-branch-dropdown') do expect(first('.dropdown-toggle-text').text).to eq('master') end end diff --git a/spec/features/user_can_display_performance_bar_spec.rb b/spec/features/user_can_display_performance_bar_spec.rb new file mode 100644 index 00000000000..c2842255b86 --- /dev/null +++ b/spec/features/user_can_display_performance_bar_spec.rb @@ -0,0 +1,81 @@ +require 'rails_helper' + +describe 'User can display performacne bar', :js do + shared_examples 'performance bar is disabled' do + it 'does not show the performance bar by default' do + expect(page).not_to have_css('#peek') + end + + context 'when user press `pb`' do + before do + find('body').native.send_keys('pb') + end + + it 'does not show the performance bar by default' do + expect(page).not_to have_css('#peek') + end + end + end + + shared_examples 'performance bar is enabled' do + it 'does not show the performance bar by default' do + expect(page).not_to have_css('#peek') + end + + context 'when user press `pb`' do + before do + find('body').native.send_keys('pb') + end + + it 'does not show the performance bar by default' do + expect(page).not_to have_css('#peek') + end + end + end + + context 'when user is logged-out' do + before do + visit root_path + end + + context 'when the gitlab_performance_bar feature is disabled' do + before do + Feature.disable('gitlab_performance_bar') + end + + it_behaves_like 'performance bar is disabled' + end + + context 'when the gitlab_performance_bar feature is enabled' do + before do + Feature.enable('gitlab_performance_bar') + end + + it_behaves_like 'performance bar is disabled' + end + end + + context 'when user is logged-in' do + before do + login_as :user + + visit root_path + end + + context 'when the gitlab_performance_bar feature is disabled' do + before do + Feature.disable('gitlab_performance_bar') + end + + it_behaves_like 'performance bar is disabled' + end + + context 'when the gitlab_performance_bar feature is enabled' do + before do + Feature.enable('gitlab_performance_bar') + end + + it_behaves_like 'performance bar is enabled' + end + end +end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index a695621b87a..3ed0b0a756b 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -300,4 +300,37 @@ describe ProjectsHelper do expect(helper.send(:visibility_select_options, project, Gitlab::VisibilityLevel::PRIVATE)).to include('Private') end end + + describe '#get_project_nav_tabs' do + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + + before do + allow(helper).to receive(:can?) { true } + end + + subject do + helper.send(:get_project_nav_tabs, project, user) + end + + context 'when builds feature is enabled' do + before do + allow(project).to receive(:builds_enabled?).and_return(true) + end + + it "does include pipelines tab" do + is_expected.to include(:pipelines) + end + end + + context 'when builds feature is disabled' do + before do + allow(project).to receive(:builds_enabled?).and_return(false) + end + + it "do not include pipelines tab" do + is_expected.not_to include(:pipelines) + end + end + end end diff --git a/spec/helpers/u2f_helper_spec.rb b/spec/helpers/u2f_helper_spec.rb new file mode 100644 index 00000000000..0d65b4fe0b8 --- /dev/null +++ b/spec/helpers/u2f_helper_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe U2fHelper do + describe 'when not on mobile' do + it 'does not inject u2f on chrome 40' do + device = double(mobile?: false) + browser = double(chrome?: true, opera?: false, version: 40, device: device) + allow(helper).to receive(:browser).and_return(browser) + expect(helper.inject_u2f_api?).to eq false + end + + it 'injects u2f on chrome 41' do + device = double(mobile?: false) + browser = double(chrome?: true, opera?: false, version: 41, device: device) + allow(helper).to receive(:browser).and_return(browser) + expect(helper.inject_u2f_api?).to eq true + end + + it 'does not inject u2f on opera 39' do + device = double(mobile?: false) + browser = double(chrome?: false, opera?: true, version: 39, device: device) + allow(helper).to receive(:browser).and_return(browser) + expect(helper.inject_u2f_api?).to eq false + end + + it 'injects u2f on opera 40' do + device = double(mobile?: false) + browser = double(chrome?: false, opera?: true, version: 40, device: device) + allow(helper).to receive(:browser).and_return(browser) + expect(helper.inject_u2f_api?).to eq true + end + end + + describe 'when on mobile' do + it 'does not inject u2f on chrome 41' do + device = double(mobile?: true) + browser = double(chrome?: true, opera?: false, version: 41, device: device) + allow(helper).to receive(:browser).and_return(browser) + expect(helper.inject_u2f_api?).to eq false + end + + it 'does not inject u2f on opera 40' do + device = double(mobile?: true) + browser = double(chrome?: false, opera?: true, version: 40, device: device) + allow(helper).to receive(:browser).and_return(browser) + expect(helper.inject_u2f_api?).to eq false + end + end +end diff --git a/spec/javascripts/vue_shared/components/time_ago_tooltip_spec.js b/spec/javascripts/vue_shared/components/time_ago_tooltip_spec.js index bf28019ef24..f3b4adc0b70 100644 --- a/spec/javascripts/vue_shared/components/time_ago_tooltip_spec.js +++ b/spec/javascripts/vue_shared/components/time_ago_tooltip_spec.js @@ -22,7 +22,7 @@ describe('Time ago with tooltip component', () => { }).$mount(); expect(vm.$el.tagName).toEqual('TIME'); - expect(vm.$el.classList.contains('js-timeago')).toEqual(true); + expect(vm.$el.classList.contains('js-vue-timeago')).toEqual(true); expect( vm.$el.getAttribute('data-original-title'), ).toEqual(gl.utils.formatDate('2017-05-08T14:57:39.781Z')); @@ -44,17 +44,6 @@ describe('Time ago with tooltip component', () => { expect(vm.$el.getAttribute('data-placement')).toEqual('bottom'); }); - it('should render short format class', () => { - vm = new TimeagoTooltip({ - propsData: { - time: '2017-05-08T14:57:39.781Z', - shortFormat: true, - }, - }).$mount(); - - expect(vm.$el.classList.contains('js-short-timeago')).toEqual(true); - }); - it('should render provided html class', () => { vm = new TimeagoTooltip({ propsData: { diff --git a/spec/lib/banzai/filter/abstract_reference_filter_spec.rb b/spec/lib/banzai/filter/abstract_reference_filter_spec.rb index 27684882435..787c2372c5b 100644 --- a/spec/lib/banzai/filter/abstract_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/abstract_reference_filter_spec.rb @@ -47,16 +47,7 @@ describe Banzai::Filter::AbstractReferenceFilter do end end - context 'with RequestStore enabled' do - before do - RequestStore.begin! - end - - after do - RequestStore.end! - RequestStore.clear! - end - + context 'with RequestStore enabled', :request_store do it 'returns a list of Projects for a list of paths' do expect(filter.find_projects_for_paths([project.path_with_namespace])). to eq([project]) diff --git a/spec/lib/banzai/issuable_extractor_spec.rb b/spec/lib/banzai/issuable_extractor_spec.rb index e5d332efb08..866297f94a9 100644 --- a/spec/lib/banzai/issuable_extractor_spec.rb +++ b/spec/lib/banzai/issuable_extractor_spec.rb @@ -29,16 +29,7 @@ describe Banzai::IssuableExtractor, lib: true do expect(result).to eq(issue_link => issue, merge_request_link => merge_request) end - describe 'caching' do - before do - RequestStore.begin! - end - - after do - RequestStore.end! - RequestStore.clear! - end - + describe 'caching', :request_store do it 'saves records to cache' do extractor.extract([issue_link, merge_request_link]) diff --git a/spec/lib/banzai/reference_parser/user_parser_spec.rb b/spec/lib/banzai/reference_parser/user_parser_spec.rb index 592ed0d2b98..4d560667342 100644 --- a/spec/lib/banzai/reference_parser/user_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/user_parser_spec.rb @@ -43,18 +43,9 @@ describe Banzai::ReferenceParser::UserParser, lib: true do expect(subject.referenced_by([link])).to eq([user]) end - context 'when RequestStore is active' do + context 'when RequestStore is active', :request_store do let(:other_user) { create(:user) } - before do - RequestStore.begin! - end - - after do - RequestStore.end! - RequestStore.clear! - end - it 'does not return users from the first call in the second' do link['data-user'] = user.id.to_s diff --git a/spec/lib/gitlab/background_migration_spec.rb b/spec/lib/gitlab/background_migration_spec.rb new file mode 100644 index 00000000000..f2073b9bcb3 --- /dev/null +++ b/spec/lib/gitlab/background_migration_spec.rb @@ -0,0 +1,48 @@ +require 'spec_helper' + +describe Gitlab::BackgroundMigration do + describe '.steal' do + it 'steals jobs from a queue' do + queue = [double(:job, args: ['Foo', [10, 20]])] + + allow(Sidekiq::Queue).to receive(:new). + with(BackgroundMigrationWorker.sidekiq_options['queue']). + and_return(queue) + + expect(queue[0]).to receive(:delete) + + expect(described_class).to receive(:perform).with('Foo', [10, 20]) + + described_class.steal('Foo') + end + + it 'does not steal jobs for a different migration' do + queue = [double(:job, args: ['Foo', [10, 20]])] + + allow(Sidekiq::Queue).to receive(:new). + with(BackgroundMigrationWorker.sidekiq_options['queue']). + and_return(queue) + + expect(described_class).not_to receive(:perform) + + expect(queue[0]).not_to receive(:delete) + + described_class.steal('Bar') + end + end + + describe '.perform' do + it 'performs a background migration' do + instance = double(:instance) + klass = double(:klass, new: instance) + + expect(described_class).to receive(:const_get). + with('Foo'). + and_return(klass) + + expect(instance).to receive(:perform).with(10, 20) + + described_class.perform('Foo', [10, 20]) + end + end +end diff --git a/spec/lib/gitlab/ci/status/external/common_spec.rb b/spec/lib/gitlab/ci/status/external/common_spec.rb index 5a97d98b55f..e58f5d8d4df 100644 --- a/spec/lib/gitlab/ci/status/external/common_spec.rb +++ b/spec/lib/gitlab/ci/status/external/common_spec.rb @@ -4,9 +4,10 @@ describe Gitlab::Ci::Status::External::Common do let(:user) { create(:user) } let(:project) { external_status.project } let(:external_target_url) { 'http://example.gitlab.com/status' } + let(:external_description) { 'my description' } let(:external_status) do - create(:generic_commit_status, target_url: external_target_url) + create(:generic_commit_status, target_url: external_target_url, description: external_description) end subject do @@ -15,6 +16,12 @@ describe Gitlab::Ci::Status::External::Common do .extend(described_class) end + describe '#label' do + it 'returns description' do + expect(subject.label).to eq external_description + end + end + describe '#has_action?' do it { is_expected.not_to have_action } end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 3fdafd867da..30aa463faf8 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -7,7 +7,42 @@ describe Gitlab::Database::MigrationHelpers, lib: true do ) end - before { allow(model).to receive(:puts) } + before do + allow(model).to receive(:puts) + end + + describe '#add_timestamps_with_timezone' do + before do + allow(model).to receive(:transaction_open?).and_return(false) + end + + context 'using PostgreSQL' do + before do + allow(Gitlab::Database).to receive(:postgresql?).and_return(true) + allow(model).to receive(:disable_statement_timeout) + end + + it 'adds "created_at" and "updated_at" fields with the "datetime_with_timezone" data type' do + expect(model).to receive(:add_column).with(:foo, :created_at, :datetime_with_timezone, { null: false }) + expect(model).to receive(:add_column).with(:foo, :updated_at, :datetime_with_timezone, { null: false }) + + model.add_timestamps_with_timezone(:foo) + end + end + + context 'using MySQL' do + before do + allow(Gitlab::Database).to receive(:postgresql?).and_return(false) + end + + it 'adds "created_at" and "updated_at" fields with "datetime_with_timezone" data type' do + expect(model).to receive(:add_column).with(:foo, :created_at, :datetime_with_timezone, { null: false }) + expect(model).to receive(:add_column).with(:foo, :updated_at, :datetime_with_timezone, { null: false }) + + model.add_timestamps_with_timezone(:foo) + end + end + end describe '#add_concurrent_index' do context 'outside a transaction' do diff --git a/spec/models/ci/legacy_stage_spec.rb b/spec/models/ci/legacy_stage_spec.rb index 48116c7e701..d43c33d3807 100644 --- a/spec/models/ci/legacy_stage_spec.rb +++ b/spec/models/ci/legacy_stage_spec.rb @@ -55,6 +55,17 @@ describe Ci::LegacyStage, :models do expect(stage.groups.map(&:name)) .to eq %w[aaaaa rspec spinach] end + + context 'when a name is nil on legacy pipelines' do + before do + pipeline.builds.first.update_attribute(:name, nil) + end + + it 'returns an array of three groups' do + expect(stage.groups.map(&:name)) + .to eq ['', 'aaaaa', 'rspec', 'spinach'] + end + end end describe '#statuses_count' do diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 140fd2720bf..ba247dcc5cf 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -20,8 +20,8 @@ describe Commit, models: true do end it 'caches the author' do + allow(RequestStore).to receive(:active?).and_return(true) user = create(:user, email: commit.author_email) - expect(RequestStore).to receive(:active?).twice.and_return(true) expect_any_instance_of(Commit).to receive(:find_author_by_any_email).and_call_original expect(commit.author).to eq(user) diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index 0e10d91836d..65f05121b40 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -122,16 +122,7 @@ describe Group, 'Routable' do it { expect(group.full_path).to eq(group.path) } it { expect(nested_group.full_path).to eq("#{group.full_path}/#{nested_group.path}") } - context 'with RequestStore active' do - before do - RequestStore.begin! - end - - after do - RequestStore.end! - RequestStore.clear! - end - + context 'with RequestStore active', :request_store do it 'does not load the route table more than once' do expect(group).to receive(:uncached_full_path).once.and_call_original diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 362565506e5..497e3cdf415 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -389,16 +389,7 @@ describe ProjectTeam, models: true do end describe '#max_member_access_for_user_ids' do - context 'with RequestStore enabled' do - before do - RequestStore.begin! - end - - after do - RequestStore.end! - RequestStore.clear! - end - + context 'with RequestStore enabled', :request_store do include_examples 'max member access for users' def access_levels(users) diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 0d3af1f4499..848fd547e10 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -139,6 +139,18 @@ describe ProjectPolicy, models: true do is_expected.not_to include(:read_build, :read_pipeline) end end + + context 'when builds are disabled' do + before do + project.project_feature.update( + builds_access_level: ProjectFeature::DISABLED) + end + + it do + is_expected.not_to include(:read_build) + is_expected.to include(:read_pipeline) + end + end end context 'reporter' do diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 73ef58bcc42..5a4f0513248 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -434,8 +434,29 @@ describe API::Runner do expect(response).to have_http_status(201) expect(json_response['id']).to eq(test_job.id) expect(json_response['dependencies'].count).to eq(2) - expect(json_response['dependencies']).to include({ 'id' => job.id, 'name' => job.name, 'token' => job.token }, - { 'id' => job2.id, 'name' => job2.name, 'token' => job2.token }) + expect(json_response['dependencies']).to include( + { 'id' => job.id, 'name' => job.name, 'token' => job.token }, + { 'id' => job2.id, 'name' => job2.name, 'token' => job2.token }) + end + end + + context 'when pipeline have jobs with artifacts' do + let!(:job) { create(:ci_build_tag, :artifacts, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } + let!(:test_job) { create(:ci_build, pipeline: pipeline, name: 'deploy', stage: 'deploy', stage_idx: 1) } + + before do + job.success + end + + it 'returns dependent jobs' do + request_job + + expect(response).to have_http_status(201) + expect(json_response['id']).to eq(test_job.id) + expect(json_response['dependencies'].count).to eq(1) + expect(json_response['dependencies']).to include( + { 'id' => job.id, 'name' => job.name, 'token' => job.token, + 'artifacts_file' => { 'filename' => 'ci_build_artifacts.zip', 'size' => 106365 } }) end end diff --git a/spec/rubocop/cop/migration/add_timestamps_spec.rb b/spec/rubocop/cop/migration/add_timestamps_spec.rb new file mode 100644 index 00000000000..18df62dec3e --- /dev/null +++ b/spec/rubocop/cop/migration/add_timestamps_spec.rb @@ -0,0 +1,90 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/add_timestamps' + +describe RuboCop::Cop::Migration::AddTimestamps do + include CopHelper + + subject(:cop) { described_class.new } + let(:migration_with_add_timestamps) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column(:users, :username, :text) + add_timestamps(:users) + end + end + ) + end + + let(:migration_without_add_timestamps) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column(:users, :username, :text) + end + end + ) + end + + let(:migration_with_add_timestamps_with_timezone) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column(:users, :username, :text) + add_timestamps_with_timezone(:users) + end + end + ) + end + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + it 'registers an offense when the "add_timestamps" method is used' do + inspect_source(cop, migration_with_add_timestamps) + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([7]) + end + end + + it 'does not register an offense when the "add_timestamps" method is not used' do + inspect_source(cop, migration_without_add_timestamps) + + aggregate_failures do + expect(cop.offenses.size).to eq(0) + end + end + + it 'does not register an offense when the "add_timestamps_with_timezone" method is used' do + inspect_source(cop, migration_with_add_timestamps_with_timezone) + + aggregate_failures do + expect(cop.offenses.size).to eq(0) + end + end + end + + context 'outside of migration' do + it 'registers no offense' do + inspect_source(cop, migration_with_add_timestamps) + inspect_source(cop, migration_without_add_timestamps) + inspect_source(cop, migration_with_add_timestamps_with_timezone) + + expect(cop.offenses.size).to eq(0) + end + end +end diff --git a/spec/rubocop/cop/migration/datetime_spec.rb b/spec/rubocop/cop/migration/datetime_spec.rb new file mode 100644 index 00000000000..388b086ce6a --- /dev/null +++ b/spec/rubocop/cop/migration/datetime_spec.rb @@ -0,0 +1,90 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/datetime' + +describe RuboCop::Cop::Migration::Datetime do + include CopHelper + + subject(:cop) { described_class.new } + let(:migration_with_datetime) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column(:users, :username, :text) + add_column(:users, :last_sign_in, :datetime) + end + end + ) + end + + let(:migration_without_datetime) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column(:users, :username, :text) + end + end + ) + end + + let(:migration_with_datetime_with_timezone) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column(:users, :username, :text) + add_column(:users, :last_sign_in, :datetime_with_timezone) + end + end + ) + end + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + it 'registers an offense when the ":datetime" data type is used' do + inspect_source(cop, migration_with_datetime) + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([7]) + end + end + + it 'does not register an offense when the ":datetime" data type is not used' do + inspect_source(cop, migration_without_datetime) + + aggregate_failures do + expect(cop.offenses.size).to eq(0) + end + end + + it 'does not register an offense when the ":datetime_with_timezone" data type is used' do + inspect_source(cop, migration_with_datetime_with_timezone) + + aggregate_failures do + expect(cop.offenses.size).to eq(0) + end + end + end + + context 'outside of migration' do + it 'registers no offense' do + inspect_source(cop, migration_with_datetime) + inspect_source(cop, migration_without_datetime) + inspect_source(cop, migration_with_datetime_with_timezone) + + expect(cop.offenses.size).to eq(0) + end + end +end diff --git a/spec/rubocop/cop/migration/timestamps_spec.rb b/spec/rubocop/cop/migration/timestamps_spec.rb new file mode 100644 index 00000000000..cdf1423d0c5 --- /dev/null +++ b/spec/rubocop/cop/migration/timestamps_spec.rb @@ -0,0 +1,99 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/timestamps' + +describe RuboCop::Cop::Migration::Timestamps do + include CopHelper + + subject(:cop) { described_class.new } + let(:migration_with_timestamps) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + create_table :users do |t| + t.string :username, null: false + t.timestamps null: true + t.string :password + end + end + end + ) + end + + let(:migration_without_timestamps) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + create_table :users do |t| + t.string :username, null: false + t.string :password + end + end + end + ) + end + + let(:migration_with_timestamps_with_timezone) do + %q( + class Users < ActiveRecord::Migration + DOWNTIME = false + + def change + create_table :users do |t| + t.string :username, null: false + t.timestamps_with_timezone null: true + t.string :password + end + end + end + ) + end + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + it 'registers an offense when the "timestamps" method is used' do + inspect_source(cop, migration_with_timestamps) + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([8]) + end + end + + it 'does not register an offense when the "timestamps" method is not used' do + inspect_source(cop, migration_without_timestamps) + + aggregate_failures do + expect(cop.offenses.size).to eq(0) + end + end + + it 'does not register an offense when the "timestamps_with_timezone" method is used' do + inspect_source(cop, migration_with_timestamps_with_timezone) + + aggregate_failures do + expect(cop.offenses.size).to eq(0) + end + end + end + + context 'outside of migration' do + it 'registers no offense' do + inspect_source(cop, migration_with_timestamps) + inspect_source(cop, migration_without_timestamps) + inspect_source(cop, migration_with_timestamps_with_timezone) + + expect(cop.offenses.size).to eq(0) + end + end +end diff --git a/spec/serializers/build_details_entity_spec.rb b/spec/serializers/build_details_entity_spec.rb index e2511e8968c..b92c1c28ba8 100644 --- a/spec/serializers/build_details_entity_spec.rb +++ b/spec/serializers/build_details_entity_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe BuildDetailsEntity do set(:user) { create(:admin) } - it 'inherits from BuildEntity' do - expect(described_class).to be < BuildEntity + it 'inherits from JobEntity' do + expect(described_class).to be < JobEntity end describe '#as_json' do @@ -29,7 +29,7 @@ describe BuildDetailsEntity do it 'contains the needed key value pairs' do expect(subject).to include(:coverage, :erased_at, :duration) - expect(subject).to include(:artifacts, :runner, :pipeline) + expect(subject).to include(:runner, :pipeline) expect(subject).to include(:raw_path, :merge_request) expect(subject).to include(:new_issue_path) end diff --git a/spec/serializers/build_entity_spec.rb b/spec/serializers/job_entity_spec.rb index e51ff9fc709..5ca7bf2fcaf 100644 --- a/spec/serializers/build_entity_spec.rb +++ b/spec/serializers/job_entity_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' -describe BuildEntity do +describe JobEntity do let(:user) { create(:user) } - let(:build) { create(:ci_build) } - let(:project) { build.project } + let(:job) { create(:ci_build) } + let(:project) { job.project } let(:request) { double('request') } before do @@ -12,12 +12,12 @@ describe BuildEntity do end let(:entity) do - described_class.new(build, request: request) + described_class.new(job, request: request) end subject { entity.as_json } - it 'contains paths to build page action' do + it 'contains paths to job page action' do expect(subject).to include(:build_path) end @@ -27,7 +27,7 @@ describe BuildEntity do end it 'contains whether it is playable' do - expect(subject[:playable]).to eq build.playable? + expect(subject[:playable]).to eq job.playable? end it 'contains timestamps' do @@ -39,9 +39,9 @@ describe BuildEntity do expect(subject[:status]).to include :icon, :favicon, :text, :label end - context 'when build is retryable' do + context 'when job is retryable' do before do - build.update(status: :failed) + job.update(status: :failed) end it 'contains cancel path' do @@ -49,9 +49,9 @@ describe BuildEntity do end end - context 'when build is cancelable' do + context 'when job is cancelable' do before do - build.update(status: :running) + job.update(status: :running) end it 'contains cancel path' do @@ -59,7 +59,7 @@ describe BuildEntity do end end - context 'when build is a regular build' do + context 'when job is a regular job' do it 'does not contain path to play action' do expect(subject).not_to include(:play_path) end @@ -69,8 +69,8 @@ describe BuildEntity do end end - context 'when build is a manual action' do - let(:build) { create(:ci_build, :manual) } + context 'when job is a manual action' do + let(:job) { create(:ci_build, :manual) } context 'when user is allowed to trigger action' do before do @@ -99,4 +99,25 @@ describe BuildEntity do end end end + + context 'when job is generic commit status' do + let(:job) { create(:generic_commit_status, target_url: 'http://google.com') } + + it 'contains paths to target action' do + expect(subject).to include(:build_path) + end + + it 'does not contain paths to other action paths' do + expect(subject).not_to include(:retry_path, :cancel_path, :play_path) + end + + it 'contains timestamps' do + expect(subject).to include(:created_at, :updated_at) + end + + it 'contains details' do + expect(subject).to include :status + expect(subject[:status]).to include :icon, :favicon, :text, :label + end + end end diff --git a/spec/serializers/pipeline_details_entity_spec.rb b/spec/serializers/pipeline_details_entity_spec.rb index 03cc5ae9b63..5cb9b9945b6 100644 --- a/spec/serializers/pipeline_details_entity_spec.rb +++ b/spec/serializers/pipeline_details_entity_spec.rb @@ -91,6 +91,20 @@ describe PipelineDetailsEntity do end end + context 'when pipeline has commit statuses' do + let(:pipeline) { create(:ci_empty_pipeline) } + + before do + create(:generic_commit_status, pipeline: pipeline) + end + + it 'contains stages' do + expect(subject).to include(:details) + expect(subject[:details]).to include(:stages) + expect(subject[:details][:stages].first).to include(name: 'external') + end + end + context 'when pipeline has YAML errors' do let(:pipeline) do create(:ci_pipeline, config: { rspec: { invalid: :value } }) diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index 088f24eb180..34b19fb9fc4 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -102,18 +102,11 @@ describe PipelineSerializer do Ci::Pipeline::AVAILABLE_STATUSES.each do |status| create_pipeline(status) end - - RequestStore.begin! - end - - after do - RequestStore.end! - RequestStore.clear! end - it "verifies number of queries" do + it 'verifies number of queries', :request_store do recorded = ActiveRecord::QueryRecorder.new { subject } - expect(recorded.count).to be_within(1).of(60) + expect(recorded.count).to be_within(1).of(57) expect(recorded.cached_count).to eq(0) end diff --git a/spec/serializers/stage_entity_spec.rb b/spec/serializers/stage_entity_spec.rb index 64b3217b809..40e303f7b89 100644 --- a/spec/serializers/stage_entity_spec.rb +++ b/spec/serializers/stage_entity_spec.rb @@ -54,6 +54,17 @@ describe StageEntity do it 'exposes the group key' do expect(subject).to include :groups end + + context 'and contains commit status' do + before do + create(:generic_commit_status, pipeline: pipeline, stage: 'test') + end + + it 'contains commit status' do + groups = subject[:groups].map { |group| group[:name] } + expect(groups).to include('generic') + end + end end end end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 5184c1d5f19..a78866a2c32 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -31,6 +31,13 @@ describe Issues::UpdateService, services: true do end end + def find_notes(action) + issue + .notes + .joins(:system_note_metadata) + .where(system_note_metadata: { action: action }) + end + def update_issue(opts) described_class.new(project, user, opts).execute(issue) end @@ -330,6 +337,9 @@ describe Issues::UpdateService, services: true do expect(note1).not_to be_nil expect(note2).not_to be_nil + + description_notes = find_notes('description') + expect(description_notes.length).to eq(1) end end @@ -345,6 +355,9 @@ describe Issues::UpdateService, services: true do expect(note1).not_to be_nil expect(note2).not_to be_nil + + description_notes = find_notes('description') + expect(description_notes.length).to eq(1) end end @@ -354,10 +367,12 @@ describe Issues::UpdateService, services: true do update_issue(description: "- [x] Task 1\n- [ ] Task 3\n- [ ] Task 2") end - it 'does not create a system note' do - note = find_note('marked the task **Task 2** as incomplete') + it 'does not create a system note for the task' do + task_note = find_note('marked the task **Task 2** as incomplete') + description_notes = find_notes('description') - expect(note).to be_nil + expect(task_note).to be_nil + expect(description_notes.length).to eq(2) end end @@ -368,9 +383,11 @@ describe Issues::UpdateService, services: true do end it 'does not create a system note referencing the position the old item' do - note = find_note('marked the task **Two** as incomplete') + task_note = find_note('marked the task **Two** as incomplete') + description_notes = find_notes('description') - expect(note).to be_nil + expect(task_note).to be_nil + expect(description_notes.length).to eq(2) end it 'does not generate a new note at all' do diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index d371fc68312..091c193aaa6 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -30,6 +30,13 @@ describe MergeRequests::UpdateService, services: true do end end + def find_notes(action) + @merge_request + .notes + .joins(:system_note_metadata) + .where(system_note_metadata: { action: action }) + end + def update_merge_request(opts) @merge_request = MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) @merge_request.reload @@ -394,6 +401,9 @@ describe MergeRequests::UpdateService, services: true do expect(note1).not_to be_nil expect(note2).not_to be_nil + + description_notes = find_notes('description') + expect(description_notes.length).to eq(1) end end @@ -409,6 +419,9 @@ describe MergeRequests::UpdateService, services: true do expect(note1).not_to be_nil expect(note2).not_to be_nil + + description_notes = find_notes('description') + expect(description_notes.length).to eq(1) end end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index c499b1bb343..9295c09aefc 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -1052,7 +1052,7 @@ describe SystemNoteService, services: true do let(:action) { 'task' } end - it "posts the 'marked as a Work In Progress from commit' system note" do + it "posts the 'marked the task as complete' system note" do expect(subject.note).to eq("marked the task **task** as completed") end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 8b8fbf6e862..a81d3573f8d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -74,10 +74,18 @@ RSpec.configure do |config| TestEnv.cleanup end + config.before(:example, :request_store) do + RequestStore.begin! + end + + config.after(:example, :request_store) do + RequestStore.end! + RequestStore.clear! + end + if ENV['CI'] - # Retry only on feature specs that use JS - config.around :each, :js do |ex| - ex.run_with_retry retry: 3 + config.around(:each) do |ex| + ex.run_with_retry retry: 2 end end @@ -98,15 +106,13 @@ RSpec.configure do |config| Sidekiq.redis(&:flushall) end - config.around(:example, :migration) do |example| - begin - ActiveRecord::Migrator - .migrate(migrations_paths, previous_migration.version) + config.before(:example, :migration) do + ActiveRecord::Migrator + .migrate(migrations_paths, previous_migration.version) + end - example.run - ensure - ActiveRecord::Migrator.migrate(migrations_paths) - end + config.after(:example, :migration) do + ActiveRecord::Migrator.migrate(migrations_paths) end config.around(:each, :nested_groups) do |example| diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index b8ca8f22a3d..c34e76fa72f 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -14,8 +14,10 @@ Capybara.register_driver :poltergeist do |app| js_errors: true, timeout: timeout, window_size: [1366, 768], + url_whitelist: %w[localhost 127.0.0.1], + url_blacklist: %w[.mp4 .png .gif .avi .bmp .jpg .jpeg], phantomjs_options: [ - '--load-images=no' + '--load-images=yes' ] ) end diff --git a/spec/uploaders/artifact_uploader_spec.rb b/spec/uploaders/artifact_uploader_spec.rb index 24e2e3a9f0e..2a3bd0e3bb2 100644 --- a/spec/uploaders/artifact_uploader_spec.rb +++ b/spec/uploaders/artifact_uploader_spec.rb @@ -17,22 +17,45 @@ describe ArtifactUploader do describe '.artifacts_upload_path' do subject { described_class.artifacts_upload_path } - + it { is_expected.to start_with(path) } it { is_expected.to end_with('tmp/uploads/') } end describe '#store_dir' do subject { uploader.store_dir } - + it { is_expected.to start_with(path) } it { is_expected.to end_with("#{job.project_id}/#{job.id}") } end describe '#cache_dir' do subject { uploader.cache_dir } - + + it { is_expected.to start_with(path) } + it { is_expected.to end_with('/tmp/cache') } + end + + describe '#work_dir' do + subject { uploader.work_dir } + it { is_expected.to start_with(path) } - it { is_expected.to end_with('tmp/cache') } + it { is_expected.to end_with('/tmp/work') } + end + + describe '#filename' do + # we need to use uploader, as this makes to use mounter + # which initialises uploader.file object + let(:uploader) { job.artifacts_file } + + subject { uploader.filename } + + it { is_expected.to be_nil } + + context 'with artifacts' do + let(:job) { create(:ci_build, :artifacts) } + + it { is_expected.not_to be_nil } + end end end diff --git a/spec/uploaders/gitlab_uploader_spec.rb b/spec/uploaders/gitlab_uploader_spec.rb index 78e9d9cf46c..a144b39f74f 100644 --- a/spec/uploaders/gitlab_uploader_spec.rb +++ b/spec/uploaders/gitlab_uploader_spec.rb @@ -53,4 +53,19 @@ describe GitlabUploader do expect(subject.move_to_store).to eq(true) end end + + describe '#cache!' do + it 'moves the file from the working directory to the cache directory' do + # One to get the work dir, the other to remove it + expect(subject).to receive(:workfile_path).exactly(2).times.and_call_original + # Test https://github.com/carrierwavesubject/carrierwave/blob/v1.0.0/lib/carrierwave/sanitized_file.rb#L200 + expect(FileUtils).to receive(:mv).with(anything, /^#{subject.work_dir}/).and_call_original + expect(FileUtils).to receive(:mv).with(/^#{subject.work_dir}/, /#{subject.cache_dir}/).and_call_original + + fixture = Rails.root.join('spec', 'fixtures', 'rails_sample.jpg') + subject.cache!(fixture_file_upload(fixture)) + + expect(subject.file.path).to match(/#{subject.cache_dir}/) + end + end end diff --git a/spec/uploaders/lfs_object_uploader_spec.rb b/spec/uploaders/lfs_object_uploader_spec.rb index c3b72e7d677..7088bc23334 100644 --- a/spec/uploaders/lfs_object_uploader_spec.rb +++ b/spec/uploaders/lfs_object_uploader_spec.rb @@ -1,21 +1,9 @@ require 'spec_helper' describe LfsObjectUploader do - let(:uploader) { described_class.new(build_stubbed(:empty_project)) } - - describe '#cache!' do - it 'caches the file in the cache directory' do - # One to get the work dir, the other to remove it - expect(uploader).to receive(:workfile_path).exactly(2).times.and_call_original - expect(FileUtils).to receive(:mv).with(anything, /^#{uploader.work_dir}/).and_call_original - expect(FileUtils).to receive(:mv).with(/^#{uploader.work_dir}/, /^#{uploader.cache_dir}/).and_call_original - - fixture = Rails.root.join('spec', 'fixtures', 'rails_sample.jpg') - uploader.cache!(fixture_file_upload(fixture)) - - expect(uploader.file.path).to start_with(uploader.cache_dir) - end - end + let(:lfs_object) { create(:lfs_object, :with_file) } + let(:uploader) { described_class.new(lfs_object) } + let(:path) { Gitlab.config.lfs.storage_path } describe '#move_to_cache' do it 'is true' do @@ -28,4 +16,25 @@ describe LfsObjectUploader do expect(uploader.move_to_store).to eq(true) end end + + describe '#store_dir' do + subject { uploader.store_dir } + + it { is_expected.to start_with(path) } + it { is_expected.to end_with("#{lfs_object.oid[0, 2]}/#{lfs_object.oid[2, 2]}") } + end + + describe '#cache_dir' do + subject { uploader.cache_dir } + + it { is_expected.to start_with(path) } + it { is_expected.to end_with('/tmp/cache') } + end + + describe '#work_dir' do + subject { uploader.work_dir } + + it { is_expected.to start_with(path) } + it { is_expected.to end_with('/tmp/work') } + end end diff --git a/spec/workers/background_migration_worker_spec.rb b/spec/workers/background_migration_worker_spec.rb new file mode 100644 index 00000000000..0d742ae9dc7 --- /dev/null +++ b/spec/workers/background_migration_worker_spec.rb @@ -0,0 +1,13 @@ +require 'spec_helper' + +describe BackgroundMigrationWorker do + describe '.perform' do + it 'performs a background migration' do + expect(Gitlab::BackgroundMigration). + to receive(:perform). + with('Foo', [10, 20]) + + described_class.new.perform('Foo', [10, 20]) + end + end +end |