diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-03 18:08:33 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-03 18:08:33 +0300 |
commit | 511e761b41b81484c85e3d125f45873ce38e9201 (patch) | |
tree | 6bb98a6356de6e1d736951d2eef6ec83e6aa3dd2 /spec | |
parent | 4247e67be1faa9d52691757dad954a7fa63e8bfe (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/application_controller_spec.rb | 46 | ||||
-rw-r--r-- | spec/frontend/notes/components/discussion_notes_spec.js | 18 | ||||
-rw-r--r-- | spec/lib/gitlab/application_context_spec.rb | 82 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/yaml_processor_spec.rb | 13 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/project_tree_restorer_spec.rb | 5 | ||||
-rw-r--r-- | spec/lib/gitlab/profiler_spec.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/sidekiq_middleware/correlation_injector_spec.rb | 49 | ||||
-rw-r--r-- | spec/lib/gitlab/sidekiq_middleware/correlation_logger_spec.rb | 35 | ||||
-rw-r--r-- | spec/lib/gitlab/sidekiq_middleware_spec.rb | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/utils/lazy_attributes_spec.rb | 70 | ||||
-rw-r--r-- | spec/requests/api/internal/base_spec.rb | 12 | ||||
-rw-r--r-- | spec/requests/api/projects_spec.rb | 8 | ||||
-rw-r--r-- | spec/services/ci/find_exposed_artifacts_service_spec.rb | 29 | ||||
-rw-r--r-- | spec/spec_helper.rb | 6 | ||||
-rw-r--r-- | spec/support/shared_examples/logging_application_context_shared_examples.rb | 24 |
15 files changed, 306 insertions, 97 deletions
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index e72ab16f62a..0c299dcda34 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -895,4 +895,50 @@ describe ApplicationController do end end end + + context '#set_current_context' do + controller(described_class) do + def index + Labkit::Context.with_context do |context| + render json: context.to_h + end + end + end + + let_it_be(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'does not break anything when no group or project method is defined' do + get :index + + expect(response).to have_gitlab_http_status(:success) + end + + it 'sets the username in the context when signed in' do + get :index + + expect(json_response['meta.user']).to eq(user.username) + end + + it 'sets the group if it was available' do + group = build_stubbed(:group) + controller.instance_variable_set(:@group, group) + + get :index, format: :json + + expect(json_response['meta.root_namespace']).to eq(group.path) + end + + it 'sets the project if one was available' do + project = build_stubbed(:project) + controller.instance_variable_set(:@project, project) + + get :index, format: :json + + expect(json_response['meta.project']).to eq(project.full_path) + end + end end diff --git a/spec/frontend/notes/components/discussion_notes_spec.js b/spec/frontend/notes/components/discussion_notes_spec.js index 51c7f45d5b0..d259e79de84 100644 --- a/spec/frontend/notes/components/discussion_notes_spec.js +++ b/spec/frontend/notes/components/discussion_notes_spec.js @@ -6,7 +6,6 @@ import NoteableNote from '~/notes/components/noteable_note.vue'; import PlaceholderNote from '~/vue_shared/components/notes/placeholder_note.vue'; import PlaceholderSystemNote from '~/vue_shared/components/notes/placeholder_system_note.vue'; import SystemNote from '~/vue_shared/components/notes/system_note.vue'; -import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue'; import createStore from '~/notes/stores'; import { noteableDataMock, discussionMock, notesDataMock } from '../../notes/mock_data'; @@ -48,13 +47,13 @@ describe('DiscussionNotes', () => { it('renders an element for each note in the discussion', () => { createComponent(); const notesCount = discussionMock.notes.length; - const els = wrapper.findAll(TimelineEntryItem); + const els = wrapper.findAll(NoteableNote); expect(els.length).toBe(notesCount); }); it('renders one element if replies groupping is enabled', () => { createComponent({ shouldGroupReplies: true }); - const els = wrapper.findAll(TimelineEntryItem); + const els = wrapper.findAll(NoteableNote); expect(els.length).toBe(1); }); @@ -85,7 +84,7 @@ describe('DiscussionNotes', () => { ]; discussion.notes = notesData; createComponent({ discussion, shouldRenderDiffs: true }); - const notes = wrapper.findAll('.notes > li'); + const notes = wrapper.findAll('.notes > *'); expect(notes.at(0).is(PlaceholderSystemNote)).toBe(true); expect(notes.at(1).is(PlaceholderNote)).toBe(true); @@ -111,7 +110,14 @@ describe('DiscussionNotes', () => { describe('events', () => { describe('with groupped notes and replies expanded', () => { - const findNoteAtIndex = index => wrapper.find(`.note:nth-of-type(${index + 1}`); + const findNoteAtIndex = index => { + const noteComponents = [NoteableNote, SystemNote, PlaceholderNote, PlaceholderSystemNote]; + const allowedNames = noteComponents.map(c => c.name); + return wrapper + .findAll('.notes *') + .filter(w => allowedNames.includes(w.name())) + .at(index); + }; beforeEach(() => { createComponent({ shouldGroupReplies: true, isExpanded: true }); @@ -146,7 +152,7 @@ describe('DiscussionNotes', () => { let note; beforeEach(() => { createComponent(); - note = wrapper.find('.note'); + note = wrapper.find('.notes > *'); }); it('emits deleteNote when first note emits handleDeleteNote', () => { diff --git a/spec/lib/gitlab/application_context_spec.rb b/spec/lib/gitlab/application_context_spec.rb new file mode 100644 index 00000000000..1c8fcb8d737 --- /dev/null +++ b/spec/lib/gitlab/application_context_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::ApplicationContext do + describe '.with_context' do + it 'yields the block' do + expect { |b| described_class.with_context({}, &b) }.to yield_control + end + + it 'passes the expected context on to labkit' do + fake_proc = duck_type(:call) + expected_context = hash_including(user: fake_proc, project: fake_proc, root_namespace: fake_proc) + + expect(Labkit::Context).to receive(:with_context).with(expected_context) + + described_class.with_context( + user: build(:user), + project: build(:project), + namespace: build(:namespace)) {} + end + + it 'raises an error when passing invalid options' do + expect { described_class.with_context(no: 'option') {} }.to raise_error(ArgumentError) + end + end + + describe '.push' do + it 'passes the expected context on to labkit' do + fake_proc = duck_type(:call) + expected_context = hash_including(user: fake_proc, project: fake_proc, root_namespace: fake_proc) + + expect(Labkit::Context).to receive(:push).with(expected_context) + + described_class.push(user: build(:user)) + end + + it 'raises an error when passing invalid options' do + expect { described_class.push(no: 'option')}.to raise_error(ArgumentError) + end + end + + describe '#to_lazy_hash' do + let(:user) { build(:user) } + let(:project) { build(:project) } + let(:namespace) { build(:group) } + let(:subgroup) { build(:group, parent: namespace) } + + def result(context) + context.to_lazy_hash.transform_values { |v| v.call } + end + + it 'does not call the attributes until needed' do + fake_proc = double('Proc') + + expect(fake_proc).not_to receive(:call) + + described_class.new(user: fake_proc, project: fake_proc, namespace: fake_proc).to_lazy_hash + end + + it 'correctly loads the expected values when they are wrapped in a block' do + context = described_class.new(user: -> { user }, project: -> { project }, namespace: -> { subgroup }) + + expect(result(context)) + .to include(user: user.username, project: project.full_path, root_namespace: namespace.full_path) + end + + it 'correctly loads the expected values when passed directly' do + context = described_class.new(user: user, project: project, namespace: subgroup) + + expect(result(context)) + .to include(user: user.username, project: project.full_path, root_namespace: namespace.full_path) + end + + it 'falls back to a projects namespace when a project is passed but no namespace' do + context = described_class.new(project: project) + + expect(result(context)) + .to include(project: project.full_path, root_namespace: project.full_path_components.first) + end + end +end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index f61b28b06c8..2e470d59345 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -1270,6 +1270,19 @@ module Gitlab expect(builds.first[:options][:artifacts][:when]).to eq(when_state) end end + + it "gracefully handles errors in artifacts type" do + config = <<~YAML + test: + script: + - echo "Hello world" + artifacts: + - paths: + - test/ + YAML + + expect { described_class.new(config) }.to raise_error(described_class::ValidationError) + end end describe '#environment' do diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index ec1b935ad63..dae5b028c15 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -654,13 +654,10 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do let(:user) { create(:user) } let!(:project) { create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project') } let(:project_tree_restorer) { described_class.new(user: user, shared: shared, project: project) } - let(:correlation_id) { 'my-correlation-id' } before do setup_import_export_config('with_invalid_records') - # Import is running from the rake task, `correlation_id` is not assigned - expect(Labkit::Correlation::CorrelationId).to receive(:new_id).and_return(correlation_id) subject end @@ -682,7 +679,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect(import_failure.relation_index).to be_present expect(import_failure.exception_class).to eq('ActiveRecord::RecordInvalid') expect(import_failure.exception_message).to be_present - expect(import_failure.correlation_id_value).to eq('my-correlation-id') + expect(import_failure.correlation_id_value).not_to be_empty expect(import_failure.created_at).to be_present end end diff --git a/spec/lib/gitlab/profiler_spec.rb b/spec/lib/gitlab/profiler_spec.rb index c27e3ca7ace..8f6fb6eda65 100644 --- a/spec/lib/gitlab/profiler_spec.rb +++ b/spec/lib/gitlab/profiler_spec.rb @@ -84,7 +84,7 @@ describe Gitlab::Profiler do expect(severity).to eq(Logger::DEBUG) expect(message).to include('public').and include(described_class::FILTERED_STRING) expect(message).not_to include(private_token) - end.twice + end.at_least(1) # This spec could be wrapped in more blocks in the future custom_logger.debug("public #{private_token}") end diff --git a/spec/lib/gitlab/sidekiq_middleware/correlation_injector_spec.rb b/spec/lib/gitlab/sidekiq_middleware/correlation_injector_spec.rb deleted file mode 100644 index d5ed939485a..00000000000 --- a/spec/lib/gitlab/sidekiq_middleware/correlation_injector_spec.rb +++ /dev/null @@ -1,49 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::SidekiqMiddleware::CorrelationInjector do - class TestWorker - include ApplicationWorker - end - - before do |example| - Sidekiq.client_middleware do |chain| - chain.add described_class - end - end - - after do |example| - Sidekiq.client_middleware do |chain| - chain.remove described_class - end - - Sidekiq::Queues.clear_all - end - - around do |example| - Sidekiq::Testing.fake! do - example.run - end - end - - it 'injects into payload the correlation id' do - expect_next_instance_of(described_class) do |instance| - expect(instance).to receive(:call).and_call_original - end - - Labkit::Correlation::CorrelationId.use_id('new-correlation-id') do - TestWorker.perform_async(1234) - end - - expected_job_params = { - "class" => "TestWorker", - "args" => [1234], - "correlation_id" => "new-correlation-id" - } - - expect(Sidekiq::Queues.jobs_by_worker).to a_hash_including( - "TestWorker" => a_collection_containing_exactly( - a_hash_including(expected_job_params))) - end -end diff --git a/spec/lib/gitlab/sidekiq_middleware/correlation_logger_spec.rb b/spec/lib/gitlab/sidekiq_middleware/correlation_logger_spec.rb deleted file mode 100644 index 27eea963402..00000000000 --- a/spec/lib/gitlab/sidekiq_middleware/correlation_logger_spec.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::SidekiqMiddleware::CorrelationLogger do - class TestWorker - include ApplicationWorker - end - - before do |example| - Sidekiq::Testing.server_middleware do |chain| - chain.add described_class - end - end - - after do |example| - Sidekiq::Testing.server_middleware do |chain| - chain.remove described_class - end - end - - it 'injects into payload the correlation id', :sidekiq_might_not_need_inline do - expect_any_instance_of(described_class).to receive(:call).and_call_original - - expect_any_instance_of(TestWorker).to receive(:perform).with(1234) do - expect(Labkit::Correlation::CorrelationId.current_id).to eq('new-correlation-id') - end - - Sidekiq::Client.push( - 'queue' => 'test', - 'class' => TestWorker, - 'args' => [1234], - 'correlation_id' => 'new-correlation-id') - end -end diff --git a/spec/lib/gitlab/sidekiq_middleware_spec.rb b/spec/lib/gitlab/sidekiq_middleware_spec.rb index aef472e0648..ef4a898bdb6 100644 --- a/spec/lib/gitlab/sidekiq_middleware_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware_spec.rb @@ -38,7 +38,7 @@ describe Gitlab::SidekiqMiddleware do [ Gitlab::SidekiqMiddleware::Monitor, Gitlab::SidekiqMiddleware::BatchLoader, - Gitlab::SidekiqMiddleware::CorrelationLogger, + Labkit::Middleware::Sidekiq::Server, Gitlab::SidekiqMiddleware::InstrumentationLogger, Gitlab::SidekiqStatus::ServerMiddleware, Gitlab::SidekiqMiddleware::Metrics, @@ -120,7 +120,7 @@ describe Gitlab::SidekiqMiddleware do # This test ensures that this does not happen it "invokes the chain" do expect_any_instance_of(Gitlab::SidekiqStatus::ClientMiddleware).to receive(:call).with(*middleware_expected_args).once.and_call_original - expect_any_instance_of(Gitlab::SidekiqMiddleware::CorrelationInjector).to receive(:call).with(*middleware_expected_args).once.and_call_original + expect_any_instance_of(Labkit::Middleware::Sidekiq::Client).to receive(:call).with(*middleware_expected_args).once.and_call_original expect { |b| chain.invoke(worker_class_arg, job, queue, redis_pool, &b) }.to yield_control.once end diff --git a/spec/lib/gitlab/utils/lazy_attributes_spec.rb b/spec/lib/gitlab/utils/lazy_attributes_spec.rb new file mode 100644 index 00000000000..c0005c194c4 --- /dev/null +++ b/spec/lib/gitlab/utils/lazy_attributes_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true +require 'fast_spec_helper' +require 'active_support/concern' + +describe Gitlab::Utils::LazyAttributes do + subject(:klass) do + Class.new do + include Gitlab::Utils::LazyAttributes + + lazy_attr_reader :number, type: Numeric + lazy_attr_reader :reader_1, :reader_2 + lazy_attr_accessor :incorrect_type, :string_attribute, :accessor_2, type: String + + def initialize + @number = -> { 1 } + @reader_1, @reader_2 = 'reader_1', -> { 'reader_2' } + @incorrect_type, @accessor_2 = -> { :incorrect_type }, -> { 'accessor_2' } + end + end + end + + context 'class methods' do + it { is_expected.to respond_to(:lazy_attr_reader, :lazy_attr_accessor) } + it { is_expected.not_to respond_to(:define_lazy_reader, :define_lazy_writer) } + end + + context 'instance methods' do + subject(:instance) { klass.new } + + it do + is_expected.to respond_to(:number, :reader_1, :reader_2, :incorrect_type, + :incorrect_type=, :accessor_2, :accessor_2=, + :string_attribute, :string_attribute=) + end + + context 'reading attributes' do + it 'returns the correct values for procs', :aggregate_failures do + expect(instance.number).to eq(1) + expect(instance.reader_2).to eq('reader_2') + expect(instance.accessor_2).to eq('accessor_2') + end + + it 'does not return the value if the type did not match what was specified' do + expect(instance.incorrect_type).to be_nil + end + + it 'only calls the block once even if it returned `nil`', :aggregate_failures do + expect(instance.instance_variable_get('@number')).to receive(:call).once.and_call_original + expect(instance.instance_variable_get('@accessor_2')).to receive(:call).once.and_call_original + expect(instance.instance_variable_get('@incorrect_type')).to receive(:call).once.and_call_original + + 2.times do + instance.number + instance.incorrect_type + instance.accessor_2 + end + end + end + + context 'writing attributes' do + it 'sets the correct values', :aggregate_failures do + instance.string_attribute = -> { 'updated 1' } + instance.accessor_2 = nil + + expect(instance.string_attribute).to eq('updated 1') + expect(instance.accessor_2).to be_nil + end + end + end +end diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 4fe0d4a5983..12e6e7c7a09 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -389,6 +389,12 @@ describe API::Internal::Base do end end end + + it_behaves_like 'storing arguments in the application context' do + let(:expected_params) { { user: key.user.username, project: project.full_path } } + + subject { push(key, project) } + end end context "access denied" do @@ -885,6 +891,12 @@ describe API::Internal::Base do post api('/internal/post_receive'), params: valid_params end + it_behaves_like 'storing arguments in the application context' do + let(:expected_params) { { user: user.username, project: project.full_path } } + + subject { post api('/internal/post_receive'), params: valid_params } + end + context 'when there are merge_request push options' do before do valid_params[:push_options] = ['merge_request.create'] diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 9af4f484f99..975e59346b8 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1626,6 +1626,14 @@ describe API::Projects do end end end + + it_behaves_like 'storing arguments in the application context' do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :public) } + let(:expected_params) { { user: user.username, project: project.full_path } } + + subject { get api("/projects/#{project.id}", user) } + end end describe 'GET /projects/:id/users' do diff --git a/spec/services/ci/find_exposed_artifacts_service_spec.rb b/spec/services/ci/find_exposed_artifacts_service_spec.rb index f6309822fe0..b0f190b0e7a 100644 --- a/spec/services/ci/find_exposed_artifacts_service_spec.rb +++ b/spec/services/ci/find_exposed_artifacts_service_spec.rb @@ -50,10 +50,39 @@ describe Ci::FindExposedArtifactsService do end end + shared_examples 'does not find any matches' do + it 'returns empty array' do + expect(subject).to eq [] + end + end + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } subject { described_class.new(project, user).for_pipeline(pipeline) } + context 'with jobs having no exposed artifacts' do + let!(:job) do + create_job_with_artifacts(artifacts: { + paths: ['other_artifacts_0.1.2/doc_sample.txt', 'something-else.html'] + }) + end + + it_behaves_like 'does not find any matches' + end + + context 'with jobs having no artifacts (metadata)' do + let!(:job) do + create(:ci_build, pipeline: pipeline, options: { + artifacts: { + expose_as: 'Exposed artifact', + paths: ['other_artifacts_0.1.2/doc_sample.txt', 'something-else.html'] + } + }) + end + + it_behaves_like 'does not find any matches' + end + context 'with jobs having at most 1 matching exposed artifact' do let!(:job) do create_job_with_artifacts(artifacts: { diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1f0119108a8..6393e482904 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -245,6 +245,12 @@ RSpec.configure do |config| Rails.cache = caching_store end + config.around do |example| + # Wrap each example in it's own context to make sure the contexts don't + # leak + Labkit::Context.with_context { example.run } + end + config.around(:each, :clean_gitlab_redis_cache) do |example| redis_cache_cleanup! diff --git a/spec/support/shared_examples/logging_application_context_shared_examples.rb b/spec/support/shared_examples/logging_application_context_shared_examples.rb new file mode 100644 index 00000000000..038ede884c8 --- /dev/null +++ b/spec/support/shared_examples/logging_application_context_shared_examples.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'storing arguments in the application context' do + around do |example| + Labkit::Context.with_context { example.run } + end + + it 'places the expected params in the application context' do + # Stub the clearing of the context so we can validate it later + # The `around` block above makes sure we do clean it up later + allow(Labkit::Context).to receive(:pop) + + subject + + Labkit::Context.with_context do |context| + expect(context.to_h) + .to include(log_hash(expected_params)) + end + end + + def log_hash(hash) + hash.transform_keys! { |key| "meta.#{key}" } + end +end |