From 7519a25a6161b2ca540ca9fc4e42e4b03aed4eeb Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Tue, 5 Jun 2018 17:21:33 +0100 Subject: Use `started` key to show the triggered date --- spec/javascripts/jobs/header_spec.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/javascripts/jobs/header_spec.js b/spec/javascripts/jobs/header_spec.js index 4f861c39d3f..cef30a007db 100644 --- a/spec/javascripts/jobs/header_spec.js +++ b/spec/javascripts/jobs/header_spec.js @@ -13,6 +13,9 @@ describe('Job details header', () => { const threeWeeksAgo = new Date(); threeWeeksAgo.setDate(threeWeeksAgo.getDate() - 21); + const twoDaysAgo = new Date(); + twoDaysAgo.setDate(twoDaysAgo.getDate() - 2); + props = { job: { status: { @@ -31,7 +34,7 @@ describe('Job details header', () => { email: 'foo@bar.com', avatar_url: 'link', }, - started: '2018-01-08T09:48:27.319Z', + started: twoDaysAgo.toISOString(), new_issue_path: 'path', }, isLoading: false, @@ -69,7 +72,7 @@ describe('Job details header', () => { .querySelector('.header-main-content') .textContent.replace(/\s+/g, ' ') .trim(), - ).toEqual('failed Job #123 triggered 3 weeks ago by Foo'); + ).toEqual('failed Job #123 triggered 2 days ago by Foo'); }); it('should render new issue link', () => { -- cgit v1.2.3 From fc916b068199f2624c5b9cfeb0389760448910ff Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Sun, 27 May 2018 22:02:47 +0200 Subject: Migrate jobs in object_storage_upload queue --- ...ate_object_storage_upload_sidekiq_queue_spec.rb | 33 ++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 spec/migrations/migrate_object_storage_upload_sidekiq_queue_spec.rb (limited to 'spec') diff --git a/spec/migrations/migrate_object_storage_upload_sidekiq_queue_spec.rb b/spec/migrations/migrate_object_storage_upload_sidekiq_queue_spec.rb new file mode 100644 index 00000000000..1ee6c440cf4 --- /dev/null +++ b/spec/migrations/migrate_object_storage_upload_sidekiq_queue_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20180603190921_migrate_object_storage_upload_sidekiq_queue.rb') + +describe MigrateObjectStorageUploadSidekiqQueue, :sidekiq, :redis do + include Gitlab::Database::MigrationHelpers + + context 'when there are jobs in the queue' do + it 'correctly migrates queue when migrating up' do + Sidekiq::Testing.disable! do + stubbed_worker(queue: 'object_storage_upload').perform_async('Something', [1]) + stubbed_worker(queue: 'object_storage:object_storage_background_move').perform_async('Something', [1]) + + described_class.new.up + + expect(sidekiq_queue_length('object_storage_upload')).to eq 0 + expect(sidekiq_queue_length('object_storage:object_storage_background_move')).to eq 2 + end + end + end + + context 'when there are no jobs in the queues' do + it 'does not raise error when migrating up' do + expect { described_class.new.up }.not_to raise_error + end + end + + def stubbed_worker(queue:) + Class.new do + include Sidekiq::Worker + sidekiq_options queue: queue + end + end +end -- cgit v1.2.3 From 9c6c17cbcdb8bf8185fc1b873dcfd08f723e4df5 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Wed, 16 Aug 2017 14:04:41 +0100 Subject: Add a minimal GraphQL API --- spec/controllers/graphql_controller_spec.rb | 58 ++++++++++++++++++++++++ spec/graphql/gitlab_schema_spec.rb | 27 +++++++++++ spec/graphql/loaders/full_path_loader_spec.rb | 38 ++++++++++++++++ spec/graphql/loaders/iid_loader_spec.rb | 64 +++++++++++++++++++++++++++ spec/graphql/types/query_type_spec.rb | 37 ++++++++++++++++ spec/graphql/types/time_type_spec.rb | 16 +++++++ spec/lib/gitlab/path_regex_spec.rb | 12 ++--- spec/support/helpers/graphql_helpers.rb | 29 ++++++++++++ spec/support/matchers/graphql_matchers.rb | 31 +++++++++++++ 9 files changed, 307 insertions(+), 5 deletions(-) create mode 100644 spec/controllers/graphql_controller_spec.rb create mode 100644 spec/graphql/gitlab_schema_spec.rb create mode 100644 spec/graphql/loaders/full_path_loader_spec.rb create mode 100644 spec/graphql/loaders/iid_loader_spec.rb create mode 100644 spec/graphql/types/query_type_spec.rb create mode 100644 spec/graphql/types/time_type_spec.rb create mode 100644 spec/support/helpers/graphql_helpers.rb create mode 100644 spec/support/matchers/graphql_matchers.rb (limited to 'spec') diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb new file mode 100644 index 00000000000..d6689dbc3c6 --- /dev/null +++ b/spec/controllers/graphql_controller_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe GraphqlController do + describe 'execute' do + before do + sign_in(user) if user + + run_test_query! + end + + subject { query_response } + + context 'graphql is disabled by feature flag' do + let(:user) { nil } + + before do + stub_feature_flags(graphql: false) + end + + it 'returns 404' do + run_test_query! + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'signed out' do + let(:user) { nil } + + it 'runs the query with current_user: nil' do + is_expected.to eq('echo' => 'nil says: test success') + end + end + + context 'signed in' do + let(:user) { create(:user, username: 'Simon') } + + it 'runs the query with current_user set' do + is_expected.to eq('echo' => '"Simon" says: test success') + end + end + end + + # Chosen to exercise all the moving parts in GraphqlController#execute + def run_test_query! + query = <<~QUERY + query Echo($text: String) { + echo(text: $text) + } + QUERY + + post :execute, query: query, operationName: 'Echo', variables: { 'text' => 'test success' } + end + + def query_response + json_response['data'] + end +end diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb new file mode 100644 index 00000000000..3582f297866 --- /dev/null +++ b/spec/graphql/gitlab_schema_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe GitlabSchema do + it 'uses batch loading' do + expect(described_class.instrumenters[:multiplex]).to include(GraphQL::Batch::SetupMultiplex) + end + + it 'enables the preload instrumenter' do + expect(field_instrumenters).to include(instance_of(::GraphQL::Preload::Instrument)) + end + + it 'enables the authorization instrumenter' do + expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Authorize)) + end + + it 'has the base mutation' do + expect(described_class.mutation).to eq(::Types::MutationType) + end + + it 'has the base query' do + expect(described_class.query).to eq(::Types::QueryType) + end + + def field_instrumenters + described_class.instrumenters[:field] + end +end diff --git a/spec/graphql/loaders/full_path_loader_spec.rb b/spec/graphql/loaders/full_path_loader_spec.rb new file mode 100644 index 00000000000..2a473239550 --- /dev/null +++ b/spec/graphql/loaders/full_path_loader_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe Loaders::FullPathLoader do + include GraphqlHelpers + + set(:project1) { create(:project) } + set(:project2) { create(:project) } + + set(:other_project) { create(:project) } + + describe '.project' do + it 'batch-resolves projects by full path' do + paths = [project1.full_path, project2.full_path] + + result = batch(max_queries: 1) do + paths.map { |path| resolve_project(path) } + end + + expect(result).to contain_exactly(project1, project2) + end + + it 'resolves an unknown full_path to nil' do + result = batch { resolve_project('unknown/project') } + + expect(result).to be_nil + end + + it 'returns a promise' do + batch do + expect(resolve_project(project1.full_path)).to be_a(Promise) + end + end + end + + def resolve_project(full_path) + resolve(described_class, :project, args: { full_path: full_path }) + end +end diff --git a/spec/graphql/loaders/iid_loader_spec.rb b/spec/graphql/loaders/iid_loader_spec.rb new file mode 100644 index 00000000000..8a0c1f0791a --- /dev/null +++ b/spec/graphql/loaders/iid_loader_spec.rb @@ -0,0 +1,64 @@ +require 'spec_helper' + +describe Loaders::IidLoader do + include GraphqlHelpers + + set(:project) { create(:project, :repository) } + set(:merge_request_1) { create(:merge_request, :simple, source_project: project, target_project: project) } + set(:merge_request_2) { create(:merge_request, :rebased, source_project: project, target_project: project) } + + set(:other_project) { create(:project, :repository) } + set(:other_merge_request) { create(:merge_request, source_project: other_project, target_project: other_project) } + + let(:full_path) { project.full_path } + let(:iid_1) { merge_request_1.iid } + let(:iid_2) { merge_request_2.iid } + + let(:other_full_path) { other_project.full_path } + let(:other_iid) { other_merge_request.iid } + + describe '.merge_request' do + it 'batch-resolves merge requests by target project full path and IID' do + path = full_path # avoid database query + + result = batch(max_queries: 2) do + [resolve_mr(path, iid_1), resolve_mr(path, iid_2)] + end + + expect(result).to contain_exactly(merge_request_1, merge_request_2) + end + + it 'can batch-resolve merge requests from different projects' do + path = project.full_path # avoid database queries + other_path = other_full_path + + result = batch(max_queries: 3) do + [resolve_mr(path, iid_1), resolve_mr(path, iid_2), resolve_mr(other_path, other_iid)] + end + + expect(result).to contain_exactly(merge_request_1, merge_request_2, other_merge_request) + end + + it 'resolves an unknown iid to nil' do + result = batch { resolve_mr(full_path, -1) } + + expect(result).to be_nil + end + + it 'resolves a known iid for an unknown full_path to nil' do + result = batch { resolve_mr('unknown/project', iid_1) } + + expect(result).to be_nil + end + + it 'returns a promise' do + batch do + expect(resolve_mr(full_path, iid_1)).to be_a(Promise) + end + end + end + + def resolve_mr(full_path, iid) + resolve(described_class, :merge_request, args: { project: full_path, iid: iid }) + end +end diff --git a/spec/graphql/types/query_type_spec.rb b/spec/graphql/types/query_type_spec.rb new file mode 100644 index 00000000000..17d9395504c --- /dev/null +++ b/spec/graphql/types/query_type_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe GitlabSchema.types['Query'] do + it 'is called Query' do + expect(described_class.name).to eq('Query') + end + + it { is_expected.to have_graphql_fields(:project, :merge_request, :echo) } + + describe 'project field' do + subject { described_class.fields['project'] } + + it 'finds projects by full path' do + is_expected.to have_graphql_arguments(:full_path) + is_expected.to have_graphql_type(Types::ProjectType) + is_expected.to have_graphql_resolver(Loaders::FullPathLoader[:project]) + end + + it 'authorizes with read_project' do + is_expected.to require_graphql_authorizations(:read_project) + end + end + + describe 'merge_request field' do + subject { described_class.fields['merge_request'] } + + it 'finds MRs by project and IID' do + is_expected.to have_graphql_arguments(:project, :iid) + is_expected.to have_graphql_type(Types::MergeRequestType) + is_expected.to have_graphql_resolver(Loaders::IidLoader[:merge_request]) + end + + it 'authorizes with read_merge_request' do + is_expected.to require_graphql_authorizations(:read_merge_request) + end + end +end diff --git a/spec/graphql/types/time_type_spec.rb b/spec/graphql/types/time_type_spec.rb new file mode 100644 index 00000000000..087655cc67d --- /dev/null +++ b/spec/graphql/types/time_type_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe GitlabSchema.types['Time'] do + let(:float) { 1504630455.96215 } + let(:time) { Time.at(float) } + + it { expect(described_class.name).to eq('Time') } + + it 'coerces Time into fractional seconds since epoch' do + expect(described_class.coerce_isolated_result(time)).to eq(float) + end + + it 'coerces fractional seconds since epoch into Time' do + expect(described_class.coerce_isolated_input(float)).to eq(time) + end +end diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb index a40330d853f..e90e0aba0a4 100644 --- a/spec/lib/gitlab/path_regex_spec.rb +++ b/spec/lib/gitlab/path_regex_spec.rb @@ -90,11 +90,13 @@ describe Gitlab::PathRegex do let(:routes_not_starting_in_wildcard) { routes_without_format.select { |p| p !~ %r{^/[:*]} } } let(:top_level_words) do - words = routes_not_starting_in_wildcard.map do |route| - route.split('/')[1] - end.compact - - (words + ee_top_level_words + files_in_public + Array(API::API.prefix.to_s)).uniq + routes_not_starting_in_wildcard + .map { |route| route.split('/')[1] } + .concat(ee_top_level_words) + .concat(files_in_public) + .concat(Array(API::API.prefix.to_s)) + .compact + .uniq end let(:ee_top_level_words) do diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb new file mode 100644 index 00000000000..5bb2cf9dd9e --- /dev/null +++ b/spec/support/helpers/graphql_helpers.rb @@ -0,0 +1,29 @@ +module GraphqlHelpers + # Run a loader's named resolver + def resolve(kls, name, obj: nil, args: {}, ctx: {}) + kls[name].call(obj, args, ctx) + end + + # Runs a block inside a GraphQL::Batch wrapper + def batch(max_queries: nil, &blk) + wrapper = proc do + GraphQL::Batch.batch do + result = yield + + if result.is_a?(Array) + Promise.all(result) + else + result + end + end + end + + if max_queries + result = nil + expect { result = wrapper.call }.not_to exceed_query_limit(max_queries) + result + else + wrapper.call + end + end +end diff --git a/spec/support/matchers/graphql_matchers.rb b/spec/support/matchers/graphql_matchers.rb new file mode 100644 index 00000000000..c0ed16ecaba --- /dev/null +++ b/spec/support/matchers/graphql_matchers.rb @@ -0,0 +1,31 @@ +RSpec::Matchers.define :require_graphql_authorizations do |*expected| + match do |field| + authorizations = field.metadata[:authorize] + + expect(authorizations).to contain_exactly(*expected) + end +end + +RSpec::Matchers.define :have_graphql_fields do |*expected| + match do |kls| + expect(kls.fields.keys).to contain_exactly(*expected.map(&:to_s)) + end +end + +RSpec::Matchers.define :have_graphql_arguments do |*expected| + match do |field| + expect(field.arguments.keys).to contain_exactly(*expected.map(&:to_s)) + end +end + +RSpec::Matchers.define :have_graphql_type do |expected| + match do |field| + expect(field.type).to eq(expected) + end +end + +RSpec::Matchers.define :have_graphql_resolver do |expected| + match do |field| + expect(field.resolve_proc).to eq(expected) + end +end -- cgit v1.2.3 From 287c34ca1f9af4e395493c99623af8437f82d919 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Fri, 23 Feb 2018 15:36:40 +0000 Subject: Convert from GraphQL::Batch to BatchLoader --- spec/graphql/gitlab_schema_spec.rb | 2 +- spec/graphql/loaders/full_path_loader_spec.rb | 6 ------ spec/graphql/loaders/iid_loader_spec.rb | 6 ------ spec/support/helpers/graphql_helpers.rb | 17 +++++++---------- 4 files changed, 8 insertions(+), 23 deletions(-) (limited to 'spec') diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb index 3582f297866..070b851b109 100644 --- a/spec/graphql/gitlab_schema_spec.rb +++ b/spec/graphql/gitlab_schema_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe GitlabSchema do it 'uses batch loading' do - expect(described_class.instrumenters[:multiplex]).to include(GraphQL::Batch::SetupMultiplex) + expect(field_instrumenters).to include(BatchLoader::GraphQL) end it 'enables the preload instrumenter' do diff --git a/spec/graphql/loaders/full_path_loader_spec.rb b/spec/graphql/loaders/full_path_loader_spec.rb index 2a473239550..2732dd8c9da 100644 --- a/spec/graphql/loaders/full_path_loader_spec.rb +++ b/spec/graphql/loaders/full_path_loader_spec.rb @@ -24,12 +24,6 @@ describe Loaders::FullPathLoader do expect(result).to be_nil end - - it 'returns a promise' do - batch do - expect(resolve_project(project1.full_path)).to be_a(Promise) - end - end end def resolve_project(full_path) diff --git a/spec/graphql/loaders/iid_loader_spec.rb b/spec/graphql/loaders/iid_loader_spec.rb index 8a0c1f0791a..0a57d7c4ed4 100644 --- a/spec/graphql/loaders/iid_loader_spec.rb +++ b/spec/graphql/loaders/iid_loader_spec.rb @@ -50,12 +50,6 @@ describe Loaders::IidLoader do expect(result).to be_nil end - - it 'returns a promise' do - batch do - expect(resolve_mr(full_path, iid_1)).to be_a(Promise) - end - end end def resolve_mr(full_path, iid) diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index 5bb2cf9dd9e..1eaa7603af0 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -4,20 +4,17 @@ module GraphqlHelpers kls[name].call(obj, args, ctx) end - # Runs a block inside a GraphQL::Batch wrapper + # Runs a block inside a BatchLoader::Executor wrapper def batch(max_queries: nil, &blk) wrapper = proc do - GraphQL::Batch.batch do - result = yield - - if result.is_a?(Array) - Promise.all(result) - else - result - end + begin + BatchLoader::Executor.ensure_current + blk.call + ensure + BatchLoader::Executor.clear_current end end - + if max_queries result = nil expect { result = wrapper.call }.not_to exceed_query_limit(max_queries) -- cgit v1.2.3 From aa4b1ae71260720b47695b8a256134f20280f61a Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Mon, 21 May 2018 09:52:24 +0200 Subject: Add `present_using` to types By specifying a presenter for the object type, we can keep the logic out of `GitlabSchema`. The presenter gets initialized using the object being presented, and the context (including the `current_user`). --- spec/graphql/gitlab_schema_spec.rb | 7 +++++ spec/requests/graphql/merge_request_query_spec.rb | 24 ++++++++++++++++ spec/requests/graphql/project_query_spec.rb | 23 +++++++++++++++ spec/support/helpers/graphql_helpers.rb | 33 ++++++++++++++++++++-- .../requests/graphql_shared_examples.rb | 18 ++++++++++++ 5 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 spec/requests/graphql/merge_request_query_spec.rb create mode 100644 spec/requests/graphql/project_query_spec.rb create mode 100644 spec/support/shared_examples/requests/graphql_shared_examples.rb (limited to 'spec') diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb index 070b851b109..f25cc2fd6c9 100644 --- a/spec/graphql/gitlab_schema_spec.rb +++ b/spec/graphql/gitlab_schema_spec.rb @@ -13,7 +13,14 @@ describe GitlabSchema do expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Authorize)) end + it 'enables using presenters' do + expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Present)) + end + it 'has the base mutation' do + pending <<~REASON + Having empty mutations breaks the automatic documentation in Graphiql, so removed for now." + REASON expect(described_class.mutation).to eq(::Types::MutationType) end diff --git a/spec/requests/graphql/merge_request_query_spec.rb b/spec/requests/graphql/merge_request_query_spec.rb new file mode 100644 index 00000000000..cbc19782e2f --- /dev/null +++ b/spec/requests/graphql/merge_request_query_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe 'getting merge request information' do + include GraphqlHelpers + + let(:project) { create(:project, :repository, :public) } + let(:merge_request) { create(:merge_request, source_project: project) } + + let(:query) do + <<~QUERY + { + merge_request(project: "#{merge_request.project.full_path}", iid: "#{merge_request.iid}") { + #{all_graphql_fields_for(MergeRequest)} + } + } + QUERY + end + + it_behaves_like 'a working graphql query' do + it 'renders a merge request with all fields' do + expect(response_data['merge_request']).not_to be_nil + end + end +end diff --git a/spec/requests/graphql/project_query_spec.rb b/spec/requests/graphql/project_query_spec.rb new file mode 100644 index 00000000000..110ed433e03 --- /dev/null +++ b/spec/requests/graphql/project_query_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe 'getting project information' do + include GraphqlHelpers + + let(:project) { create(:project, :repository, :public) } + + let(:query) do + <<~QUERY + { + project(full_path: "#{project.full_path}") { + #{all_graphql_fields_for(Project)} + } + } + QUERY + end + + it_behaves_like 'a working graphql query' do + it 'renders a project with all fields' do + expect(response_data['project']).not_to be_nil + end + end +end diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index 1eaa7603af0..ae29b16e32c 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -9,12 +9,12 @@ module GraphqlHelpers wrapper = proc do begin BatchLoader::Executor.ensure_current - blk.call + yield ensure BatchLoader::Executor.clear_current end end - + if max_queries result = nil expect { result = wrapper.call }.not_to exceed_query_limit(max_queries) @@ -23,4 +23,33 @@ module GraphqlHelpers wrapper.call end end + + def all_graphql_fields_for(klass) + type = GitlabSchema.types[klass.name] + return "" unless type + + type.fields.map do |name, field| + if scalar?(field) + name + else + "#{name} { #{all_graphql_fields_for(field_type(field))} }" + end + end.join("\n") + end + + def post_graphql(query) + post '/api/graphql', query: query + end + + def scalar?(field) + field_type(field).kind.scalar? + end + + def field_type(field) + if field.type.respond_to?(:of_type) + field.type.of_type + else + field.type + end + end end diff --git a/spec/support/shared_examples/requests/graphql_shared_examples.rb b/spec/support/shared_examples/requests/graphql_shared_examples.rb new file mode 100644 index 00000000000..c143b83696e --- /dev/null +++ b/spec/support/shared_examples/requests/graphql_shared_examples.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +shared_examples 'a working graphql query' do + include GraphqlHelpers + + let(:parsed_response) { JSON.parse(response.body) } + let(:response_data) { parsed_response['data'] } + + before do + post_graphql(query) + end + + it 'is returns a successfull response', :aggregate_failures do + expect(response).to be_success + expect(parsed_response['errors']).to be_nil + expect(response_data).not_to be_empty + end +end -- cgit v1.2.3 From c443133e779c4c508b9c6429dd4ba623d64f03f1 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Mon, 21 May 2018 13:42:07 +0200 Subject: Handle exceptions outside the GraphQL schema This allows us to report JSON parse exceptions to clients and ignore them in sentry. --- spec/controllers/graphql_controller_spec.rb | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index d6689dbc3c6..1449036e148 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' describe GraphqlController do describe 'execute' do + let(:user) { nil } + before do sign_in(user) if user @@ -39,17 +41,26 @@ describe GraphqlController do is_expected.to eq('echo' => '"Simon" says: test success') end end + + context 'invalid variables' do + it 'returns an error' do + run_test_query!(variables: "This is not JSON") + + expect(response).to have_gitlab_http_status(422) + expect(json_response['errors'].first['message']).not_to be_nil + end + end end # Chosen to exercise all the moving parts in GraphqlController#execute - def run_test_query! + def run_test_query!(variables: { 'text' => 'test success' }) query = <<~QUERY query Echo($text: String) { echo(text: $text) } QUERY - post :execute, query: query, operationName: 'Echo', variables: { 'text' => 'test success' } + post :execute, query: query, operationName: 'Echo', variables: variables end def query_response -- cgit v1.2.3 From bbafb85395cf2786f21a5a92745b9b3a181a4ce8 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Tue, 5 Jun 2018 21:42:18 +1000 Subject: Add missing tests around rendering invalid merge requests (HTML+JSON) --- .../projects/merge_requests_controller_spec.rb | 20 ++++++++++++++++++++ spec/factories/merge_requests.rb | 6 ++++++ 2 files changed, 26 insertions(+) (limited to 'spec') diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 6e8de6db9c3..6e710c9b20b 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -80,6 +80,16 @@ describe Projects::MergeRequestsController do )) end end + + context "that is invalid" do + let(:merge_request) { create(:invalid_merge_request, target_project: project, source_project: project) } + + it "renders merge request page" do + go(format: :html) + + expect(response).to be_success + end + end end describe 'as json' do @@ -106,6 +116,16 @@ describe Projects::MergeRequestsController do expect(response).to match_response_schema('entities/merge_request_widget') end end + + context "that is invalid" do + let(:merge_request) { create(:invalid_merge_request, target_project: project, source_project: project) } + + it "renders merge request page" do + go(format: :json) + + expect(response).to be_success + end + end end describe "as diff" do diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index fab0ec22450..3441ce1b8cb 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -54,6 +54,11 @@ FactoryBot.define do state :opened end + trait :invalid do + source_branch "feature_one" + target_branch "feature_two" + end + trait :locked do state :locked end @@ -98,6 +103,7 @@ FactoryBot.define do factory :merged_merge_request, traits: [:merged] factory :closed_merge_request, traits: [:closed] factory :reopened_merge_request, traits: [:opened] + factory :invalid_merge_request, traits: [:invalid] factory :merge_request_with_diffs, traits: [:with_diffs] factory :merge_request_with_diff_notes do after(:create) do |mr| -- cgit v1.2.3 From 2d1faddbcb3b814a381f91cf48cc597d47b29ee0 Mon Sep 17 00:00:00 2001 From: Jasper Maes Date: Wed, 6 Jun 2018 08:46:42 +0200 Subject: Rails 5 fix glob spec --- spec/lib/gitlab/sql/glob_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/lib/gitlab/sql/glob_spec.rb b/spec/lib/gitlab/sql/glob_spec.rb index f0bb4294d62..1cf8935bfe3 100644 --- a/spec/lib/gitlab/sql/glob_spec.rb +++ b/spec/lib/gitlab/sql/glob_spec.rb @@ -35,8 +35,9 @@ describe Gitlab::SQL::Glob do value = query("SELECT #{quote(string)} LIKE #{pattern}") .rows.flatten.first + check = Gitlab.rails5? ? true : 't' case value - when 't', 1 + when check, 1 true else false -- cgit v1.2.3 From 2d6022e0865642deb10d458d58d6b0b8c072e98e Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 1 Jun 2018 14:44:22 +0100 Subject: store specs --- spec/javascripts/ide/mock_data.js | 4 + .../ide/stores/modules/pipelines/actions_spec.js | 135 +++++++++++++++++++++ .../ide/stores/modules/pipelines/mutations_spec.js | 49 ++++++++ 3 files changed, 188 insertions(+) (limited to 'spec') diff --git a/spec/javascripts/ide/mock_data.js b/spec/javascripts/ide/mock_data.js index dcf857f7e04..dd87a43f370 100644 --- a/spec/javascripts/ide/mock_data.js +++ b/spec/javascripts/ide/mock_data.js @@ -75,6 +75,7 @@ export const jobs = [ }, stage: 'test', duration: 1, + started: new Date(), }, { id: 2, @@ -86,6 +87,7 @@ export const jobs = [ }, stage: 'test', duration: 1, + started: new Date(), }, { id: 3, @@ -97,6 +99,7 @@ export const jobs = [ }, stage: 'test', duration: 1, + started: new Date(), }, { id: 4, @@ -108,6 +111,7 @@ export const jobs = [ }, stage: 'build', duration: 1, + started: new Date(), }, ]; diff --git a/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js b/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js index f26eaf9c81f..f2f8e780cd1 100644 --- a/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js @@ -13,9 +13,15 @@ import actions, { receiveJobsSuccess, fetchJobs, toggleStageCollapsed, + setDetailJob, + requestJobTrace, + receiveJobTraceError, + receiveJobTraceSuccess, + fetchJobTrace, } from '~/ide/stores/modules/pipelines/actions'; import state from '~/ide/stores/modules/pipelines/state'; import * as types from '~/ide/stores/modules/pipelines/mutation_types'; +import { rightSidebarViews } from '~/ide/constants'; import testAction from '../../../../helpers/vuex_action_helper'; import { pipelines, jobs } from '../../../mock_data'; @@ -281,4 +287,133 @@ describe('IDE pipelines actions', () => { ); }); }); + + describe('setDetailJob', () => { + it('commits job', done => { + testAction( + setDetailJob, + 'job', + mockedState, + [{ type: types.SET_DETAIL_JOB, payload: 'job' }], + [{ type: 'setRightPane' }], + done, + ); + }); + + it('dispatches setRightPane as pipeline when job is null', done => { + testAction( + setDetailJob, + null, + mockedState, + [{ type: types.SET_DETAIL_JOB }], + [{ type: 'setRightPane', payload: rightSidebarViews.pipelines }], + done, + ); + }); + + it('dispatches setRightPane as job', done => { + testAction( + setDetailJob, + 'job', + mockedState, + [{ type: types.SET_DETAIL_JOB }], + [{ type: 'setRightPane', payload: rightSidebarViews.jobsDetail }], + done, + ); + }); + }); + + describe('requestJobTrace', () => { + it('commits request', done => { + testAction(requestJobTrace, null, mockedState, [{ type: types.REQUEST_JOB_TRACE }], [], done); + }); + }); + + describe('receiveJobTraceError', () => { + it('commits error', done => { + testAction( + receiveJobTraceError, + null, + mockedState, + [{ type: types.RECEIVE_JOB_TRACE_ERROR }], + [], + done, + ); + }); + + it('creates flash message', () => { + const flashSpy = spyOnDependency(actions, 'flash'); + + receiveJobTraceError({ commit() {} }); + + expect(flashSpy).toHaveBeenCalled(); + }); + }); + + describe('receiveJobTraceSuccess', () => { + it('commits data', done => { + testAction( + receiveJobTraceSuccess, + 'data', + mockedState, + [{ type: types.RECEIVE_JOB_TRACE_SUCCESS, payload: 'data' }], + [], + done, + ); + }); + }); + + describe('fetchJobTrace', () => { + beforeEach(() => { + mockedState.detailJob = { + path: `${gl.TEST_HOST}/project/builds`, + }; + }); + + describe('success', () => { + beforeEach(() => { + spyOn(axios, 'get').and.callThrough(); + mock.onGet(`${gl.TEST_HOST}/project/builds/trace`).replyOnce(200, { html: 'html' }); + }); + + it('dispatches request', done => { + testAction( + fetchJobTrace, + null, + mockedState, + [], + [ + { type: 'requestJobTrace' }, + { type: 'receiveJobTraceSuccess', payload: { html: 'html' } }, + ], + done, + ); + }); + + it('sends get request to correct URL', () => { + fetchJobTrace({ state: mockedState, dispatch() {} }); + + expect(axios.get).toHaveBeenCalledWith(`${gl.TEST_HOST}/project/builds/trace`, { + params: { format: 'json' }, + }); + }); + }); + + describe('error', () => { + beforeEach(() => { + mock.onGet(`${gl.TEST_HOST}/project/builds/trace`).replyOnce(500); + }); + + it('dispatches error', done => { + testAction( + fetchJobTrace, + null, + mockedState, + [], + [{ type: 'requestJobTrace' }, { type: 'receiveJobTraceError' }], + done, + ); + }); + }); + }); }); diff --git a/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js b/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js index 6285c01d483..eb7346bd5fc 100644 --- a/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js +++ b/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js @@ -147,6 +147,10 @@ describe('IDE pipelines mutations', () => { name: job.name, status: job.status, path: job.build_path, + rawPath: `${job.build_path}/raw`, + started: job.started, + isLoading: false, + output: '', })), ); }); @@ -171,4 +175,49 @@ describe('IDE pipelines mutations', () => { expect(mockedState.stages[0].isCollapsed).toBe(false); }); }); + + describe(types.SET_DETAIL_JOB, () => { + it('sets detail job', () => { + mutations[types.SET_DETAIL_JOB](mockedState, jobs[0]); + + expect(mockedState.detailJob).toEqual(jobs[0]); + }); + }); + + describe(types.REQUEST_JOB_TRACE, () => { + beforeEach(() => { + mockedState.detailJob = { ...jobs[0] }; + }); + + it('sets loading on detail job', () => { + mutations[types.REQUEST_JOB_TRACE](mockedState); + + expect(mockedState.detailJob.isLoading).toBe(true); + }); + }); + + describe(types.RECEIVE_JOB_TRACE_ERROR, () => { + beforeEach(() => { + mockedState.detailJob = { ...jobs[0], isLoading: true }; + }); + + it('sets loading to false on detail job', () => { + mutations[types.RECEIVE_JOB_TRACE_ERROR](mockedState); + + expect(mockedState.detailJob.isLoading).toBe(false); + }); + }); + + describe(types.RECEIVE_JOB_TRACE_SUCCESS, () => { + beforeEach(() => { + mockedState.detailJob = { ...jobs[0], isLoading: true }; + }); + + it('sets output on detail job', () => { + mutations[types.RECEIVE_JOB_TRACE_SUCCESS](mockedState, { html: 'html' }); + + expect(mockedState.detailJob.output).toBe('html'); + expect(mockedState.detailJob.isLoading).toBe(false); + }); + }); }); -- cgit v1.2.3 From f73b2c8e5ee0e02407b6674b9b0021ce4843349b Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Sat, 2 Jun 2018 14:45:58 +0100 Subject: component specs --- .../ide/components/jobs/detail/description_spec.js | 28 ++++ .../components/jobs/detail/scroll_button_spec.js | 59 +++++++ .../javascripts/ide/components/jobs/detail_spec.js | 170 +++++++++++++++++++++ 3 files changed, 257 insertions(+) create mode 100644 spec/javascripts/ide/components/jobs/detail/description_spec.js create mode 100644 spec/javascripts/ide/components/jobs/detail/scroll_button_spec.js create mode 100644 spec/javascripts/ide/components/jobs/detail_spec.js (limited to 'spec') diff --git a/spec/javascripts/ide/components/jobs/detail/description_spec.js b/spec/javascripts/ide/components/jobs/detail/description_spec.js new file mode 100644 index 00000000000..9b715a41499 --- /dev/null +++ b/spec/javascripts/ide/components/jobs/detail/description_spec.js @@ -0,0 +1,28 @@ +import Vue from 'vue'; +import Description from '~/ide/components/jobs/detail/description.vue'; +import mountComponent from '../../../../helpers/vue_mount_component_helper'; +import { jobs } from '../../../mock_data'; + +describe('IDE job description', () => { + const Component = Vue.extend(Description); + let vm; + + beforeEach(() => { + vm = mountComponent(Component, { + job: jobs[0], + }); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('renders job details', () => { + expect(vm.$el.textContent).toContain('#1'); + expect(vm.$el.textContent).toContain('test'); + }); + + it('renders CI icon', () => { + expect(vm.$el.querySelector('.ci-status-icon .ic-status_passed_borderless')).not.toBe(null); + }); +}); diff --git a/spec/javascripts/ide/components/jobs/detail/scroll_button_spec.js b/spec/javascripts/ide/components/jobs/detail/scroll_button_spec.js new file mode 100644 index 00000000000..fff382a107f --- /dev/null +++ b/spec/javascripts/ide/components/jobs/detail/scroll_button_spec.js @@ -0,0 +1,59 @@ +import Vue from 'vue'; +import ScrollButton from '~/ide/components/jobs/detail/scroll_button.vue'; +import mountComponent from '../../../../helpers/vue_mount_component_helper'; + +describe('IDE job log scroll button', () => { + const Component = Vue.extend(ScrollButton); + let vm; + + beforeEach(() => { + vm = mountComponent(Component, { + direction: 'up', + disabled: false, + }); + }); + + afterEach(() => { + vm.$destroy(); + }); + + describe('iconName', () => { + ['up', 'down'].forEach(direction => { + it(`returns icon name for ${direction}`, () => { + vm.direction = direction; + + expect(vm.iconName).toBe(`scroll_${direction}`); + }); + }); + }); + + describe('tooltipTitle', () => { + it('returns title for up', () => { + expect(vm.tooltipTitle).toBe('Scroll to top'); + }); + + it('returns title for down', () => { + vm.direction = 'down'; + + expect(vm.tooltipTitle).toBe('Scroll to bottom'); + }); + }); + + it('emits click event on click', () => { + spyOn(vm, '$emit'); + + vm.$el.querySelector('.btn-scroll').click(); + + expect(vm.$emit).toHaveBeenCalledWith('click'); + }); + + it('disables button when disabled is true', done => { + vm.disabled = true; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.btn-scroll').hasAttribute('disabled')).toBe(true); + + done(); + }); + }); +}); diff --git a/spec/javascripts/ide/components/jobs/detail_spec.js b/spec/javascripts/ide/components/jobs/detail_spec.js new file mode 100644 index 00000000000..00a2dc6c74c --- /dev/null +++ b/spec/javascripts/ide/components/jobs/detail_spec.js @@ -0,0 +1,170 @@ +import Vue from 'vue'; +import JobDetail from '~/ide/components/jobs/detail.vue'; +import { createStore } from '~/ide/stores'; +import { createComponentWithStore } from '../../../helpers/vue_mount_component_helper'; +import { jobs } from '../../mock_data'; + +describe('IDE jobs detail view', () => { + const Component = Vue.extend(JobDetail); + let vm; + + beforeEach(() => { + const store = createStore(); + + store.state.pipelines.detailJob = { + ...jobs[0], + isLoading: true, + output: 'testing', + rawPath: `${gl.TEST_HOST}/raw`, + }; + + vm = createComponentWithStore(Component, store); + + spyOn(vm, 'fetchJobTrace').and.returnValue(Promise.resolve()); + + vm = vm.$mount(); + + spyOn(vm.$refs.buildTrace, 'scrollTo'); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('calls fetchJobTrace on mount', () => { + expect(vm.fetchJobTrace).toHaveBeenCalled(); + }); + + it('scrolls to bottom on mount', done => { + setTimeout(() => { + expect(vm.$refs.buildTrace.scrollTo).toHaveBeenCalled(); + + done(); + }); + }); + + it('renders job output', () => { + expect(vm.$el.querySelector('.bash').textContent).toContain('testing'); + }); + + it('renders loading icon', () => { + expect(vm.$el.querySelector('.build-loader-animation')).not.toBe(null); + expect(vm.$el.querySelector('.build-loader-animation').style.display).toBe(''); + }); + + it('hide loading icon when isLoading is false', done => { + vm.$store.state.pipelines.detailJob.isLoading = false; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.build-loader-animation').style.display).toBe('none'); + + done(); + }); + }); + + it('resets detailJob when clicking header button', () => { + spyOn(vm, 'setDetailJob'); + + vm.$el.querySelector('.btn').click(); + + expect(vm.setDetailJob).toHaveBeenCalledWith(null); + }); + + it('renders raw path link', () => { + expect(vm.$el.querySelector('.controllers-buttons').getAttribute('href')).toBe( + `${gl.TEST_HOST}/raw`, + ); + }); + + describe('scroll buttons', () => { + it('triggers scrollDown when clicking down button', done => { + spyOn(vm, 'scrollDown'); + + vm.$el.querySelectorAll('.btn-scroll')[1].click(); + + vm.$nextTick(() => { + expect(vm.scrollDown).toHaveBeenCalled(); + + done(); + }); + }); + + it('triggers scrollUp when clicking up button', done => { + spyOn(vm, 'scrollUp'); + + vm.scrollPos = 1; + + vm + .$nextTick() + .then(() => vm.$el.querySelector('.btn-scroll').click()) + .then(() => vm.$nextTick()) + .then(() => { + expect(vm.scrollUp).toHaveBeenCalled(); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('scrollDown', () => { + it('scrolls build trace to bottom', () => { + spyOnProperty(vm.$refs.buildTrace, 'scrollHeight').and.returnValue(1000); + + vm.scrollDown(); + + expect(vm.$refs.buildTrace.scrollTo).toHaveBeenCalledWith(0, 1000); + }); + }); + + describe('scrollUp', () => { + it('scrolls build trace to top', () => { + vm.scrollUp(); + + expect(vm.$refs.buildTrace.scrollTo).toHaveBeenCalledWith(0, 0); + }); + }); + + describe('scrollBuildLog', () => { + beforeEach(() => { + spyOnProperty(vm.$refs.buildTrace, 'offsetHeight').and.returnValue(100); + spyOnProperty(vm.$refs.buildTrace, 'scrollHeight').and.returnValue(200); + }); + + it('sets scrollPos to bottom when at the bottom', done => { + spyOnProperty(vm.$refs.buildTrace, 'scrollTop').and.returnValue(100); + + vm.scrollBuildLog(); + + setTimeout(() => { + expect(vm.scrollPos).toBe(1); + + done(); + }); + }); + + it('sets scrollPos to top when at the top', done => { + spyOnProperty(vm.$refs.buildTrace, 'scrollTop').and.returnValue(0); + vm.scrollPos = 1; + + vm.scrollBuildLog(); + + setTimeout(() => { + expect(vm.scrollPos).toBe(0); + + done(); + }); + }); + + it('resets scrollPos when not at top or bottom', done => { + spyOnProperty(vm.$refs.buildTrace, 'scrollTop').and.returnValue(10); + + vm.scrollBuildLog(); + + setTimeout(() => { + expect(vm.scrollPos).toBe(''); + + done(); + }); + }); + }); +}); -- cgit v1.2.3 From ec37b1b20c09d3a5b5a0e2cd0b03311ec95d7c68 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 6 Jun 2018 08:43:15 +0100 Subject: added empty state --- spec/javascripts/ide/components/jobs/detail_spec.js | 10 ++++++++++ spec/javascripts/ide/components/jobs/item_spec.js | 10 ++++++++++ 2 files changed, 20 insertions(+) (limited to 'spec') diff --git a/spec/javascripts/ide/components/jobs/detail_spec.js b/spec/javascripts/ide/components/jobs/detail_spec.js index 00a2dc6c74c..641ba06f653 100644 --- a/spec/javascripts/ide/components/jobs/detail_spec.js +++ b/spec/javascripts/ide/components/jobs/detail_spec.js @@ -47,6 +47,16 @@ describe('IDE jobs detail view', () => { expect(vm.$el.querySelector('.bash').textContent).toContain('testing'); }); + it('renders empty message output', done => { + vm.$store.state.pipelines.detailJob.output = ''; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.bash').textContent).toContain('No messages were logged'); + + done(); + }); + }); + it('renders loading icon', () => { expect(vm.$el.querySelector('.build-loader-animation')).not.toBe(null); expect(vm.$el.querySelector('.build-loader-animation').style.display).toBe(''); diff --git a/spec/javascripts/ide/components/jobs/item_spec.js b/spec/javascripts/ide/components/jobs/item_spec.js index 7c1dd4e475c..79e07f00e7b 100644 --- a/spec/javascripts/ide/components/jobs/item_spec.js +++ b/spec/javascripts/ide/components/jobs/item_spec.js @@ -26,4 +26,14 @@ describe('IDE jobs item', () => { it('renders CI icon', () => { expect(vm.$el.querySelector('.ic-status_passed_borderless')).not.toBe(null); }); + + it('does not render view logs button if not started', done => { + vm.job.started = false; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.btn')).toBe(null); + + done(); + }); + }); }); -- cgit v1.2.3 From c8f0e4b5da0c9d578035e74b524f5adcb80efcf6 Mon Sep 17 00:00:00 2001 From: Imre Date: Fri, 18 May 2018 10:49:02 +0200 Subject: Add Avatar API --- spec/requests/api/avatar_spec.rb | 106 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 spec/requests/api/avatar_spec.rb (limited to 'spec') diff --git a/spec/requests/api/avatar_spec.rb b/spec/requests/api/avatar_spec.rb new file mode 100644 index 00000000000..26e0435a6d5 --- /dev/null +++ b/spec/requests/api/avatar_spec.rb @@ -0,0 +1,106 @@ +require 'spec_helper' + +describe API::Avatar do + let(:gravatar_service) { double('GravatarService') } + + describe 'GET /avatar' do + context 'avatar uploaded to GitLab' do + context 'user with matching public email address' do + let(:user) { create(:user, :with_avatar, email: 'public@example.com', public_email: 'public@example.com') } + + before do + user + end + + it 'returns the avatar url' do + get api('/avatar'), { email: 'public@example.com' } + + expect(response.status).to eq 200 + expect(json_response['avatar_url']).to eql("#{::Settings.gitlab.base_url}#{user.avatar.local_url}") + end + end + + context 'no user with matching public email address' do + before do + expect(GravatarService).to receive(:new).and_return(gravatar_service) + expect(gravatar_service).to( + receive(:execute) + .with('private@example.com', nil, 2, { username: nil }) + .and_return('https://gravatar')) + end + + it 'returns the avatar url from Gravatar' do + get api('/avatar'), { email: 'private@example.com' } + + expect(response.status).to eq 200 + expect(json_response['avatar_url']).to eq('https://gravatar') + end + end + end + + context 'avatar uploaded to Gravatar' do + context 'user with matching public email address' do + let(:user) { create(:user, email: 'public@example.com', public_email: 'public@example.com') } + + before do + user + + expect(GravatarService).to receive(:new).and_return(gravatar_service) + expect(gravatar_service).to( + receive(:execute) + .with('public@example.com', nil, 2, { username: user.username }) + .and_return('https://gravatar')) + end + + it 'returns the avatar url from Gravatar' do + get api('/avatar'), { email: 'public@example.com' } + + expect(response.status).to eq 200 + expect(json_response['avatar_url']).to eq('https://gravatar') + end + end + + context 'no user with matching public email address' do + before do + expect(GravatarService).to receive(:new).and_return(gravatar_service) + expect(gravatar_service).to( + receive(:execute) + .with('private@example.com', nil, 2, { username: nil }) + .and_return('https://gravatar')) + end + + it 'returns the avatar url from Gravatar' do + get api('/avatar'), { email: 'private@example.com' } + + expect(response.status).to eq 200 + expect(json_response['avatar_url']).to eq('https://gravatar') + end + end + + context 'public visibility level restricted' do + let(:user) { create(:user, :with_avatar, email: 'public@example.com', public_email: 'public@example.com') } + + before do + user + + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) + end + + context 'when authenticated' do + it 'returns the avatar url' do + get api('/avatar', user), { email: 'public@example.com' } + + expect(response.status).to eq 200 + expect(json_response['avatar_url']).to eql("#{::Settings.gitlab.base_url}#{user.avatar.local_url}") + end + end + + context 'when unauthenticated' do + it_behaves_like '403 response' do + let(:request) { get api('/avatar'), { email: 'public@example.com' } } + end + end + end + end + end +end -- cgit v1.2.3 From 3a722ff53fe86ce6194f81ade810196f4f8e870c Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 5 Jun 2018 22:34:06 -0700 Subject: Show a more helpful error for import status Importing a project from GitHub for a project namespace that already exists would show an unhelpful error, "An error occurred while importing project." We now add the base message from Projects::CreateService when this fails. Closes #47365 --- spec/javascripts/importer_status_spec.js | 18 ++++++++++++++++ .../githubish_import_controller_shared_examples.rb | 24 ++++++++++++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/javascripts/importer_status_spec.js b/spec/javascripts/importer_status_spec.js index 87b46ccf7c3..63cdb3d5114 100644 --- a/spec/javascripts/importer_status_spec.js +++ b/spec/javascripts/importer_status_spec.js @@ -50,6 +50,24 @@ describe('Importer Status', () => { }) .catch(done.fail); }); + + it('shows error message after failed POST request', (done) => { + appendSetFixtures('
'); + + mock.onPost(importUrl).reply(422, { + errors: 'You forgot your lunch', + }); + + instance.addToImport({ + currentTarget: document.querySelector('.js-add-to-import'), + }) + .then(() => { + const flashMessage = document.querySelector('.flash-text'); + expect(flashMessage.textContent.trim()).toEqual('An error occurred while importing project: You forgot your lunch'); + done(); + }) + .catch(done.fail); + }); }); describe('autoUpdate', () => { diff --git a/spec/support/controllers/githubish_import_controller_shared_examples.rb b/spec/support/controllers/githubish_import_controller_shared_examples.rb index 368439aa5b0..1a53c5fa487 100644 --- a/spec/support/controllers/githubish_import_controller_shared_examples.rb +++ b/spec/support/controllers/githubish_import_controller_shared_examples.rb @@ -118,14 +118,34 @@ shared_examples 'a GitHub-ish import controller: POST create' do expect(response).to have_gitlab_http_status(200) end - it 'returns 422 response when the project could not be imported' do + it 'returns 422 response with the base error when the project could not be imported' do + project = build(:project) + error_message = 'This is an error' + project.errors.add(:base, error_message) + + allow(Gitlab::LegacyGithubImport::ProjectCreator) + .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) + .and_return(double(execute: project)) + + post :create, format: :json + + expect(response).to have_gitlab_http_status(422) + expect(json_response['errors']).to eq(error_message) + end + + it 'returns 422 response with a combined error when the project could not be imported' do + project = build(:project) + project.errors.add(:name, 'is invalid') + project.errors.add(:path, 'is old') + allow(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) - .and_return(double(execute: build(:project))) + .and_return(double(execute: project)) post :create, format: :json expect(response).to have_gitlab_http_status(422) + expect(json_response['errors']).to eq('Name is invalid, Path is old') end context "when the repository owner is the provider user" do -- cgit v1.2.3 From 4b458ca79ad516cbc556ad9cd7e384c10bce037f Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 23 May 2018 15:51:50 +0900 Subject: Add tests --- .../workers/rescue_stale_live_trace_worker_spec.rb | 71 ++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 spec/workers/rescue_stale_live_trace_worker_spec.rb (limited to 'spec') diff --git a/spec/workers/rescue_stale_live_trace_worker_spec.rb b/spec/workers/rescue_stale_live_trace_worker_spec.rb new file mode 100644 index 00000000000..f2b6361c54b --- /dev/null +++ b/spec/workers/rescue_stale_live_trace_worker_spec.rb @@ -0,0 +1,71 @@ +require 'spec_helper' + +describe RescueStaleLiveTraceWorker do + subject { described_class.new.perform } + + before do + stub_feature_flags(ci_enable_live_trace: true) + end + + shared_examples_for 'schedules to archive traces' do + it do + expect(ArchiveTraceWorker).to receive(:bulk_perform_async).with([[build.id]]) + + subject + end + end + + shared_examples_for 'does not schedule to archive traces' do + it do + expect(ArchiveTraceWorker).not_to receive(:bulk_perform_async) + + subject + end + end + + context 'when a job was succeeded 2 hours ago' do + let!(:build) { create(:ci_build, :success, :trace_live) } + + before do + build.update(finished_at: 2.hours.ago) + end + + it_behaves_like 'schedules to archive traces' + end + + context 'when a job was failed 2 hours ago' do + let!(:build) { create(:ci_build, :failed, :trace_live) } + + before do + build.update(finished_at: 2.hours.ago) + end + + it_behaves_like 'schedules to archive traces' + end + + context 'when a job was cancelled 2 hours ago' do + let!(:build) { create(:ci_build, :canceled, :trace_live) } + + before do + build.update(finished_at: 2.hours.ago) + end + + it_behaves_like 'schedules to archive traces' + end + + context 'when a job has been finished 10 minutes ago' do + let!(:build) { create(:ci_build, :success, :trace_live) } + + before do + build.update(finished_at: 10.minutes.ago) + end + + it_behaves_like 'does not schedule to archive traces' + end + + context 'when a job is running' do + let!(:build) { create(:ci_build, :running, :trace_live) } + + it_behaves_like 'does not schedule to archive traces' + end +end -- cgit v1.2.3 From fb1e35e556f50636645d48c739dad37a4c7f722c Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 31 May 2018 16:29:17 +0900 Subject: Fix spec fiel location --- .../ci/rescue_stale_live_trace_worker_spec.rb | 71 ++++++++++++++++++++++ .../workers/rescue_stale_live_trace_worker_spec.rb | 71 ---------------------- 2 files changed, 71 insertions(+), 71 deletions(-) create mode 100644 spec/workers/ci/rescue_stale_live_trace_worker_spec.rb delete mode 100644 spec/workers/rescue_stale_live_trace_worker_spec.rb (limited to 'spec') diff --git a/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb b/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb new file mode 100644 index 00000000000..87796fd66db --- /dev/null +++ b/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb @@ -0,0 +1,71 @@ +require 'spec_helper' + +describe Ci::RescueStaleLiveTraceWorker do + subject { described_class.new.perform } + + before do + stub_feature_flags(ci_enable_live_trace: true) + end + + shared_examples_for 'schedules to archive traces' do + it do + expect(ArchiveTraceWorker).to receive(:bulk_perform_async).with([[build.id]]) + + subject + end + end + + shared_examples_for 'does not schedule to archive traces' do + it do + expect(ArchiveTraceWorker).not_to receive(:bulk_perform_async) + + subject + end + end + + context 'when a job was succeeded 2 hours ago' do + let!(:build) { create(:ci_build, :success, :trace_live) } + + before do + build.update(finished_at: 2.hours.ago) + end + + it_behaves_like 'schedules to archive traces' + end + + context 'when a job was failed 2 hours ago' do + let!(:build) { create(:ci_build, :failed, :trace_live) } + + before do + build.update(finished_at: 2.hours.ago) + end + + it_behaves_like 'schedules to archive traces' + end + + context 'when a job was cancelled 2 hours ago' do + let!(:build) { create(:ci_build, :canceled, :trace_live) } + + before do + build.update(finished_at: 2.hours.ago) + end + + it_behaves_like 'schedules to archive traces' + end + + context 'when a job has been finished 10 minutes ago' do + let!(:build) { create(:ci_build, :success, :trace_live) } + + before do + build.update(finished_at: 10.minutes.ago) + end + + it_behaves_like 'does not schedule to archive traces' + end + + context 'when a job is running' do + let!(:build) { create(:ci_build, :running, :trace_live) } + + it_behaves_like 'does not schedule to archive traces' + end +end diff --git a/spec/workers/rescue_stale_live_trace_worker_spec.rb b/spec/workers/rescue_stale_live_trace_worker_spec.rb deleted file mode 100644 index f2b6361c54b..00000000000 --- a/spec/workers/rescue_stale_live_trace_worker_spec.rb +++ /dev/null @@ -1,71 +0,0 @@ -require 'spec_helper' - -describe RescueStaleLiveTraceWorker do - subject { described_class.new.perform } - - before do - stub_feature_flags(ci_enable_live_trace: true) - end - - shared_examples_for 'schedules to archive traces' do - it do - expect(ArchiveTraceWorker).to receive(:bulk_perform_async).with([[build.id]]) - - subject - end - end - - shared_examples_for 'does not schedule to archive traces' do - it do - expect(ArchiveTraceWorker).not_to receive(:bulk_perform_async) - - subject - end - end - - context 'when a job was succeeded 2 hours ago' do - let!(:build) { create(:ci_build, :success, :trace_live) } - - before do - build.update(finished_at: 2.hours.ago) - end - - it_behaves_like 'schedules to archive traces' - end - - context 'when a job was failed 2 hours ago' do - let!(:build) { create(:ci_build, :failed, :trace_live) } - - before do - build.update(finished_at: 2.hours.ago) - end - - it_behaves_like 'schedules to archive traces' - end - - context 'when a job was cancelled 2 hours ago' do - let!(:build) { create(:ci_build, :canceled, :trace_live) } - - before do - build.update(finished_at: 2.hours.ago) - end - - it_behaves_like 'schedules to archive traces' - end - - context 'when a job has been finished 10 minutes ago' do - let!(:build) { create(:ci_build, :success, :trace_live) } - - before do - build.update(finished_at: 10.minutes.ago) - end - - it_behaves_like 'does not schedule to archive traces' - end - - context 'when a job is running' do - let!(:build) { create(:ci_build, :running, :trace_live) } - - it_behaves_like 'does not schedule to archive traces' - end -end -- cgit v1.2.3 From 32f825c648cb5fc829cd32b3392530613c62e983 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 1 Jun 2018 14:22:31 +0900 Subject: Add tests for each new code --- spec/lib/gitlab/ci/trace_spec.rb | 2 +- spec/models/ci/build_trace_chunk_spec.rb | 40 ++++++++++++++++++++++ .../shared_examples/ci_trace_shared_examples.rb | 25 ++++++++++++++ .../ci/rescue_stale_live_trace_worker_spec.rb | 34 ++++++++++++------ 4 files changed, 89 insertions(+), 12 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/ci/trace_spec.rb b/spec/lib/gitlab/ci/trace_spec.rb index e9d755c2021..d6510649dba 100644 --- a/spec/lib/gitlab/ci/trace_spec.rb +++ b/spec/lib/gitlab/ci/trace_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Ci::Trace, :clean_gitlab_redis_cache do +describe Gitlab::Ci::Trace, :clean_gitlab_redis_shared_state do let(:build) { create(:ci_build) } let(:trace) { described_class.new(build) } diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index cbcf1e55979..246152d1c8f 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -35,6 +35,46 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do end end + describe '.find_stale_in_batches' do + subject { described_class.find_stale_in_batches } + + context 'when build status is finished' do + context 'when build finished 2 days ago' do + context 'when build has an archived trace' do + let!(:build) { create(:ci_build, :success, :trace_artifact, finished_at: 2.days.ago) } + + it 'does not yield build id' do + expect { |b| described_class.find_stale_in_batches(&b) }.not_to yield_control + end + end + + context 'when build has a live trace' do + let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 2.days.ago) } + + it 'yields build id' do + expect { |b| described_class.find_stale_in_batches(&b) }.to yield_with_args([build.id]) + end + end + end + + context 'when build finished 10 minutes ago' do + let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 10.minutes.ago) } + + it 'does not yield build id' do + expect { |b| described_class.find_stale_in_batches(&b) }.not_to yield_control + end + end + end + + context 'when build status is running' do + let!(:build) { create(:ci_build, :running, :trace_live) } + + it 'does not yield build id' do + expect { |b| described_class.find_stale_in_batches(&b) }.not_to yield_control + end + end + end + describe '#data' do subject { build_trace_chunk.data } diff --git a/spec/support/shared_examples/ci_trace_shared_examples.rb b/spec/support/shared_examples/ci_trace_shared_examples.rb index 21c6f3c829f..a3a0b2d0eb5 100644 --- a/spec/support/shared_examples/ci_trace_shared_examples.rb +++ b/spec/support/shared_examples/ci_trace_shared_examples.rb @@ -227,6 +227,31 @@ shared_examples_for 'common trace features' do end end end + + describe '#archive!' do + subject { trace.archive! } + + context 'when build status is success' do + let!(:build) { create(:ci_build, :success, :trace_live) } + + it 'archives a trace' do + subject + + expect(build.job_artifacts_trace).to be_exist + end + + context 'when anothe process had already been archiving', :clean_gitlab_redis_shared_state do + before do + Gitlab::ExclusiveLease.new("trace:archive:#{trace.job.id}", timeout: 1.hour).try_obtain + end + + it 'prevents multiple archiving' do + build.reload + expect(build.job_artifacts_trace).to be_nil + end + end + end + end end shared_examples_for 'trace with disabled live trace feature' do diff --git a/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb b/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb index 87796fd66db..43a6362a131 100644 --- a/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb +++ b/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb @@ -7,19 +7,20 @@ describe Ci::RescueStaleLiveTraceWorker do stub_feature_flags(ci_enable_live_trace: true) end - shared_examples_for 'schedules to archive traces' do + shared_examples_for 'archives trace' do it do - expect(ArchiveTraceWorker).to receive(:bulk_perform_async).with([[build.id]]) - subject + + expect(build.job_artifacts_trace).to be_exist end end - shared_examples_for 'does not schedule to archive traces' do + shared_examples_for 'does not archive trace' do it do - expect(ArchiveTraceWorker).not_to receive(:bulk_perform_async) - subject + + build.reload + expect(build.job_artifacts_trace).to be_nil end end @@ -30,7 +31,18 @@ describe Ci::RescueStaleLiveTraceWorker do build.update(finished_at: 2.hours.ago) end - it_behaves_like 'schedules to archive traces' + it_behaves_like 'archives trace' + + context 'when build has both archived trace and live trace' do + let!(:build2) { create(:ci_build, :success, :trace_live, finished_at: 2.days.ago) } + + it 'archives only available targets' do + subject + + build.reload + expect(build.job_artifacts_trace).to be_exist + end + end end context 'when a job was failed 2 hours ago' do @@ -40,7 +52,7 @@ describe Ci::RescueStaleLiveTraceWorker do build.update(finished_at: 2.hours.ago) end - it_behaves_like 'schedules to archive traces' + it_behaves_like 'archives trace' end context 'when a job was cancelled 2 hours ago' do @@ -50,7 +62,7 @@ describe Ci::RescueStaleLiveTraceWorker do build.update(finished_at: 2.hours.ago) end - it_behaves_like 'schedules to archive traces' + it_behaves_like 'archives trace' end context 'when a job has been finished 10 minutes ago' do @@ -60,12 +72,12 @@ describe Ci::RescueStaleLiveTraceWorker do build.update(finished_at: 10.minutes.ago) end - it_behaves_like 'does not schedule to archive traces' + it_behaves_like 'does not archive trace' end context 'when a job is running' do let!(:build) { create(:ci_build, :running, :trace_live) } - it_behaves_like 'does not schedule to archive traces' + it_behaves_like 'does not archive trace' end end -- cgit v1.2.3 From 2522691eda46ef3ed572b747074e9b3b2e776198 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 1 Jun 2018 16:15:41 +0900 Subject: Fix ambiguous stuck ci job worker's spec. Rename lease key of archive --- spec/workers/ci/rescue_stale_live_trace_worker_spec.rb | 2 +- spec/workers/stuck_ci_jobs_worker_spec.rb | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb b/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb index 43a6362a131..1a694290562 100644 --- a/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb +++ b/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb @@ -35,7 +35,7 @@ describe Ci::RescueStaleLiveTraceWorker do context 'when build has both archived trace and live trace' do let!(:build2) { create(:ci_build, :success, :trace_live, finished_at: 2.days.ago) } - + it 'archives only available targets' do subject diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb index bdc64c6785b..c3294fce5ea 100644 --- a/spec/workers/stuck_ci_jobs_worker_spec.rb +++ b/spec/workers/stuck_ci_jobs_worker_spec.rb @@ -132,8 +132,10 @@ describe StuckCiJobsWorker do end it 'cancels exclusive lease after worker perform' do - expect(Gitlab::ExclusiveLease).to receive(:cancel).with(described_class::EXCLUSIVE_LEASE_KEY, exclusive_lease_uuid) worker.perform + + expect(Gitlab::ExclusiveLease.new(described_class::EXCLUSIVE_LEASE_KEY, timeout: 1.hour).exists?) + .to be_falsy end end end -- cgit v1.2.3 From 4064481501a24d31872914c845f5d8c2cfc08040 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 1 Jun 2018 16:25:17 +0900 Subject: Rename find_stale_in_batches to find_builds_from_stale_live_trace. Fix comments --- spec/models/ci/build_trace_chunk_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'spec') diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 246152d1c8f..5d524d7cf49 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -35,8 +35,8 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do end end - describe '.find_stale_in_batches' do - subject { described_class.find_stale_in_batches } + describe '.find_builds_from_stale_live_trace' do + subject { described_class.find_builds_from_stale_live_trace } context 'when build status is finished' do context 'when build finished 2 days ago' do @@ -44,7 +44,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let!(:build) { create(:ci_build, :success, :trace_artifact, finished_at: 2.days.ago) } it 'does not yield build id' do - expect { |b| described_class.find_stale_in_batches(&b) }.not_to yield_control + expect { |b| described_class.find_builds_from_stale_live_trace(&b) }.not_to yield_control end end @@ -52,7 +52,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 2.days.ago) } it 'yields build id' do - expect { |b| described_class.find_stale_in_batches(&b) }.to yield_with_args([build.id]) + expect { |b| described_class.find_builds_from_stale_live_trace(&b) }.to yield_with_args([build.id]) end end end @@ -61,7 +61,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 10.minutes.ago) } it 'does not yield build id' do - expect { |b| described_class.find_stale_in_batches(&b) }.not_to yield_control + expect { |b| described_class.find_builds_from_stale_live_trace(&b) }.not_to yield_control end end end @@ -70,7 +70,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let!(:build) { create(:ci_build, :running, :trace_live) } it 'does not yield build id' do - expect { |b| described_class.find_stale_in_batches(&b) }.not_to yield_control + expect { |b| described_class.find_builds_from_stale_live_trace(&b) }.not_to yield_control end end end -- cgit v1.2.3 From 2084e7ab9aad92d4a8196af01b9b8e02ffacb0a4 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 1 Jun 2018 17:09:46 +0900 Subject: Move find_builds_from_stale_live_traces method to Ci::Build --- spec/models/ci/build_trace_chunk_spec.rb | 12 ++++++------ spec/workers/ci/rescue_stale_live_trace_worker_spec.rb | 17 +++++++++-------- 2 files changed, 15 insertions(+), 14 deletions(-) (limited to 'spec') diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 5d524d7cf49..2dd8e935d62 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -35,8 +35,8 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do end end - describe '.find_builds_from_stale_live_trace' do - subject { described_class.find_builds_from_stale_live_trace } + describe '.find_builds_from_stale_live_traces' do + subject { described_class.find_builds_from_stale_live_traces } context 'when build status is finished' do context 'when build finished 2 days ago' do @@ -44,7 +44,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let!(:build) { create(:ci_build, :success, :trace_artifact, finished_at: 2.days.ago) } it 'does not yield build id' do - expect { |b| described_class.find_builds_from_stale_live_trace(&b) }.not_to yield_control + expect { |b| described_class.find_builds_from_stale_live_traces(&b) }.not_to yield_control end end @@ -52,7 +52,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 2.days.ago) } it 'yields build id' do - expect { |b| described_class.find_builds_from_stale_live_trace(&b) }.to yield_with_args([build.id]) + expect { |b| described_class.find_builds_from_stale_live_traces(&b) }.to yield_with_args([build.id]) end end end @@ -61,7 +61,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 10.minutes.ago) } it 'does not yield build id' do - expect { |b| described_class.find_builds_from_stale_live_trace(&b) }.not_to yield_control + expect { |b| described_class.find_builds_from_stale_live_traces(&b) }.not_to yield_control end end end @@ -70,7 +70,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let!(:build) { create(:ci_build, :running, :trace_live) } it 'does not yield build id' do - expect { |b| described_class.find_builds_from_stale_live_trace(&b) }.not_to yield_control + expect { |b| described_class.find_builds_from_stale_live_traces(&b) }.not_to yield_control end end end diff --git a/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb b/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb index 1a694290562..6eee08fdbb5 100644 --- a/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb +++ b/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb @@ -11,6 +11,7 @@ describe Ci::RescueStaleLiveTraceWorker do it do subject + build.reload expect(build.job_artifacts_trace).to be_exist end end @@ -33,16 +34,16 @@ describe Ci::RescueStaleLiveTraceWorker do it_behaves_like 'archives trace' - context 'when build has both archived trace and live trace' do - let!(:build2) { create(:ci_build, :success, :trace_live, finished_at: 2.days.ago) } + # context 'when build has both archived trace and live trace' do + # let!(:build2) { create(:ci_build, :success, :trace_live, finished_at: 2.days.ago) } - it 'archives only available targets' do - subject + # it 'archives only available targets' do + # subject - build.reload - expect(build.job_artifacts_trace).to be_exist - end - end + # build.reload + # expect(build.job_artifacts_trace).to be_exist + # end + # end end context 'when a job was failed 2 hours ago' do -- cgit v1.2.3 From 5a1ee0c391a06a323d960eab6ffb33dfcf2edca7 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 2 Jun 2018 13:08:34 +0900 Subject: Fix the query to select stale live traces --- spec/models/ci/build_spec.rb | 20 +++++++++ spec/models/ci/build_trace_chunk_spec.rb | 40 ------------------ .../ci/rescue_stale_live_trace_worker_spec.rb | 47 +++++----------------- spec/workers/stuck_ci_jobs_worker_spec.rb | 4 +- 4 files changed, 33 insertions(+), 78 deletions(-) (limited to 'spec') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 5e27cca6771..1b26c8d3b49 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -116,6 +116,26 @@ describe Ci::Build do end end + describe '.with_live_trace' do + subject { described_class.with_live_trace } + + context 'when build has live trace' do + let!(:build) { create(:ci_build, :success, :trace_live) } + + it 'selects the build' do + is_expected.to eq([build]) + end + end + + context 'when build does not have live trace' do + let!(:build) { create(:ci_build, :success, :trace_artifact) } + + it 'selects the build' do + is_expected.to be_empty + end + end + end + describe '#actionize' do context 'when build is a created' do before do diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 2dd8e935d62..cbcf1e55979 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -35,46 +35,6 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do end end - describe '.find_builds_from_stale_live_traces' do - subject { described_class.find_builds_from_stale_live_traces } - - context 'when build status is finished' do - context 'when build finished 2 days ago' do - context 'when build has an archived trace' do - let!(:build) { create(:ci_build, :success, :trace_artifact, finished_at: 2.days.ago) } - - it 'does not yield build id' do - expect { |b| described_class.find_builds_from_stale_live_traces(&b) }.not_to yield_control - end - end - - context 'when build has a live trace' do - let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 2.days.ago) } - - it 'yields build id' do - expect { |b| described_class.find_builds_from_stale_live_traces(&b) }.to yield_with_args([build.id]) - end - end - end - - context 'when build finished 10 minutes ago' do - let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 10.minutes.ago) } - - it 'does not yield build id' do - expect { |b| described_class.find_builds_from_stale_live_traces(&b) }.not_to yield_control - end - end - end - - context 'when build status is running' do - let!(:build) { create(:ci_build, :running, :trace_live) } - - it 'does not yield build id' do - expect { |b| described_class.find_builds_from_stale_live_traces(&b) }.not_to yield_control - end - end - end - describe '#data' do subject { build_trace_chunk.data } diff --git a/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb b/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb index 6eee08fdbb5..0e3fbe2d3a9 100644 --- a/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb +++ b/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb @@ -25,57 +25,32 @@ describe Ci::RescueStaleLiveTraceWorker do end end - context 'when a job was succeeded 2 hours ago' do + context 'when a job was succeeded' do let!(:build) { create(:ci_build, :success, :trace_live) } - before do - build.update(finished_at: 2.hours.ago) - end - it_behaves_like 'archives trace' - # context 'when build has both archived trace and live trace' do - # let!(:build2) { create(:ci_build, :success, :trace_live, finished_at: 2.days.ago) } + context 'when archive raised an exception' do + let!(:build) { create(:ci_build, :success, :trace_artifact, :trace_live) } + let!(:build2) { create(:ci_build, :success, :trace_live) } - # it 'archives only available targets' do - # subject - - # build.reload - # expect(build.job_artifacts_trace).to be_exist - # end - # end - end + it 'archives valid targets' do + expect(Rails.logger).to receive(:error).with("Failed to archive stale live trace. id: #{build.id} message: Already archived") - context 'when a job was failed 2 hours ago' do - let!(:build) { create(:ci_build, :failed, :trace_live) } + subject - before do - build.update(finished_at: 2.hours.ago) + build2.reload + expect(build2.job_artifacts_trace).to be_exist + end end - - it_behaves_like 'archives trace' end - context 'when a job was cancelled 2 hours ago' do + context 'when a job was cancelled' do let!(:build) { create(:ci_build, :canceled, :trace_live) } - before do - build.update(finished_at: 2.hours.ago) - end - it_behaves_like 'archives trace' end - context 'when a job has been finished 10 minutes ago' do - let!(:build) { create(:ci_build, :success, :trace_live) } - - before do - build.update(finished_at: 10.minutes.ago) - end - - it_behaves_like 'does not archive trace' - end - context 'when a job is running' do let!(:build) { create(:ci_build, :running, :trace_live) } diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb index c3294fce5ea..2605c14334f 100644 --- a/spec/workers/stuck_ci_jobs_worker_spec.rb +++ b/spec/workers/stuck_ci_jobs_worker_spec.rb @@ -134,8 +134,8 @@ describe StuckCiJobsWorker do it 'cancels exclusive lease after worker perform' do worker.perform - expect(Gitlab::ExclusiveLease.new(described_class::EXCLUSIVE_LEASE_KEY, timeout: 1.hour).exists?) - .to be_falsy + expect(Gitlab::ExclusiveLease.new(described_class::EXCLUSIVE_LEASE_KEY, timeout: 1.hour)) + .not_to be_exists end end end -- cgit v1.2.3 From f9b821f08dfc8feee2ed8a06c8f21c85c3c9d2ec Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 2 Jun 2018 14:09:24 +0900 Subject: Fix specs for exclusive lease --- .../shared_examples/ci_trace_shared_examples.rb | 29 ++++++++++++++-------- 1 file changed, 19 insertions(+), 10 deletions(-) (limited to 'spec') diff --git a/spec/support/shared_examples/ci_trace_shared_examples.rb b/spec/support/shared_examples/ci_trace_shared_examples.rb index a3a0b2d0eb5..59efce1b5f0 100644 --- a/spec/support/shared_examples/ci_trace_shared_examples.rb +++ b/spec/support/shared_examples/ci_trace_shared_examples.rb @@ -234,20 +234,29 @@ shared_examples_for 'common trace features' do context 'when build status is success' do let!(:build) { create(:ci_build, :success, :trace_live) } - it 'archives a trace' do - subject - - expect(build.job_artifacts_trace).to be_exist + it 'does not have an archived trace yet' do + expect(build.job_artifacts_trace).to be_nil end - context 'when anothe process had already been archiving', :clean_gitlab_redis_shared_state do - before do - Gitlab::ExclusiveLease.new("trace:archive:#{trace.job.id}", timeout: 1.hour).try_obtain - end + context 'when archives' do + it 'has an archived trace' do + subject - it 'prevents multiple archiving' do build.reload - expect(build.job_artifacts_trace).to be_nil + expect(build.job_artifacts_trace).to be_exist + end + + context 'when anothe process had already been archiving', :clean_gitlab_redis_shared_state do + before do + Gitlab::ExclusiveLease.new("trace:archive:#{trace.job.id}", timeout: 1.hour).try_obtain + end + + it 'prevents to archive concurently' do + subject + + build.reload + expect(build.job_artifacts_trace).to be_nil + end end end end -- cgit v1.2.3 From cae17352761d9c34de444cb95e77d8fa1a8bd56a Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 4 Jun 2018 14:12:45 +0900 Subject: Fix typos and add a small spec --- spec/models/ci/build_spec.rb | 2 +- spec/support/shared_examples/ci_trace_shared_examples.rb | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) (limited to 'spec') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 1b26c8d3b49..0a0d7d3fea9 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -130,7 +130,7 @@ describe Ci::Build do context 'when build does not have live trace' do let!(:build) { create(:ci_build, :success, :trace_artifact) } - it 'selects the build' do + it 'does not select the build' do is_expected.to be_empty end end diff --git a/spec/support/shared_examples/ci_trace_shared_examples.rb b/spec/support/shared_examples/ci_trace_shared_examples.rb index 59efce1b5f0..6dbe0f6f980 100644 --- a/spec/support/shared_examples/ci_trace_shared_examples.rb +++ b/spec/support/shared_examples/ci_trace_shared_examples.rb @@ -246,12 +246,14 @@ shared_examples_for 'common trace features' do expect(build.job_artifacts_trace).to be_exist end - context 'when anothe process had already been archiving', :clean_gitlab_redis_shared_state do + context 'when another process has already been archiving', :clean_gitlab_redis_shared_state do before do Gitlab::ExclusiveLease.new("trace:archive:#{trace.job.id}", timeout: 1.hour).try_obtain end - it 'prevents to archive concurently' do + it 'blocks concurrent archiving' do + expect(Rails.logger).to receive(:error).with('Cannot obtain an exclusive lease. There must be another instance already in execution.') + subject build.reload -- cgit v1.2.3 From 9b65d4bb417fb4939289eab94487c894f0a62db6 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Wed, 23 May 2018 09:55:14 +0200 Subject: Initial setup GraphQL using graphql-ruby 1.8 - All definitions have been replaced by classes: http://graphql-ruby.org/schema/class_based_api.html - Authorization & Presentation have been refactored to work in the class based system - Loaders have been replaced by resolvers - Times are now coersed as ISO 8601 --- spec/graphql/gitlab_schema_spec.rb | 15 +++--- spec/graphql/loaders/full_path_loader_spec.rb | 32 ------------ spec/graphql/loaders/iid_loader_spec.rb | 58 ---------------------- .../resolvers/merge_request_resolver_spec.rb | 58 ++++++++++++++++++++++ spec/graphql/resolvers/project_resolver_spec.rb | 32 ++++++++++++ spec/graphql/types/project_type_spec.rb | 5 ++ spec/graphql/types/query_type_spec.rb | 10 ++-- spec/graphql/types/time_type_spec.rb | 14 +++--- .../api/graphql/merge_request_query_spec.rb | 49 ++++++++++++++++++ spec/requests/api/graphql/project_query_spec.rb | 39 +++++++++++++++ spec/requests/graphql/merge_request_query_spec.rb | 24 --------- spec/requests/graphql/project_query_spec.rb | 23 --------- spec/routing/api_routing_spec.rb | 31 ++++++++++++ spec/support/helpers/graphql_helpers.rb | 47 +++++++++++++++--- spec/support/matchers/graphql_matchers.rb | 23 ++++++--- .../requests/graphql_shared_examples.rb | 11 +--- 16 files changed, 292 insertions(+), 179 deletions(-) delete mode 100644 spec/graphql/loaders/full_path_loader_spec.rb delete mode 100644 spec/graphql/loaders/iid_loader_spec.rb create mode 100644 spec/graphql/resolvers/merge_request_resolver_spec.rb create mode 100644 spec/graphql/resolvers/project_resolver_spec.rb create mode 100644 spec/graphql/types/project_type_spec.rb create mode 100644 spec/requests/api/graphql/merge_request_query_spec.rb create mode 100644 spec/requests/api/graphql/project_query_spec.rb delete mode 100644 spec/requests/graphql/merge_request_query_spec.rb delete mode 100644 spec/requests/graphql/project_query_spec.rb create mode 100644 spec/routing/api_routing_spec.rb (limited to 'spec') diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb index f25cc2fd6c9..b892f6b44ed 100644 --- a/spec/graphql/gitlab_schema_spec.rb +++ b/spec/graphql/gitlab_schema_spec.rb @@ -6,26 +6,25 @@ describe GitlabSchema do end it 'enables the preload instrumenter' do - expect(field_instrumenters).to include(instance_of(::GraphQL::Preload::Instrument)) + expect(field_instrumenters).to include(BatchLoader::GraphQL) end it 'enables the authorization instrumenter' do - expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Authorize)) + expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Authorize::Instrumentation)) end it 'enables using presenters' do - expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Present)) + expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Present::Instrumentation)) end it 'has the base mutation' do - pending <<~REASON - Having empty mutations breaks the automatic documentation in Graphiql, so removed for now." - REASON - expect(described_class.mutation).to eq(::Types::MutationType) + pending('Adding an empty mutation breaks the documentation explorer') + + expect(described_class.mutation).to eq(::Types::MutationType.to_graphql) end it 'has the base query' do - expect(described_class.query).to eq(::Types::QueryType) + expect(described_class.query).to eq(::Types::QueryType.to_graphql) end def field_instrumenters diff --git a/spec/graphql/loaders/full_path_loader_spec.rb b/spec/graphql/loaders/full_path_loader_spec.rb deleted file mode 100644 index 2732dd8c9da..00000000000 --- a/spec/graphql/loaders/full_path_loader_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -require 'spec_helper' - -describe Loaders::FullPathLoader do - include GraphqlHelpers - - set(:project1) { create(:project) } - set(:project2) { create(:project) } - - set(:other_project) { create(:project) } - - describe '.project' do - it 'batch-resolves projects by full path' do - paths = [project1.full_path, project2.full_path] - - result = batch(max_queries: 1) do - paths.map { |path| resolve_project(path) } - end - - expect(result).to contain_exactly(project1, project2) - end - - it 'resolves an unknown full_path to nil' do - result = batch { resolve_project('unknown/project') } - - expect(result).to be_nil - end - end - - def resolve_project(full_path) - resolve(described_class, :project, args: { full_path: full_path }) - end -end diff --git a/spec/graphql/loaders/iid_loader_spec.rb b/spec/graphql/loaders/iid_loader_spec.rb deleted file mode 100644 index 0a57d7c4ed4..00000000000 --- a/spec/graphql/loaders/iid_loader_spec.rb +++ /dev/null @@ -1,58 +0,0 @@ -require 'spec_helper' - -describe Loaders::IidLoader do - include GraphqlHelpers - - set(:project) { create(:project, :repository) } - set(:merge_request_1) { create(:merge_request, :simple, source_project: project, target_project: project) } - set(:merge_request_2) { create(:merge_request, :rebased, source_project: project, target_project: project) } - - set(:other_project) { create(:project, :repository) } - set(:other_merge_request) { create(:merge_request, source_project: other_project, target_project: other_project) } - - let(:full_path) { project.full_path } - let(:iid_1) { merge_request_1.iid } - let(:iid_2) { merge_request_2.iid } - - let(:other_full_path) { other_project.full_path } - let(:other_iid) { other_merge_request.iid } - - describe '.merge_request' do - it 'batch-resolves merge requests by target project full path and IID' do - path = full_path # avoid database query - - result = batch(max_queries: 2) do - [resolve_mr(path, iid_1), resolve_mr(path, iid_2)] - end - - expect(result).to contain_exactly(merge_request_1, merge_request_2) - end - - it 'can batch-resolve merge requests from different projects' do - path = project.full_path # avoid database queries - other_path = other_full_path - - result = batch(max_queries: 3) do - [resolve_mr(path, iid_1), resolve_mr(path, iid_2), resolve_mr(other_path, other_iid)] - end - - expect(result).to contain_exactly(merge_request_1, merge_request_2, other_merge_request) - end - - it 'resolves an unknown iid to nil' do - result = batch { resolve_mr(full_path, -1) } - - expect(result).to be_nil - end - - it 'resolves a known iid for an unknown full_path to nil' do - result = batch { resolve_mr('unknown/project', iid_1) } - - expect(result).to be_nil - end - end - - def resolve_mr(full_path, iid) - resolve(described_class, :merge_request, args: { project: full_path, iid: iid }) - end -end diff --git a/spec/graphql/resolvers/merge_request_resolver_spec.rb b/spec/graphql/resolvers/merge_request_resolver_spec.rb new file mode 100644 index 00000000000..af015533209 --- /dev/null +++ b/spec/graphql/resolvers/merge_request_resolver_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe Resolvers::MergeRequestResolver do + include GraphqlHelpers + + set(:project) { create(:project, :repository) } + set(:merge_request_1) { create(:merge_request, :simple, source_project: project, target_project: project) } + set(:merge_request_2) { create(:merge_request, :rebased, source_project: project, target_project: project) } + + set(:other_project) { create(:project, :repository) } + set(:other_merge_request) { create(:merge_request, source_project: other_project, target_project: other_project) } + + let(:full_path) { project.full_path } + let(:iid_1) { merge_request_1.iid } + let(:iid_2) { merge_request_2.iid } + + let(:other_full_path) { other_project.full_path } + let(:other_iid) { other_merge_request.iid } + + describe '#resolve' do + it 'batch-resolves merge requests by target project full path and IID' do + path = full_path # avoid database query + + result = batch(max_queries: 2) do + [resolve_mr(path, iid_1), resolve_mr(path, iid_2)] + end + + expect(result).to contain_exactly(merge_request_1, merge_request_2) + end + + it 'can batch-resolve merge requests from different projects' do + path = project.full_path # avoid database queries + other_path = other_full_path + + result = batch(max_queries: 3) do + [resolve_mr(path, iid_1), resolve_mr(path, iid_2), resolve_mr(other_path, other_iid)] + end + + expect(result).to contain_exactly(merge_request_1, merge_request_2, other_merge_request) + end + + it 'resolves an unknown iid to nil' do + result = batch { resolve_mr(full_path, -1) } + + expect(result).to be_nil + end + + it 'resolves a known iid for an unknown full_path to nil' do + result = batch { resolve_mr('unknown/project', iid_1) } + + expect(result).to be_nil + end + end + + def resolve_mr(full_path, iid) + resolve(described_class, args: { full_path: full_path, iid: iid }) + end +end diff --git a/spec/graphql/resolvers/project_resolver_spec.rb b/spec/graphql/resolvers/project_resolver_spec.rb new file mode 100644 index 00000000000..d4990c6492c --- /dev/null +++ b/spec/graphql/resolvers/project_resolver_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Resolvers::ProjectResolver do + include GraphqlHelpers + + set(:project1) { create(:project) } + set(:project2) { create(:project) } + + set(:other_project) { create(:project) } + + describe '#resolve' do + it 'batch-resolves projects by full path' do + paths = [project1.full_path, project2.full_path] + + result = batch(max_queries: 1) do + paths.map { |path| resolve_project(path) } + end + + expect(result).to contain_exactly(project1, project2) + end + + it 'resolves an unknown full_path to nil' do + result = batch { resolve_project('unknown/project') } + + expect(result).to be_nil + end + end + + def resolve_project(full_path) + resolve(described_class, args: { full_path: full_path }) + end +end diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb new file mode 100644 index 00000000000..e0f89105b86 --- /dev/null +++ b/spec/graphql/types/project_type_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe GitlabSchema.types['Project'] do + it { expect(described_class.graphql_name).to eq('Project') } +end diff --git a/spec/graphql/types/query_type_spec.rb b/spec/graphql/types/query_type_spec.rb index 17d9395504c..8488252fd59 100644 --- a/spec/graphql/types/query_type_spec.rb +++ b/spec/graphql/types/query_type_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe GitlabSchema.types['Query'] do it 'is called Query' do - expect(described_class.name).to eq('Query') + expect(described_class.graphql_name).to eq('Query') end it { is_expected.to have_graphql_fields(:project, :merge_request, :echo) } @@ -13,7 +13,7 @@ describe GitlabSchema.types['Query'] do it 'finds projects by full path' do is_expected.to have_graphql_arguments(:full_path) is_expected.to have_graphql_type(Types::ProjectType) - is_expected.to have_graphql_resolver(Loaders::FullPathLoader[:project]) + is_expected.to have_graphql_resolver(Resolvers::ProjectResolver) end it 'authorizes with read_project' do @@ -22,12 +22,12 @@ describe GitlabSchema.types['Query'] do end describe 'merge_request field' do - subject { described_class.fields['merge_request'] } + subject { described_class.fields['mergeRequest'] } it 'finds MRs by project and IID' do - is_expected.to have_graphql_arguments(:project, :iid) + is_expected.to have_graphql_arguments(:full_path, :iid) is_expected.to have_graphql_type(Types::MergeRequestType) - is_expected.to have_graphql_resolver(Loaders::IidLoader[:merge_request]) + is_expected.to have_graphql_resolver(Resolvers::MergeRequestResolver) end it 'authorizes with read_merge_request' do diff --git a/spec/graphql/types/time_type_spec.rb b/spec/graphql/types/time_type_spec.rb index 087655cc67d..4196d9d27d4 100644 --- a/spec/graphql/types/time_type_spec.rb +++ b/spec/graphql/types/time_type_spec.rb @@ -1,16 +1,16 @@ require 'spec_helper' describe GitlabSchema.types['Time'] do - let(:float) { 1504630455.96215 } - let(:time) { Time.at(float) } + let(:iso) { "2018-06-04T15:23:50+02:00" } + let(:time) { Time.parse(iso) } - it { expect(described_class.name).to eq('Time') } + it { expect(described_class.graphql_name).to eq('Time') } - it 'coerces Time into fractional seconds since epoch' do - expect(described_class.coerce_isolated_result(time)).to eq(float) + it 'coerces Time object into ISO 8601' do + expect(described_class.coerce_isolated_result(time)).to eq(iso) end - it 'coerces fractional seconds since epoch into Time' do - expect(described_class.coerce_isolated_input(float)).to eq(time) + it 'coerces an ISO-time into Time object' do + expect(described_class.coerce_isolated_input(iso)).to eq(time) end end diff --git a/spec/requests/api/graphql/merge_request_query_spec.rb b/spec/requests/api/graphql/merge_request_query_spec.rb new file mode 100644 index 00000000000..12b1d5d18a2 --- /dev/null +++ b/spec/requests/api/graphql/merge_request_query_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe 'getting merge request information' do + include GraphqlHelpers + + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request, source_project: project) } + let(:current_user) { create(:user) } + + let(:query) do + attributes = { + 'fullPath' => merge_request.project.full_path, + 'iid' => merge_request.iid + } + graphql_query_for('mergeRequest', attributes) + end + + context 'when the user has access to the merge request' do + before do + project.add_developer(current_user) + post_graphql(query, current_user: current_user) + end + + it 'returns the merge request' do + expect(graphql_data['mergeRequest']).not_to be_nil + end + + # This is a field coming from the `MergeRequestPresenter` + it 'includes a web_url' do + expect(graphql_data['mergeRequest']['webUrl']).to be_present + end + + it_behaves_like 'a working graphql query' + end + + context 'when the user does not have access to the merge request' do + before do + post_graphql(query, current_user: current_user) + end + + it 'returns an empty field' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['mergeRequest']).to be_nil + end + + it_behaves_like 'a working graphql query' + end +end diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb new file mode 100644 index 00000000000..8196bcfa87c --- /dev/null +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe 'getting project information' do + include GraphqlHelpers + + let(:project) { create(:project, :repository) } + let(:current_user) { create(:user) } + + let(:query) do + graphql_query_for('project', 'fullPath' => project.full_path) + end + + context 'when the user has access to the project' do + before do + project.add_developer(current_user) + post_graphql(query, current_user: current_user) + end + + it 'includes the project' do + expect(graphql_data['project']).not_to be_nil + end + + it_behaves_like 'a working graphql query' + end + + context 'when the user does not have access to the project' do + before do + post_graphql(query, current_user: current_user) + end + + it 'returns an empty field' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['project']).to be_nil + end + + it_behaves_like 'a working graphql query' + end +end diff --git a/spec/requests/graphql/merge_request_query_spec.rb b/spec/requests/graphql/merge_request_query_spec.rb deleted file mode 100644 index cbc19782e2f..00000000000 --- a/spec/requests/graphql/merge_request_query_spec.rb +++ /dev/null @@ -1,24 +0,0 @@ -require 'spec_helper' - -describe 'getting merge request information' do - include GraphqlHelpers - - let(:project) { create(:project, :repository, :public) } - let(:merge_request) { create(:merge_request, source_project: project) } - - let(:query) do - <<~QUERY - { - merge_request(project: "#{merge_request.project.full_path}", iid: "#{merge_request.iid}") { - #{all_graphql_fields_for(MergeRequest)} - } - } - QUERY - end - - it_behaves_like 'a working graphql query' do - it 'renders a merge request with all fields' do - expect(response_data['merge_request']).not_to be_nil - end - end -end diff --git a/spec/requests/graphql/project_query_spec.rb b/spec/requests/graphql/project_query_spec.rb deleted file mode 100644 index 110ed433e03..00000000000 --- a/spec/requests/graphql/project_query_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'spec_helper' - -describe 'getting project information' do - include GraphqlHelpers - - let(:project) { create(:project, :repository, :public) } - - let(:query) do - <<~QUERY - { - project(full_path: "#{project.full_path}") { - #{all_graphql_fields_for(Project)} - } - } - QUERY - end - - it_behaves_like 'a working graphql query' do - it 'renders a project with all fields' do - expect(response_data['project']).not_to be_nil - end - end -end diff --git a/spec/routing/api_routing_spec.rb b/spec/routing/api_routing_spec.rb new file mode 100644 index 00000000000..5fde4bd885b --- /dev/null +++ b/spec/routing/api_routing_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe 'api', 'routing' do + context 'when graphql is disabled' do + before do + stub_feature_flags(graphql: false) + end + + it 'does not route to the GraphqlController' do + expect(get('/api/graphql')).not_to route_to('graphql#execute') + end + + it 'does not expose graphiql' do + expect(get('/-/graphql-explorer')).not_to route_to('graphiql/rails/editors#show') + end + end + + context 'when graphql is disabled' do + before do + stub_feature_flags(graphql: true) + end + + it 'routes to the GraphqlController' do + expect(get('/api/graphql')).not_to route_to('graphql#execute') + end + + it 'exposes graphiql' do + expect(get('/-/graphql-explorer')).not_to route_to('graphiql/rails/editors#show') + end + end +end diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index ae29b16e32c..30ff9a1196a 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -1,7 +1,16 @@ module GraphqlHelpers + # makes an underscored string look like a fieldname + # "merge_request" => "mergeRequest" + def self.fieldnamerize(underscored_field_name) + graphql_field_name = underscored_field_name.to_s.camelize + graphql_field_name[0] = graphql_field_name[0].downcase + + graphql_field_name + end + # Run a loader's named resolver - def resolve(kls, name, obj: nil, args: {}, ctx: {}) - kls[name].call(obj, args, ctx) + def resolve(resolver_class, obj: nil, args: {}, ctx: {}) + resolver_class.new(object: obj, context: ctx).resolve(args) end # Runs a block inside a BatchLoader::Executor wrapper @@ -24,8 +33,20 @@ module GraphqlHelpers end end - def all_graphql_fields_for(klass) - type = GitlabSchema.types[klass.name] + def graphql_query_for(name, attributes = {}, fields = nil) + fields ||= all_graphql_fields_for(name.classify) + attributes = attributes_to_graphql(attributes) + <<~QUERY + { + #{name}(#{attributes}) { + #{fields} + } + } + QUERY + end + + def all_graphql_fields_for(class_name) + type = GitlabSchema.types[class_name.to_s] return "" unless type type.fields.map do |name, field| @@ -37,8 +58,22 @@ module GraphqlHelpers end.join("\n") end - def post_graphql(query) - post '/api/graphql', query: query + def attributes_to_graphql(attributes) + attributes.map do |name, value| + "#{GraphqlHelpers.fieldnamerize(name.to_s)}: \"#{value}\"" + end.join(", ") + end + + def post_graphql(query, current_user: nil) + post api('/', current_user, version: 'graphql'), query: query + end + + def graphql_data + json_response['data'] + end + + def graphql_errors + json_response['data'] end def scalar?(field) diff --git a/spec/support/matchers/graphql_matchers.rb b/spec/support/matchers/graphql_matchers.rb index c0ed16ecaba..ba7a1c8cde0 100644 --- a/spec/support/matchers/graphql_matchers.rb +++ b/spec/support/matchers/graphql_matchers.rb @@ -1,31 +1,40 @@ RSpec::Matchers.define :require_graphql_authorizations do |*expected| match do |field| - authorizations = field.metadata[:authorize] - - expect(authorizations).to contain_exactly(*expected) + field_definition = field.metadata[:type_class] + expect(field_definition).to respond_to(:required_permissions) + expect(field_definition.required_permissions).to contain_exactly(*expected) end end RSpec::Matchers.define :have_graphql_fields do |*expected| match do |kls| - expect(kls.fields.keys).to contain_exactly(*expected.map(&:to_s)) + field_names = expected.map { |name| GraphqlHelpers.fieldnamerize(name) } + expect(kls.fields.keys).to contain_exactly(*field_names) end end RSpec::Matchers.define :have_graphql_arguments do |*expected| + include GraphqlHelpers + match do |field| - expect(field.arguments.keys).to contain_exactly(*expected.map(&:to_s)) + argument_names = expected.map { |name| GraphqlHelpers.fieldnamerize(name) } + expect(field.arguments.keys).to contain_exactly(*argument_names) end end RSpec::Matchers.define :have_graphql_type do |expected| match do |field| - expect(field.type).to eq(expected) + expect(field.type).to eq(expected.to_graphql) end end RSpec::Matchers.define :have_graphql_resolver do |expected| match do |field| - expect(field.resolve_proc).to eq(expected) + case expected + when Method + expect(field.metadata[:type_class].resolve_proc).to eq(expected) + else + expect(field.metadata[:type_class].resolver).to eq(expected) + end end end diff --git a/spec/support/shared_examples/requests/graphql_shared_examples.rb b/spec/support/shared_examples/requests/graphql_shared_examples.rb index c143b83696e..9b2b74593a5 100644 --- a/spec/support/shared_examples/requests/graphql_shared_examples.rb +++ b/spec/support/shared_examples/requests/graphql_shared_examples.rb @@ -3,16 +3,9 @@ require 'spec_helper' shared_examples 'a working graphql query' do include GraphqlHelpers - let(:parsed_response) { JSON.parse(response.body) } - let(:response_data) { parsed_response['data'] } - - before do - post_graphql(query) - end - it 'is returns a successfull response', :aggregate_failures do expect(response).to be_success - expect(parsed_response['errors']).to be_nil - expect(response_data).not_to be_empty + expect(graphql_errors['errors']).to be_nil + expect(json_response.keys).to include('data') end end -- cgit v1.2.3 From 332275b766283ff65d0637ada3e4080c3cd4f038 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 6 Jun 2018 01:56:50 -0700 Subject: Simplify error message handling in Projects::CreateService There's no need to add a redundant message to the errors if the model is invalid. This cleans up the message as well for the importer. --- .../githubish_import_controller_shared_examples.rb | 15 --------------- 1 file changed, 15 deletions(-) (limited to 'spec') diff --git a/spec/support/controllers/githubish_import_controller_shared_examples.rb b/spec/support/controllers/githubish_import_controller_shared_examples.rb index 1a53c5fa487..1c1b68c12a2 100644 --- a/spec/support/controllers/githubish_import_controller_shared_examples.rb +++ b/spec/support/controllers/githubish_import_controller_shared_examples.rb @@ -119,21 +119,6 @@ shared_examples 'a GitHub-ish import controller: POST create' do end it 'returns 422 response with the base error when the project could not be imported' do - project = build(:project) - error_message = 'This is an error' - project.errors.add(:base, error_message) - - allow(Gitlab::LegacyGithubImport::ProjectCreator) - .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) - .and_return(double(execute: project)) - - post :create, format: :json - - expect(response).to have_gitlab_http_status(422) - expect(json_response['errors']).to eq(error_message) - end - - it 'returns 422 response with a combined error when the project could not be imported' do project = build(:project) project.errors.add(:name, 'is invalid') project.errors.add(:path, 'is old') -- cgit v1.2.3 From c5e44dc5d1a98a92362b93c30342ebf1f972061c Mon Sep 17 00:00:00 2001 From: Imre Farkas Date: Tue, 5 Jun 2018 22:20:37 +0200 Subject: Use Github repo visibility during import while respecting restricted visibility levels --- .../legacy_github_import/project_creator_spec.rb | 40 ++++++++++++++++++---- 1 file changed, 34 insertions(+), 6 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/legacy_github_import/project_creator_spec.rb b/spec/lib/gitlab/legacy_github_import/project_creator_spec.rb index eb1b13704ea..972b17d5b12 100644 --- a/spec/lib/gitlab/legacy_github_import/project_creator_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/project_creator_spec.rb @@ -44,16 +44,44 @@ describe Gitlab::LegacyGithubImport::ProjectCreator do end context 'when GitHub project is public' do - before do - allow_any_instance_of(ApplicationSetting).to receive(:default_project_visibility).and_return(Gitlab::VisibilityLevel::INTERNAL) - end - - it 'sets project visibility to the default project visibility' do + it 'sets project visibility to public' do repo.private = false project = service.execute - expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) + expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC) + end + end + + context 'when visibility level is restricted' do + context 'when GitHub project is private' do + before do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PRIVATE]) + allow_any_instance_of(ApplicationSetting).to receive(:default_project_visibility).and_return(Gitlab::VisibilityLevel::INTERNAL) + end + + it 'sets project visibility to the default project visibility' do + repo.private = true + + project = service.execute + + expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) + end + end + + context 'when GitHub project is public' do + before do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) + allow_any_instance_of(ApplicationSetting).to receive(:default_project_visibility).and_return(Gitlab::VisibilityLevel::INTERNAL) + end + + it 'sets project visibility to the default project visibility' do + repo.private = false + + project = service.execute + + expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) + end end end -- cgit v1.2.3 From f8d8b63f32176975c802313d966fb81c2bb2122a Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 5 Jun 2018 08:57:13 +0100 Subject: fixed karma --- .../stores/modules/merge_requests/actions_spec.js | 84 +++++++++++++++++----- 1 file changed, 66 insertions(+), 18 deletions(-) (limited to 'spec') diff --git a/spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js b/spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js index b571cfb963a..d178a44b76a 100644 --- a/spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js @@ -8,7 +8,9 @@ import actions, { receiveMergeRequestsSuccess, fetchMergeRequests, resetMergeRequests, + openMergeRequest, } from '~/ide/stores/modules/merge_requests/actions'; +import router from '~/ide/ide_router'; import { mergeRequests } from '../../../mock_data'; import testAction from '../../../../helpers/vuex_action_helper'; @@ -29,9 +31,9 @@ describe('IDE merge requests actions', () => { it('should should commit request', done => { testAction( requestMergeRequests, - null, + 'created', mockedState, - [{ type: types.REQUEST_MERGE_REQUESTS }], + [{ type: types.REQUEST_MERGE_REQUESTS, payload: 'created' }], [], done, ); @@ -48,16 +50,16 @@ describe('IDE merge requests actions', () => { it('should should commit error', done => { testAction( receiveMergeRequestsError, - null, + 'created', mockedState, - [{ type: types.RECEIVE_MERGE_REQUESTS_ERROR }], + [{ type: types.RECEIVE_MERGE_REQUESTS_ERROR, payload: 'created' }], [], done, ); }); it('creates flash message', () => { - receiveMergeRequestsError({ commit() {} }); + receiveMergeRequestsError({ commit() {} }, 'created'); expect(flashSpy).toHaveBeenCalled(); }); @@ -67,9 +69,14 @@ describe('IDE merge requests actions', () => { it('should commit received data', done => { testAction( receiveMergeRequestsSuccess, - 'data', + { type: 'created', data: 'data' }, mockedState, - [{ type: types.RECEIVE_MERGE_REQUESTS_SUCCESS, payload: 'data' }], + [ + { + type: types.RECEIVE_MERGE_REQUESTS_SUCCESS, + payload: { type: 'created', data: 'data' }, + }, + ], [], done, ); @@ -86,14 +93,14 @@ describe('IDE merge requests actions', () => { mock.onGet(/\/api\/v4\/merge_requests(.*)$/).replyOnce(200, mergeRequests); }); - it('calls API with params from state', () => { + it('calls API with params', () => { const apiSpy = spyOn(axios, 'get').and.callThrough(); - fetchMergeRequests({ dispatch() {}, state: mockedState }); + fetchMergeRequests({ dispatch() {}, state: mockedState }, { type: 'created' }); expect(apiSpy).toHaveBeenCalledWith(jasmine.anything(), { params: { - scope: 'assigned-to-me', + scope: 'created-by-me', state: 'opened', search: '', }, @@ -103,11 +110,14 @@ describe('IDE merge requests actions', () => { it('calls API with search', () => { const apiSpy = spyOn(axios, 'get').and.callThrough(); - fetchMergeRequests({ dispatch() {}, state: mockedState }, 'testing search'); + fetchMergeRequests( + { dispatch() {}, state: mockedState }, + { type: 'created', search: 'testing search' }, + ); expect(apiSpy).toHaveBeenCalledWith(jasmine.anything(), { params: { - scope: 'assigned-to-me', + scope: 'created-by-me', state: 'opened', search: 'testing search', }, @@ -117,7 +127,7 @@ describe('IDE merge requests actions', () => { it('dispatches request', done => { testAction( fetchMergeRequests, - null, + { type: 'created' }, mockedState, [], [ @@ -132,13 +142,16 @@ describe('IDE merge requests actions', () => { it('dispatches success with received data', done => { testAction( fetchMergeRequests, - null, + { type: 'created' }, mockedState, [], [ { type: 'requestMergeRequests' }, { type: 'resetMergeRequests' }, - { type: 'receiveMergeRequestsSuccess', payload: mergeRequests }, + { + type: 'receiveMergeRequestsSuccess', + payload: { type: 'created', data: mergeRequests }, + }, ], done, ); @@ -153,7 +166,7 @@ describe('IDE merge requests actions', () => { it('dispatches error', done => { testAction( fetchMergeRequests, - null, + { type: 'created' }, mockedState, [], [ @@ -171,12 +184,47 @@ describe('IDE merge requests actions', () => { it('commits reset', done => { testAction( resetMergeRequests, - null, + 'created', mockedState, - [{ type: types.RESET_MERGE_REQUESTS }], + [{ type: types.RESET_MERGE_REQUESTS, payload: 'created' }], [], done, ); }); }); + + describe('openMergeRequest', () => { + beforeEach(() => { + spyOn(router, 'push'); + }); + + it('commits reset mutations and actions', done => { + testAction( + openMergeRequest, + { projectPath: 'gitlab-org/gitlab-ce', id: '1' }, + mockedState, + [ + { type: 'CLEAR_PROJECTS' }, + { type: 'SET_CURRENT_MERGE_REQUEST', payload: '1' }, + { type: 'RESET_OPEN_FILES' }, + ], + [ + { type: 'pipelines/stopPipelinePolling' }, + { type: 'pipelines/clearEtagPoll' }, + { type: 'pipelines/resetLatestPipeline' }, + { type: 'setCurrentBranchId', payload: '' }, + ], + done, + ); + }); + + it('pushes new route', () => { + openMergeRequest( + { commit() {}, dispatch() {} }, + { projectPath: 'gitlab-org/gitlab-ce', id: '1' }, + ); + + expect(router.push).toHaveBeenCalledWith('/project/gitlab-org/gitlab-ce/merge_requests/1'); + }); + }); }); -- cgit v1.2.3 From 95b3ff056fb9edabef6c512c5de353c5c4a8bcb3 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 5 Jun 2018 09:50:48 +0100 Subject: fixed mutations spec --- .../stores/modules/merge_requests/mutations_spec.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'spec') diff --git a/spec/javascripts/ide/stores/modules/merge_requests/mutations_spec.js b/spec/javascripts/ide/stores/modules/merge_requests/mutations_spec.js index 664d3914564..ea03131d90d 100644 --- a/spec/javascripts/ide/stores/modules/merge_requests/mutations_spec.js +++ b/spec/javascripts/ide/stores/modules/merge_requests/mutations_spec.js @@ -12,26 +12,29 @@ describe('IDE merge requests mutations', () => { describe(types.REQUEST_MERGE_REQUESTS, () => { it('sets loading to true', () => { - mutations[types.REQUEST_MERGE_REQUESTS](mockedState); + mutations[types.REQUEST_MERGE_REQUESTS](mockedState, 'created'); - expect(mockedState.isLoading).toBe(true); + expect(mockedState.created.isLoading).toBe(true); }); }); describe(types.RECEIVE_MERGE_REQUESTS_ERROR, () => { it('sets loading to false', () => { - mutations[types.RECEIVE_MERGE_REQUESTS_ERROR](mockedState); + mutations[types.RECEIVE_MERGE_REQUESTS_ERROR](mockedState, 'created'); - expect(mockedState.isLoading).toBe(false); + expect(mockedState.created.isLoading).toBe(false); }); }); describe(types.RECEIVE_MERGE_REQUESTS_SUCCESS, () => { it('sets merge requests', () => { gon.gitlab_url = gl.TEST_HOST; - mutations[types.RECEIVE_MERGE_REQUESTS_SUCCESS](mockedState, mergeRequests); + mutations[types.RECEIVE_MERGE_REQUESTS_SUCCESS](mockedState, { + type: 'created', + data: mergeRequests, + }); - expect(mockedState.mergeRequests).toEqual([ + expect(mockedState.created.mergeRequests).toEqual([ { id: 1, iid: 1, @@ -47,9 +50,9 @@ describe('IDE merge requests mutations', () => { it('clears merge request array', () => { mockedState.mergeRequests = ['test']; - mutations[types.RESET_MERGE_REQUESTS](mockedState); + mutations[types.RESET_MERGE_REQUESTS](mockedState, 'created'); - expect(mockedState.mergeRequests).toEqual([]); + expect(mockedState.created.mergeRequests).toEqual([]); }); }); }); -- cgit v1.2.3 From 56efb9ee92cb96c6a378ae5d2f6510c7a870d896 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Wed, 6 Jun 2018 11:02:47 +0000 Subject: Adjust monitoring graphs to support widgets in EE --- spec/javascripts/monitoring/graph_spec.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'spec') diff --git a/spec/javascripts/monitoring/graph_spec.js b/spec/javascripts/monitoring/graph_spec.js index 220228e5c08..a46a387a534 100644 --- a/spec/javascripts/monitoring/graph_spec.js +++ b/spec/javascripts/monitoring/graph_spec.js @@ -18,9 +18,7 @@ const createComponent = propsData => { }).$mount(); }; -const convertedMetrics = convertDatesMultipleSeries( - singleRowMetricsMultipleSeries, -); +const convertedMetrics = convertDatesMultipleSeries(singleRowMetricsMultipleSeries); describe('Graph', () => { beforeEach(() => { @@ -36,7 +34,7 @@ describe('Graph', () => { projectPath, }); - expect(component.$el.querySelector('.text-center').innerText.trim()).toBe( + expect(component.$el.querySelector('.prometheus-graph-title').innerText.trim()).toBe( component.graphData.title, ); }); @@ -52,9 +50,7 @@ describe('Graph', () => { }); const transformedHeight = `${component.graphHeight - 100}`; - expect(component.axisTransform.indexOf(transformedHeight)).not.toEqual( - -1, - ); + expect(component.axisTransform.indexOf(transformedHeight)).not.toEqual(-1); }); it('outerViewBox gets a width and height property based on the DOM size of the element', () => { -- cgit v1.2.3 From dfb0d45ddb0747f5b72e7188d930737d57dabc4c Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 6 Jun 2018 20:18:32 +0900 Subject: Rename worker to ArchiveTracesCronWorker --- spec/workers/ci/archive_traces_cron_worker_spec.rb | 59 ++++++++++++++++++++++ .../ci/rescue_stale_live_trace_worker_spec.rb | 59 ---------------------- 2 files changed, 59 insertions(+), 59 deletions(-) create mode 100644 spec/workers/ci/archive_traces_cron_worker_spec.rb delete mode 100644 spec/workers/ci/rescue_stale_live_trace_worker_spec.rb (limited to 'spec') diff --git a/spec/workers/ci/archive_traces_cron_worker_spec.rb b/spec/workers/ci/archive_traces_cron_worker_spec.rb new file mode 100644 index 00000000000..9af51b7d4d8 --- /dev/null +++ b/spec/workers/ci/archive_traces_cron_worker_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe Ci::ArchiveTracesCronWorker do + subject { described_class.new.perform } + + before do + stub_feature_flags(ci_enable_live_trace: true) + end + + shared_examples_for 'archives trace' do + it do + subject + + build.reload + expect(build.job_artifacts_trace).to be_exist + end + end + + shared_examples_for 'does not archive trace' do + it do + subject + + build.reload + expect(build.job_artifacts_trace).to be_nil + end + end + + context 'when a job was succeeded' do + let!(:build) { create(:ci_build, :success, :trace_live) } + + it_behaves_like 'archives trace' + + context 'when archive raised an exception' do + let!(:build) { create(:ci_build, :success, :trace_artifact, :trace_live) } + let!(:build2) { create(:ci_build, :success, :trace_live) } + + it 'archives valid targets' do + expect(Rails.logger).to receive(:error).with("Failed to archive stale live trace. id: #{build.id} message: Already archived") + + subject + + build2.reload + expect(build2.job_artifacts_trace).to be_exist + end + end + end + + context 'when a job was cancelled' do + let!(:build) { create(:ci_build, :canceled, :trace_live) } + + it_behaves_like 'archives trace' + end + + context 'when a job is running' do + let!(:build) { create(:ci_build, :running, :trace_live) } + + it_behaves_like 'does not archive trace' + end +end diff --git a/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb b/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb deleted file mode 100644 index 0e3fbe2d3a9..00000000000 --- a/spec/workers/ci/rescue_stale_live_trace_worker_spec.rb +++ /dev/null @@ -1,59 +0,0 @@ -require 'spec_helper' - -describe Ci::RescueStaleLiveTraceWorker do - subject { described_class.new.perform } - - before do - stub_feature_flags(ci_enable_live_trace: true) - end - - shared_examples_for 'archives trace' do - it do - subject - - build.reload - expect(build.job_artifacts_trace).to be_exist - end - end - - shared_examples_for 'does not archive trace' do - it do - subject - - build.reload - expect(build.job_artifacts_trace).to be_nil - end - end - - context 'when a job was succeeded' do - let!(:build) { create(:ci_build, :success, :trace_live) } - - it_behaves_like 'archives trace' - - context 'when archive raised an exception' do - let!(:build) { create(:ci_build, :success, :trace_artifact, :trace_live) } - let!(:build2) { create(:ci_build, :success, :trace_live) } - - it 'archives valid targets' do - expect(Rails.logger).to receive(:error).with("Failed to archive stale live trace. id: #{build.id} message: Already archived") - - subject - - build2.reload - expect(build2.job_artifacts_trace).to be_exist - end - end - end - - context 'when a job was cancelled' do - let!(:build) { create(:ci_build, :canceled, :trace_live) } - - it_behaves_like 'archives trace' - end - - context 'when a job is running' do - let!(:build) { create(:ci_build, :running, :trace_live) } - - it_behaves_like 'does not archive trace' - end -end -- cgit v1.2.3 From 3b78b2d327ec00d201b74e74a81a9edb65a27a4c Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 6 Jun 2018 12:40:21 +0100 Subject: component specs --- .../ide/components/merge_requests/dropdown_spec.js | 47 ++++++++ .../ide/components/merge_requests/item_spec.js | 61 ++++++++++ .../ide/components/merge_requests/list_spec.js | 126 +++++++++++++++++++++ 3 files changed, 234 insertions(+) create mode 100644 spec/javascripts/ide/components/merge_requests/dropdown_spec.js create mode 100644 spec/javascripts/ide/components/merge_requests/item_spec.js create mode 100644 spec/javascripts/ide/components/merge_requests/list_spec.js (limited to 'spec') diff --git a/spec/javascripts/ide/components/merge_requests/dropdown_spec.js b/spec/javascripts/ide/components/merge_requests/dropdown_spec.js new file mode 100644 index 00000000000..74884c9a362 --- /dev/null +++ b/spec/javascripts/ide/components/merge_requests/dropdown_spec.js @@ -0,0 +1,47 @@ +import Vue from 'vue'; +import { createStore } from '~/ide/stores'; +import Dropdown from '~/ide/components/merge_requests/dropdown.vue'; +import { createComponentWithStore } from '../../../helpers/vue_mount_component_helper'; +import { mergeRequests } from '../../mock_data'; + +describe('IDE merge requests dropdown', () => { + const Component = Vue.extend(Dropdown); + let vm; + + beforeEach(() => { + const store = createStore(); + + vm = createComponentWithStore(Component, store, { show: false }).$mount(); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('does not render tabs when show is false', () => { + expect(vm.$el.querySelector('.nav-links')).toBe(null); + }); + + describe('when show is true', () => { + beforeEach(done => { + vm.show = true; + vm.$store.state.mergeRequests.assigned.mergeRequests.push(mergeRequests[0]); + + vm.$nextTick(done); + }); + + it('renders tabs', () => { + expect(vm.$el.querySelector('.nav-links')).not.toBe(null); + }); + + it('renders count for assigned & created data', () => { + expect(vm.$el.querySelector('.nav-links a').textContent).toContain('Created by me'); + expect(vm.$el.querySelector('.nav-links a .badge').textContent).toContain('0'); + + expect(vm.$el.querySelectorAll('.nav-links a')[1].textContent).toContain('Assigned to me'); + expect( + vm.$el.querySelectorAll('.nav-links a')[1].querySelector('.badge').textContent, + ).toContain('1'); + }); + }); +}); diff --git a/spec/javascripts/ide/components/merge_requests/item_spec.js b/spec/javascripts/ide/components/merge_requests/item_spec.js new file mode 100644 index 00000000000..51c4cddef2f --- /dev/null +++ b/spec/javascripts/ide/components/merge_requests/item_spec.js @@ -0,0 +1,61 @@ +import Vue from 'vue'; +import Item from '~/ide/components/merge_requests/item.vue'; +import mountCompontent from '../../../helpers/vue_mount_component_helper'; + +describe('IDE merge request item', () => { + const Component = Vue.extend(Item); + let vm; + + beforeEach(() => { + vm = mountCompontent(Component, { + item: { + iid: 1, + projectPathWithNamespace: 'gitlab-org/gitlab-ce', + title: 'Merge request title', + }, + currentId: '1', + currentProjectId: 'gitlab-org/gitlab-ce', + }); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('renders merge requests data', () => { + expect(vm.$el.textContent).toContain('Merge request title'); + expect(vm.$el.textContent).toContain('gitlab-org/gitlab-ce!1'); + }); + + it('renders icon if ID matches currentId', () => { + expect(vm.$el.querySelector('.ic-mobile-issue-close')).not.toBe(null); + }); + + it('does not render icon if ID does not match currentId', done => { + vm.currentId = '2'; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ic-mobile-issue-close')).toBe(null); + + done(); + }); + }); + + it('does not render icon if project ID does not match', done => { + vm.currentProjectId = 'test/test'; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ic-mobile-issue-close')).toBe(null); + + done(); + }); + }); + + it('emits click event on click', () => { + spyOn(vm, '$emit'); + + vm.$el.click(); + + expect(vm.$emit).toHaveBeenCalledWith('click', vm.item); + }); +}); diff --git a/spec/javascripts/ide/components/merge_requests/list_spec.js b/spec/javascripts/ide/components/merge_requests/list_spec.js new file mode 100644 index 00000000000..f4b393778dc --- /dev/null +++ b/spec/javascripts/ide/components/merge_requests/list_spec.js @@ -0,0 +1,126 @@ +import Vue from 'vue'; +import store from '~/ide/stores'; +import List from '~/ide/components/merge_requests/list.vue'; +import { createComponentWithStore } from '../../../helpers/vue_mount_component_helper'; +import { mergeRequests } from '../../mock_data'; +import { resetStore } from '../../helpers'; + +describe('IDE merge requests list', () => { + const Component = Vue.extend(List); + let vm; + + beforeEach(() => { + vm = createComponentWithStore(Component, store, { + type: 'created', + emptyText: 'empty text', + }); + + spyOn(vm, 'fetchMergeRequests'); + + vm.$mount(); + }); + + afterEach(() => { + vm.$destroy(); + + resetStore(vm.$store); + }); + + it('calls fetch on mounted', () => { + expect(vm.fetchMergeRequests).toHaveBeenCalledWith({ + type: 'created', + search: '', + }); + }); + + it('renders loading icon', done => { + vm.$store.state.mergeRequests.created.isLoading = true; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.loading-container')).not.toBe(null); + + done(); + }); + }); + + it('renders empty text when no merge requests exist', () => { + expect(vm.$el.textContent).toContain('empty text'); + }); + + it('renders no search results text when search is not empty', done => { + vm.search = 'testing'; + + vm.$nextTick(() => { + expect(vm.$el.textContent).toContain('No merge requests found'); + + done(); + }); + }); + + describe('with merge requests', () => { + beforeEach(done => { + vm.$store.state.mergeRequests.created.mergeRequests.push({ + ...mergeRequests[0], + projectPathWithNamespace: 'gitlab-org/gitlab-ce', + }); + + vm.$nextTick(done); + }); + + it('renders list', () => { + expect(vm.$el.querySelectorAll('li').length).toBe(1); + expect(vm.$el.querySelector('li').textContent).toContain(mergeRequests[0].title); + }); + + it('calls openMergeRequest when clicking merge request', done => { + spyOn(vm, 'openMergeRequest'); + vm.$el.querySelector('li button').click(); + + vm.$nextTick(() => { + expect(vm.openMergeRequest).toHaveBeenCalledWith({ + projectPath: 'gitlab-org/gitlab-ce', + id: 1, + }); + + done(); + }); + }); + }); + + describe('focusSearch', () => { + it('focuses search input when loading is false', done => { + spyOn(vm.$refs.searchInput, 'focus'); + + vm.$store.state.mergeRequests.created.isLoading = false; + vm.focusSearch(); + + vm.$nextTick(() => { + expect(vm.$refs.searchInput.focus).toHaveBeenCalled(); + + done(); + }); + }); + }); + + describe('searchMergeRequests', () => { + beforeEach(() => { + spyOn(vm, 'loadMergeRequests'); + + jasmine.clock().install(); + }); + + afterEach(() => { + jasmine.clock().uninstall(); + }); + + it('calls loadMergeRequests on input in search field', () => { + const event = new Event('input'); + + vm.$el.querySelector('input').dispatchEvent(event); + + jasmine.clock().tick(300); + + expect(vm.loadMergeRequests).toHaveBeenCalled(); + }); + }); +}); -- cgit v1.2.3 From cfcc7043fe614cbc5fc82f3021a0c4cd8519e24b Mon Sep 17 00:00:00 2001 From: Mark Chao Date: Tue, 22 May 2018 19:54:50 +0800 Subject: =?UTF-8?q?Rename=20=E2=80=9CDevelopers=20+=20Masters=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/features/protected_branches_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb index 0c28a853b54..4c0f9971425 100644 --- a/spec/features/protected_branches_spec.rb +++ b/spec/features/protected_branches_spec.rb @@ -72,7 +72,7 @@ feature 'Protected Branches', :js do click_link 'No one' find(".js-allowed-to-push").click wait_for_requests - click_link 'Developers + Masters' + click_link 'Developers + Maintainers' end visit project_protected_branches_path(project) @@ -82,7 +82,7 @@ feature 'Protected Branches', :js do expect(page.find(".dropdown-toggle-text")).to have_content("No one") end page.within(".js-allowed-to-push") do - expect(page.find(".dropdown-toggle-text")).to have_content("Developers + Masters") + expect(page.find(".dropdown-toggle-text")).to have_content("Developers + Maintainers") end end end -- cgit v1.2.3 From 7c965a24f51ca93196c27880843a6d187ea063cc Mon Sep 17 00:00:00 2001 From: Mark Chao Date: Tue, 22 May 2018 19:55:07 +0800 Subject: Fix spec --- spec/features/projects/settings/user_manages_group_links_spec.rb | 2 +- spec/features/projects/settings/user_manages_project_members_spec.rb | 2 +- spec/javascripts/notes/components/note_actions_spec.js | 4 ++-- spec/models/project_team_spec.rb | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) (limited to 'spec') diff --git a/spec/features/projects/settings/user_manages_group_links_spec.rb b/spec/features/projects/settings/user_manages_group_links_spec.rb index fdf42797091..92ce2ca83c7 100644 --- a/spec/features/projects/settings/user_manages_group_links_spec.rb +++ b/spec/features/projects/settings/user_manages_group_links_spec.rb @@ -30,7 +30,7 @@ describe 'Projects > Settings > User manages group links' do click_link('Share with group') select2(group_market.id, from: '#link_group_id') - select('Master', from: 'link_group_access') + select('Maintainer', from: 'link_group_access') click_button('Share') diff --git a/spec/features/projects/settings/user_manages_project_members_spec.rb b/spec/features/projects/settings/user_manages_project_members_spec.rb index 8af95522165..d3003753ae6 100644 --- a/spec/features/projects/settings/user_manages_project_members_spec.rb +++ b/spec/features/projects/settings/user_manages_project_members_spec.rb @@ -62,7 +62,7 @@ describe 'Projects > Settings > User manages project members' do page.within('.project-members-groups') do expect(page).to have_content('OpenSource') - expect(first('.group_member')).to have_content('Master') + expect(first('.group_member')).to have_content('Maintainer') end end end diff --git a/spec/javascripts/notes/components/note_actions_spec.js b/spec/javascripts/notes/components/note_actions_spec.js index 1dfe890e05e..c9e549d2096 100644 --- a/spec/javascripts/notes/components/note_actions_spec.js +++ b/spec/javascripts/notes/components/note_actions_spec.js @@ -20,7 +20,7 @@ describe('issue_note_actions component', () => { beforeEach(() => { props = { - accessLevel: 'Master', + accessLevel: 'Maintainer', authorId: 26, canDelete: true, canEdit: true, @@ -67,7 +67,7 @@ describe('issue_note_actions component', () => { beforeEach(() => { store.dispatch('setUserData', {}); props = { - accessLevel: 'Master', + accessLevel: 'Maintainer', authorId: 26, canDelete: false, canEdit: false, diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index e07c522800a..9978f3e9566 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -179,14 +179,14 @@ describe ProjectTeam do end describe "#human_max_access" do - it 'returns Master role' do + it 'returns Maintainer role' do user = create(:user) group = create(:group) project = create(:project, namespace: group) group.add_master(user) - expect(project.team.human_max_access(user.id)).to eq 'Master' + expect(project.team.human_max_access(user.id)).to eq 'Maintainer' end it 'returns Owner role' do -- cgit v1.2.3 From a6c15c5b9750b3b6b16694974eab868a21b8683a Mon Sep 17 00:00:00 2001 From: Mark Chao Date: Mon, 4 Jun 2018 10:25:38 +0900 Subject: change wording --- spec/lib/gitlab/checks/change_access_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/checks/change_access_spec.rb b/spec/lib/gitlab/checks/change_access_spec.rb index 48e9902027c..1cb8143a9e9 100644 --- a/spec/lib/gitlab/checks/change_access_spec.rb +++ b/spec/lib/gitlab/checks/change_access_spec.rb @@ -52,7 +52,7 @@ describe Gitlab::Checks::ChangeAccess do context 'with protected tag' do let!(:protected_tag) { create(:protected_tag, project: project, name: 'v*') } - context 'as master' do + context 'as maintainer' do before do project.add_master(user) end @@ -138,7 +138,7 @@ describe Gitlab::Checks::ChangeAccess do context 'if the user is not allowed to delete protected branches' do it 'raises an error' do - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to delete protected branches from this project. Only a project master or owner can delete a protected branch.') + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to delete protected branches from this project. Only a project maintainer or owner can delete a protected branch.') end end -- cgit v1.2.3 From 748f8fef457f2795ab2592bf6041055cf613bb10 Mon Sep 17 00:00:00 2001 From: Mark Chao Date: Mon, 4 Jun 2018 10:32:57 +0900 Subject: group runners --- spec/features/runners_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'spec') diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb index 9ce7d538004..fe0b03a7e00 100644 --- a/spec/features/runners_spec.rb +++ b/spec/features/runners_spec.rb @@ -184,7 +184,7 @@ feature 'Runners' do given(:group) { create :group } - context 'as project and group master' do + context 'as project and group maintainer' do background do group.add_master(user) end @@ -197,13 +197,13 @@ feature 'Runners' do expect(page).to have_content 'This group does not provide any group Runners yet' - expect(page).to have_content 'Group masters can register group runners in the Group CI/CD settings' - expect(page).not_to have_content 'Ask your group master to setup a group Runner' + expect(page).to have_content 'Group maintainers can register group runners in the Group CI/CD settings' + expect(page).not_to have_content 'Ask your group maintainer to setup a group Runner' end end end - context 'as project master' do + context 'as project maintainer' do context 'project without a group' do given(:project) { create :project } @@ -223,8 +223,8 @@ feature 'Runners' do expect(page).to have_content 'This group does not provide any group Runners yet.' - expect(page).not_to have_content 'Group masters can register group runners in the Group CI/CD settings' - expect(page).to have_content 'Ask your group master to setup a group Runner.' + expect(page).not_to have_content 'Group maintainers can register group runners in the Group CI/CD settings' + expect(page).to have_content 'Ask your group maintainer to setup a group Runner.' end end -- cgit v1.2.3 From 9fa70462296c85deb48574bf3e3ceed0cb2dd493 Mon Sep 17 00:00:00 2001 From: Mark Chao Date: Mon, 4 Jun 2018 12:50:28 +0900 Subject: fix lock spec --- spec/services/lfs/unlock_file_service_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/services/lfs/unlock_file_service_spec.rb b/spec/services/lfs/unlock_file_service_spec.rb index 4bea112b9c6..948401d7bdc 100644 --- a/spec/services/lfs/unlock_file_service_spec.rb +++ b/spec/services/lfs/unlock_file_service_spec.rb @@ -80,12 +80,12 @@ describe Lfs::UnlockFileService do result = subject.execute expect(result[:status]).to eq(:error) - expect(result[:message]).to match(/You must have master access/) + expect(result[:message]).to match(/You must have maintainer access/) expect(result[:http_status]).to eq(403) end end - context 'by a master user' do + context 'by a maintainer user' do let(:current_user) { master } let(:params) do { id: lock.id, -- cgit v1.2.3 From 51a9f71118e9cd4bec50b6a5872d035f219e4eaf Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Wed, 6 Jun 2018 15:21:13 +0200 Subject: Add a custom storage config instead of overwriting Otherwise the ApplicationSetting that is currently stored would be invalid as it specifies a storage that is not configured --- spec/requests/api/settings_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index aead8978dd4..57adc3ca7a6 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -35,7 +35,9 @@ describe API::Settings, 'Settings' do context "custom repository storage type set in the config" do before do - storages = { 'custom' => 'tmp/tests/custom_repositories' } + # Add a possible storage to the config + storages = Gitlab.config.repositories.storages + .merge({ 'custom' => 'tmp/tests/custom_repositories' }) allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) end -- cgit v1.2.3 From cc9468e4fa276cef5673750464889f2d2791a9a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20Carlb=C3=A4cker?= Date: Wed, 6 Jun 2018 14:28:03 +0000 Subject: Move GC/Repack to OptOut --- spec/workers/git_garbage_collect_worker_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb index 74539a7e493..f44b4edc305 100644 --- a/spec/workers/git_garbage_collect_worker_spec.rb +++ b/spec/workers/git_garbage_collect_worker_spec.rb @@ -104,7 +104,7 @@ describe GitGarbageCollectWorker do it_should_behave_like 'flushing ref caches', true end - context "with Gitaly turned off", :skip_gitaly_mock do + context "with Gitaly turned off", :disable_gitaly do it_should_behave_like 'flushing ref caches', false end -- cgit v1.2.3 From e8f49b4bee8d803953b852685889a2912609ae84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Javier=20L=C3=B3pez?= Date: Wed, 6 Jun 2018 16:42:18 +0000 Subject: Support LFS objects when creating a project by import --- spec/lib/gitlab/git/rev_list_spec.rb | 2 +- .../importer/lfs_object_importer_spec.rb | 23 ++++ .../importer/lfs_objects_importer_spec.rb | 94 +++++++++++++ .../importer/repository_importer_spec.rb | 3 +- spec/lib/gitlab/import_sources_spec.rb | 19 +++ spec/services/projects/import_service_spec.rb | 72 +++++++++- .../lfs_download_link_list_service_spec.rb | 102 ++++++++++++++ .../lfs_pointers/lfs_download_service_spec.rb | 69 ++++++++++ .../lfs_pointers/lfs_import_service_spec.rb | 146 +++++++++++++++++++++ .../projects/lfs_pointers/lfs_link_service_spec.rb | 33 +++++ .../stage/import_lfs_objects_worker_spec.rb | 28 ++++ .../stage/import_notes_worker_spec.rb | 2 +- 12 files changed, 589 insertions(+), 4 deletions(-) create mode 100644 spec/lib/gitlab/github_import/importer/lfs_object_importer_spec.rb create mode 100644 spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb create mode 100644 spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb create mode 100644 spec/services/projects/lfs_pointers/lfs_download_service_spec.rb create mode 100644 spec/services/projects/lfs_pointers/lfs_import_service_spec.rb create mode 100644 spec/services/projects/lfs_pointers/lfs_link_service_spec.rb create mode 100644 spec/workers/gitlab/github_import/stage/import_lfs_objects_worker_spec.rb (limited to 'spec') diff --git a/spec/lib/gitlab/git/rev_list_spec.rb b/spec/lib/gitlab/git/rev_list_spec.rb index 95dc47e2a00..70e90659b0f 100644 --- a/spec/lib/gitlab/git/rev_list_spec.rb +++ b/spec/lib/gitlab/git/rev_list_spec.rb @@ -88,7 +88,7 @@ describe Gitlab::Git::RevList do context '#all_objects' do it 'fetches list of all pushed objects using rev-list' do - stub_popen_rev_list('--all', '--objects', output: "sha1\nsha2") + stub_popen_rev_list('--all', '--objects', '--filter=blob:limit=200', output: "sha1\nsha2") expect { |b| rev_list.all_objects(&b) }.to yield_with_args(%w[sha1 sha2]) end diff --git a/spec/lib/gitlab/github_import/importer/lfs_object_importer_spec.rb b/spec/lib/gitlab/github_import/importer/lfs_object_importer_spec.rb new file mode 100644 index 00000000000..4857f2afbe2 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/lfs_object_importer_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::LfsObjectImporter do + let(:project) { create(:project) } + let(:download_link) { "http://www.gitlab.com/lfs_objects/oid" } + + let(:github_lfs_object) do + Gitlab::GithubImport::Representation::LfsObject.new( + oid: 'oid', download_link: download_link + ) + end + + let(:importer) { described_class.new(github_lfs_object, project, nil) } + + describe '#execute' do + it 'calls the LfsDownloadService with the lfs object attributes' do + expect_any_instance_of(Projects::LfsPointers::LfsDownloadService) + .to receive(:execute).with('oid', download_link) + + importer.execute + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb b/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb new file mode 100644 index 00000000000..5f5c6b803c0 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb @@ -0,0 +1,94 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::LfsObjectsImporter do + let(:project) { double(:project, id: 4, import_source: 'foo/bar') } + let(:client) { double(:client) } + let(:download_link) { "http://www.gitlab.com/lfs_objects/oid" } + + let(:github_lfs_object) { ['oid', download_link] } + + describe '#parallel?' do + it 'returns true when running in parallel mode' do + importer = described_class.new(project, client) + expect(importer).to be_parallel + end + + it 'returns false when running in sequential mode' do + importer = described_class.new(project, client, parallel: false) + expect(importer).not_to be_parallel + end + end + + describe '#execute' do + context 'when running in parallel mode' do + it 'imports lfs objects in parallel' do + importer = described_class.new(project, client) + + expect(importer).to receive(:parallel_import) + + importer.execute + end + end + + context 'when running in sequential mode' do + it 'imports lfs objects in sequence' do + importer = described_class.new(project, client, parallel: false) + + expect(importer).to receive(:sequential_import) + + importer.execute + end + end + end + + describe '#sequential_import' do + it 'imports each lfs object in sequence' do + importer = described_class.new(project, client, parallel: false) + lfs_object_importer = double(:lfs_object_importer) + + allow(importer) + .to receive(:each_object_to_import) + .and_yield(['oid', download_link]) + + expect(Gitlab::GithubImport::Importer::LfsObjectImporter) + .to receive(:new) + .with( + an_instance_of(Gitlab::GithubImport::Representation::LfsObject), + project, + client + ) + .and_return(lfs_object_importer) + + expect(lfs_object_importer).to receive(:execute) + + importer.sequential_import + end + end + + describe '#parallel_import' do + it 'imports each lfs object in parallel' do + importer = described_class.new(project, client) + + allow(importer) + .to receive(:each_object_to_import) + .and_yield(github_lfs_object) + + expect(Gitlab::GithubImport::ImportLfsObjectWorker) + .to receive(:perform_async) + .with(project.id, an_instance_of(Hash), an_instance_of(String)) + + waiter = importer.parallel_import + + expect(waiter).to be_an_instance_of(Gitlab::JobWaiter) + expect(waiter.jobs_remaining).to eq(1) + end + end + + describe '#collection_options' do + it 'returns an empty Hash' do + importer = described_class.new(project, client) + + expect(importer.collection_options).to eq({}) + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb index cc9e4b67e72..d8f01dcb76b 100644 --- a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb @@ -14,7 +14,8 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do disk_path: 'foo', repository: repository, create_wiki: true, - import_state: import_state + import_state: import_state, + lfs_enabled?: true ) end diff --git a/spec/lib/gitlab/import_sources_spec.rb b/spec/lib/gitlab/import_sources_spec.rb index f2fa315e3ec..10341486512 100644 --- a/spec/lib/gitlab/import_sources_spec.rb +++ b/spec/lib/gitlab/import_sources_spec.rb @@ -91,4 +91,23 @@ describe Gitlab::ImportSources do end end end + + describe 'imports_repository? checker' do + let(:allowed_importers) { %w[github gitlab_project] } + + it 'fails if any importer other than the allowed ones implements this method' do + current_importers = described_class.values.select { |kind| described_class.importer(kind).try(:imports_repository?) } + not_allowed_importers = current_importers - allowed_importers + + expect(not_allowed_importers).to be_empty, failure_message(not_allowed_importers) + end + + def failure_message(importers_class_names) + <<-MSG + It looks like the #{importers_class_names.join(', ')} importers implements its own way to import the repository. + That means that the lfs object download must be handled for each of them. You can use 'LfsImportService' and + 'LfsDownloadService' to implement it. After that, add the importer name to the list of allowed importers in this spec. + MSG + end + end end diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index 30c89ebd821..b3815045792 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -3,9 +3,17 @@ require 'spec_helper' describe Projects::ImportService do let!(:project) { create(:project) } let(:user) { project.creator } + let(:import_url) { 'http://www.gitlab.com/demo/repo.git' } + let(:oid_download_links) { { 'oid1' => "#{import_url}/gitlab-lfs/objects/oid1", 'oid2' => "#{import_url}/gitlab-lfs/objects/oid2" } } subject { described_class.new(project, user) } + before do + allow(project).to receive(:lfs_enabled?).and_return(true) + allow_any_instance_of(Projects::LfsPointers::LfsDownloadService).to receive(:execute) + allow_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(oid_download_links) + end + describe '#async?' do it 'returns true for an asynchronous importer' do importer_class = double(:importer, async?: true) @@ -63,6 +71,15 @@ describe Projects::ImportService do expect(result[:status]).to eq :error expect(result[:message]).to eq "Error importing repository #{project.import_url} into #{project.full_path} - The repository could not be created." end + + context 'when repository creation succeeds' do + it 'does not download lfs files' do + expect_any_instance_of(Projects::LfsPointers::LfsImportService).not_to receive(:execute) + expect_any_instance_of(Projects::LfsPointers::LfsDownloadService).not_to receive(:execute) + + subject.execute + end + end end context 'with known url' do @@ -91,6 +108,15 @@ describe Projects::ImportService do expect(result[:status]).to eq :error end + + context 'when repository import scheduled' do + it 'does not download lfs objects' do + expect_any_instance_of(Projects::LfsPointers::LfsImportService).not_to receive(:execute) + expect_any_instance_of(Projects::LfsPointers::LfsDownloadService).not_to receive(:execute) + + subject.execute + end + end end context 'with a non Github repository' do @@ -99,9 +125,10 @@ describe Projects::ImportService do project.import_type = 'bitbucket' end - it 'succeeds if repository import is successfully' do + it 'succeeds if repository import is successfull' do expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_return(true) expect_any_instance_of(Gitlab::BitbucketImport::Importer).to receive(:execute).and_return(true) + expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return({}) result = subject.execute @@ -116,6 +143,29 @@ describe Projects::ImportService do expect(result[:status]).to eq :error expect(result[:message]).to eq "Error importing repository #{project.import_url} into #{project.full_path} - Failed to import the repository" end + + context 'when repository import scheduled' do + before do + allow_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_return(true) + allow(subject).to receive(:import_data) + end + + it 'downloads lfs objects if lfs_enabled is enabled for project' do + allow(project).to receive(:lfs_enabled?).and_return(true) + expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(oid_download_links) + expect_any_instance_of(Projects::LfsPointers::LfsDownloadService).to receive(:execute).twice + + subject.execute + end + + it 'does not download lfs objects if lfs_enabled is not enabled for project' do + allow(project).to receive(:lfs_enabled?).and_return(false) + expect_any_instance_of(Projects::LfsPointers::LfsImportService).not_to receive(:execute) + expect_any_instance_of(Projects::LfsPointers::LfsDownloadService).not_to receive(:execute) + + subject.execute + end + end end end @@ -147,6 +197,26 @@ describe Projects::ImportService do expect(result[:status]).to eq :error end + + context 'when importer' do + it 'has a custom repository importer it does not download lfs objects' do + allow(Gitlab::GithubImport::ParallelImporter).to receive(:imports_repository?).and_return(true) + + expect_any_instance_of(Projects::LfsPointers::LfsImportService).not_to receive(:execute) + expect_any_instance_of(Projects::LfsPointers::LfsDownloadService).not_to receive(:execute) + + subject.execute + end + + it 'does not have a custom repository importer downloads lfs objects' do + allow(Gitlab::GithubImport::ParallelImporter).to receive(:imports_repository?).and_return(false) + + expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(oid_download_links) + expect_any_instance_of(Projects::LfsPointers::LfsDownloadService).to receive(:execute) + + subject.execute + end + end end context 'with blocked import_URL' do diff --git a/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb new file mode 100644 index 00000000000..d7a2829d5f8 --- /dev/null +++ b/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb @@ -0,0 +1,102 @@ +require 'spec_helper' + +describe Projects::LfsPointers::LfsDownloadLinkListService do + let(:import_url) { 'http://www.gitlab.com/demo/repo.git' } + let(:lfs_endpoint) { "#{import_url}/info/lfs/objects/batch" } + let!(:project) { create(:project, import_url: import_url) } + let(:new_oids) { { 'oid1' => 123, 'oid2' => 125 } } + let(:remote_uri) { URI.parse(lfs_endpoint) } + + let(:objects_response) do + body = new_oids.map do |oid, size| + { + 'oid' => oid, + 'size' => size, + 'actions' => { + 'download' => { 'href' => "#{import_url}/gitlab-lfs/objects/#{oid}" } + } + } + end + + Struct.new(:success?, :objects).new(true, body) + end + + let(:invalid_object_response) do + [ + 'oid' => 'whatever', + 'size' => 123 + ] + end + + subject { described_class.new(project, remote_uri: remote_uri) } + + before do + allow(project).to receive(:lfs_enabled?).and_return(true) + allow(Gitlab::HTTP).to receive(:post).and_return(objects_response) + end + + describe '#execute' do + it 'retrieves each download link of every non existent lfs object' do + subject.execute(new_oids).each do |oid, link| + expect(link).to eq "#{import_url}/gitlab-lfs/objects/#{oid}" + end + end + + context 'credentials' do + context 'when the download link and the lfs_endpoint have the same host' do + context 'when lfs_endpoint has credentials' do + let(:import_url) { 'http://user:password@www.gitlab.com/demo/repo.git' } + + it 'adds credentials to the download_link' do + result = subject.execute(new_oids) + + result.each do |oid, link| + expect(link.starts_with?('http://user:password@')).to be_truthy + end + end + end + + context 'when lfs_endpoint does not have any credentials' do + it 'does not add any credentials' do + result = subject.execute(new_oids) + + result.each do |oid, link| + expect(link.starts_with?('http://user:password@')).to be_falsey + end + end + end + end + + context 'when the download link and the lfs_endpoint have different hosts' do + let(:import_url_with_credentials) { 'http://user:password@www.otherdomain.com/demo/repo.git' } + let(:lfs_endpoint) { "#{import_url_with_credentials}/info/lfs/objects/batch" } + + it 'downloads without any credentials' do + result = subject.execute(new_oids) + + result.each do |oid, link| + expect(link.starts_with?('http://user:password@')).to be_falsey + end + end + end + end + end + + describe '#get_download_links' do + it 'raise errorif request fails' do + allow(Gitlab::HTTP).to receive(:post).and_return(Struct.new(:success?, :message).new(false, 'Failed request')) + + expect { subject.send(:get_download_links, new_oids) }.to raise_error(described_class::DownloadLinksError) + end + end + + describe '#parse_response_links' do + it 'does not add oid entry if href not found' do + expect(Rails.logger).to receive(:error).with("Link for Lfs Object with oid whatever not found or invalid.") + + result = subject.send(:parse_response_links, invalid_object_response) + + expect(result).to be_empty + end + end +end diff --git a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb new file mode 100644 index 00000000000..6af5bfc7689 --- /dev/null +++ b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +describe Projects::LfsPointers::LfsDownloadService do + let(:project) { create(:project) } + let(:oid) { '9e548e25631dd9ce6b43afd6359ab76da2819d6a5b474e66118c7819e1d8b3e8' } + let(:download_link) { "http://gitlab.com/#{oid}" } + let(:lfs_content) do + <<~HEREDOC + whatever + HEREDOC + end + + subject { described_class.new(project) } + + before do + allow(project).to receive(:lfs_enabled?).and_return(true) + WebMock.stub_request(:get, download_link).to_return(body: lfs_content) + end + + describe '#execute' do + context 'when file download succeeds' do + it 'a new lfs object is created' do + expect { subject.execute(oid, download_link) }.to change { LfsObject.count }.from(0).to(1) + end + + it 'has the same oid' do + subject.execute(oid, download_link) + + expect(LfsObject.first.oid).to eq oid + end + + it 'stores the content' do + subject.execute(oid, download_link) + + expect(File.read(LfsObject.first.file.file.file)).to eq lfs_content + end + end + + context 'when file download fails' do + it 'no lfs object is created' do + expect { subject.execute(oid, download_link) }.to change { LfsObject.count } + end + end + + context 'when credentials present' do + let(:download_link_with_credentials) { "http://user:password@gitlab.com/#{oid}" } + + before do + WebMock.stub_request(:get, download_link).with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNzd29yZA==' }).to_return(body: lfs_content) + end + + it 'the request adds authorization headers' do + subject.execute(oid, download_link_with_credentials) + end + end + + context 'when an lfs object with the same oid already exists' do + before do + create(:lfs_object, oid: 'oid') + end + + it 'does not download the file' do + expect(subject).not_to receive(:download_and_save_file) + + subject.execute('oid', download_link) + end + end + end +end diff --git a/spec/services/projects/lfs_pointers/lfs_import_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_import_service_spec.rb new file mode 100644 index 00000000000..5a75fb38dec --- /dev/null +++ b/spec/services/projects/lfs_pointers/lfs_import_service_spec.rb @@ -0,0 +1,146 @@ +require 'spec_helper' + +describe Projects::LfsPointers::LfsImportService do + let(:import_url) { 'http://www.gitlab.com/demo/repo.git' } + let(:default_endpoint) { "#{import_url}/info/lfs/objects/batch"} + let(:group) { create(:group, lfs_enabled: true)} + let!(:project) { create(:project, namespace: group, import_url: import_url, lfs_enabled: true) } + let!(:lfs_objects_project) { create_list(:lfs_objects_project, 2, project: project) } + let!(:existing_lfs_objects) { LfsObject.pluck(:oid, :size).to_h } + let(:oids) { { 'oid1' => 123, 'oid2' => 125 } } + let(:oid_download_links) { { 'oid1' => "#{import_url}/gitlab-lfs/objects/oid1", 'oid2' => "#{import_url}/gitlab-lfs/objects/oid2" } } + let(:all_oids) { existing_lfs_objects.merge(oids) } + let(:remote_uri) { URI.parse(lfs_endpoint) } + + subject { described_class.new(project) } + + before do + allow(project.repository).to receive(:lfsconfig_for).and_return(nil) + allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) + allow_any_instance_of(Projects::LfsPointers::LfsListService).to receive(:execute).and_return(all_oids) + end + + describe '#execute' do + context 'when no lfs pointer is linked' do + before do + allow_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute).and_return([]) + allow_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).and_return(oid_download_links) + expect(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:new).with(project, remote_uri: URI.parse(default_endpoint)).and_call_original + end + + it 'retrieves all lfs pointers in the project repository' do + expect_any_instance_of(Projects::LfsPointers::LfsListService).to receive(:execute) + + subject.execute + end + + it 'links existent lfs objects to the project' do + expect_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute) + + subject.execute + end + + it 'retrieves the download links of non existent objects' do + expect_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).with(all_oids) + + subject.execute + end + end + + context 'when some lfs objects are linked' do + before do + allow_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute).and_return(existing_lfs_objects.keys) + allow_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).and_return(oid_download_links) + end + + it 'retrieves the download links of non existent objects' do + expect_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).with(oids) + + subject.execute + end + end + + context 'when all lfs objects are linked' do + before do + allow_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute).and_return(all_oids.keys) + allow_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute) + end + + it 'retrieves no download links' do + expect_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).with({}).and_call_original + + expect(subject.execute).to be_empty + end + end + + context 'when lfsconfig file exists' do + before do + allow(project.repository).to receive(:lfsconfig_for).and_return("[lfs]\n\turl = #{lfs_endpoint}\n") + end + + context 'when url points to the same import url host' do + let(:lfs_endpoint) { "#{import_url}/different_endpoint" } + let(:service) { double } + + before do + allow(service).to receive(:execute) + end + it 'downloads lfs object using the new endpoint' do + expect(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:new).with(project, remote_uri: remote_uri).and_return(service) + + subject.execute + end + + context 'when import url has credentials' do + let(:import_url) { 'http://user:password@www.gitlab.com/demo/repo.git'} + + it 'adds the credentials to the new endpoint' do + expect(Projects::LfsPointers::LfsDownloadLinkListService) + .to receive(:new).with(project, remote_uri: URI.parse("http://user:password@www.gitlab.com/demo/repo.git/different_endpoint")) + .and_return(service) + + subject.execute + end + + context 'when url has its own credentials' do + let(:lfs_endpoint) { "http://user1:password1@www.gitlab.com/demo/repo.git/different_endpoint" } + + it 'does not add the import url credentials' do + expect(Projects::LfsPointers::LfsDownloadLinkListService) + .to receive(:new).with(project, remote_uri: remote_uri) + .and_return(service) + + subject.execute + end + end + end + end + + context 'when url points to a third party service' do + let(:lfs_endpoint) { 'http://third_party_service.com/info/lfs/objects/' } + + it 'disables lfs from the project' do + expect(project.lfs_enabled?).to be_truthy + + subject.execute + + expect(project.lfs_enabled?).to be_falsey + end + + it 'does not download anything' do + expect_any_instance_of(Projects::LfsPointers::LfsListService).not_to receive(:execute) + + subject.execute + end + end + end + end + + describe '#default_endpoint_uri' do + let(:import_url) { 'http://www.gitlab.com/demo/repo' } + + it 'adds suffix .git if the url does not have it' do + expect(subject.send(:default_endpoint_uri).path).to match(/repo.git/) + end + end +end diff --git a/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb new file mode 100644 index 00000000000..b7b153655db --- /dev/null +++ b/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe Projects::LfsPointers::LfsLinkService do + let!(:project) { create(:project, lfs_enabled: true) } + let!(:lfs_objects_project) { create_list(:lfs_objects_project, 2, project: project) } + let(:new_oids) { { 'oid1' => 123, 'oid2' => 125 } } + let(:all_oids) { LfsObject.pluck(:oid, :size).to_h.merge(new_oids) } + let(:new_lfs_object) { create(:lfs_object) } + let(:new_oid_list) { all_oids.merge(new_lfs_object.oid => new_lfs_object.size) } + + subject { described_class.new(project) } + + before do + allow(project).to receive(:lfs_enabled?).and_return(true) + end + + describe '#execute' do + it 'links existing lfs objects to the project' do + expect(project.all_lfs_objects.count).to eq 2 + + linked = subject.execute(new_oid_list.keys) + + expect(project.all_lfs_objects.count).to eq 3 + expect(linked.size).to eq 3 + end + + it 'returns linked oids' do + linked = lfs_objects_project.map(&:lfs_object).map(&:oid) << new_lfs_object.oid + + expect(subject.execute(new_oid_list.keys)).to eq linked + end + end +end diff --git a/spec/workers/gitlab/github_import/stage/import_lfs_objects_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_lfs_objects_worker_spec.rb new file mode 100644 index 00000000000..b19884d7991 --- /dev/null +++ b/spec/workers/gitlab/github_import/stage/import_lfs_objects_worker_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Stage::ImportLfsObjectsWorker do + let(:project) { create(:project) } + let(:worker) { described_class.new } + + describe '#import' do + it 'imports all the lfs objects' do + importer = double(:importer) + waiter = Gitlab::JobWaiter.new(2, '123') + + expect(Gitlab::GithubImport::Importer::LfsObjectsImporter) + .to receive(:new) + .with(project, nil) + .and_return(importer) + + expect(importer) + .to receive(:execute) + .and_return(waiter) + + expect(Gitlab::GithubImport::AdvanceStageWorker) + .to receive(:perform_async) + .with(project.id, { '123' => 2 }, :finish) + + worker.import(project) + end + end +end diff --git a/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb index 098d2d55386..94cff9e4e80 100644 --- a/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb @@ -21,7 +21,7 @@ describe Gitlab::GithubImport::Stage::ImportNotesWorker do expect(Gitlab::GithubImport::AdvanceStageWorker) .to receive(:perform_async) - .with(project.id, { '123' => 2 }, :finish) + .with(project.id, { '123' => 2 }, :lfs_objects) worker.import(client, project) end -- cgit v1.2.3