Commit 428a9621 authored by Alex Kalderimis's avatar Alex Kalderimis

Test fixes as a result of graphql_helpers changes

Most of these are trivial (adding `#to_a` for example, or changing
to use client side argument names). The labels_resolver_spec changes
are more invasive since this spec tested a number of impossible states
(passing arguments not declared by the resolver) - these particular
changes were approved by @acroitor as part of
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40088.

The authorization_spec changes are made in such as way as to minimise
the impact of changes from
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40088
from which this work was extracted.
parent 661b48b2
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'Gitlab::Graphql::Authorization' do RSpec.describe 'Gitlab::Graphql::Authorize' do
include GraphqlHelpers include GraphqlHelpers
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
...@@ -10,7 +10,11 @@ RSpec.describe 'Gitlab::Graphql::Authorization' do ...@@ -10,7 +10,11 @@ RSpec.describe 'Gitlab::Graphql::Authorization' do
let(:permission_collection) { [:foo, :bar] } let(:permission_collection) { [:foo, :bar] }
let(:test_object) { double(name: 'My name') } let(:test_object) { double(name: 'My name') }
let(:query_string) { '{ item { name } }' } let(:query_string) { '{ item { name } }' }
let(:result) { execute_query(query_type)['data'] } let(:result) do
schema = empty_schema
schema.use(Gitlab::Graphql::Authorize)
execute_query(query_type, schema)['data']
end
subject { result['item'] } subject { result['item'] }
......
...@@ -21,9 +21,9 @@ RSpec.describe Mutations::ReleaseAssetLinks::Create do ...@@ -21,9 +21,9 @@ RSpec.describe Mutations::ReleaseAssetLinks::Create do
let(:args) do let(:args) do
{ {
project_path: project_path, project_path: project_path,
tag: tag, tag_name: tag,
name: name, name: name,
filepath: filepath, direct_asset_path: filepath,
url: url url: url
} }
end end
...@@ -44,9 +44,9 @@ RSpec.describe Mutations::ReleaseAssetLinks::Create do ...@@ -44,9 +44,9 @@ RSpec.describe Mutations::ReleaseAssetLinks::Create do
expect(release.links.length).to be(1) expect(release.links.length).to be(1)
expect(last_release_link.name).to eq(args[:name]) expect(last_release_link.name).to eq(name)
expect(last_release_link.url).to eq(args[:url]) expect(last_release_link.url).to eq(url)
expect(last_release_link.filepath).to eq(args[:filepath]) expect(last_release_link.filepath).to eq(filepath)
end end
end end
......
...@@ -9,7 +9,6 @@ RSpec.describe ::CachingArrayResolver do ...@@ -9,7 +9,6 @@ RSpec.describe ::CachingArrayResolver do
let_it_be(:admins) { create_list(:user, 4, admin: true) } let_it_be(:admins) { create_list(:user, 4, admin: true) }
let(:query_context) { { current_user: admins.first } } let(:query_context) { { current_user: admins.first } }
let(:max_page_size) { 10 } let(:max_page_size) { 10 }
let(:field) { double('Field', max_page_size: max_page_size) }
let(:schema) do let(:schema) do
Class.new(GitlabSchema) do Class.new(GitlabSchema) do
default_max_page_size 3 default_max_page_size 3
...@@ -210,6 +209,6 @@ RSpec.describe ::CachingArrayResolver do ...@@ -210,6 +209,6 @@ RSpec.describe ::CachingArrayResolver do
args = { is_admin: admin } args = { is_admin: admin }
opts = resolver.field_options opts = resolver.field_options
allow(resolver).to receive(:field_options).and_return(opts.merge(max_page_size: max_page_size)) allow(resolver).to receive(:field_options).and_return(opts.merge(max_page_size: max_page_size))
resolve(resolver, args: args, ctx: query_context, schema: schema, field: field) resolve(resolver, args: args, ctx: query_context, schema: schema)
end end
end end
...@@ -19,7 +19,7 @@ RSpec.describe Resolvers::ErrorTracking::SentryErrorsResolver do ...@@ -19,7 +19,7 @@ RSpec.describe Resolvers::ErrorTracking::SentryErrorsResolver do
end end
describe '#resolve' do describe '#resolve' do
context 'insufficient user permission' do context 'with insufficient user permission' do
let(:user) { create(:user) } let(:user) { create(:user) }
it 'returns nil' do it 'returns nil' do
...@@ -29,7 +29,7 @@ RSpec.describe Resolvers::ErrorTracking::SentryErrorsResolver do ...@@ -29,7 +29,7 @@ RSpec.describe Resolvers::ErrorTracking::SentryErrorsResolver do
end end
end end
context 'user with permission' do context 'with sufficient permission' do
before do before do
project.add_developer(current_user) project.add_developer(current_user)
...@@ -93,7 +93,7 @@ RSpec.describe Resolvers::ErrorTracking::SentryErrorsResolver do ...@@ -93,7 +93,7 @@ RSpec.describe Resolvers::ErrorTracking::SentryErrorsResolver do
end end
it 'returns an externally paginated array' do it 'returns an externally paginated array' do
expect(resolve_errors).to be_a Gitlab::Graphql::ExternallyPaginatedArray expect(resolve_errors).to be_a Gitlab::Graphql::Pagination::ExternallyPaginatedArrayConnection
end end
end end
end end
......
...@@ -42,7 +42,7 @@ RSpec.describe Resolvers::GroupLabelsResolver do ...@@ -42,7 +42,7 @@ RSpec.describe Resolvers::GroupLabelsResolver do
context 'without parent' do context 'without parent' do
it 'returns no labels' do it 'returns no labels' do
expect(resolve_labels(nil)).to eq(Label.none) expect(resolve_labels(nil)).to be_empty
end end
end end
......
...@@ -264,7 +264,7 @@ RSpec.describe Resolvers::IssuesResolver do ...@@ -264,7 +264,7 @@ RSpec.describe Resolvers::IssuesResolver do
end end
it 'finds a specific issue with iid', :request_store do it 'finds a specific issue with iid', :request_store do
result = batch_sync(max_queries: 4) { resolve_issues(iid: issue1.iid) } result = batch_sync(max_queries: 4) { resolve_issues(iid: issue1.iid).to_a }
expect(result).to contain_exactly(issue1) expect(result).to contain_exactly(issue1)
end end
...@@ -281,7 +281,7 @@ RSpec.describe Resolvers::IssuesResolver do ...@@ -281,7 +281,7 @@ RSpec.describe Resolvers::IssuesResolver do
it 'finds a specific issue with iids', :request_store do it 'finds a specific issue with iids', :request_store do
result = batch_sync(max_queries: 4) do result = batch_sync(max_queries: 4) do
resolve_issues(iids: [issue1.iid]) resolve_issues(iids: [issue1.iid]).to_a
end end
expect(result).to contain_exactly(issue1) expect(result).to contain_exactly(issue1)
...@@ -290,7 +290,7 @@ RSpec.describe Resolvers::IssuesResolver do ...@@ -290,7 +290,7 @@ RSpec.describe Resolvers::IssuesResolver do
it 'finds multiple issues with iids' do it 'finds multiple issues with iids' do
create(:issue, project: project, author: current_user) create(:issue, project: project, author: current_user)
expect(batch_sync { resolve_issues(iids: [issue1.iid, issue2.iid]) }) expect(batch_sync { resolve_issues(iids: [issue1.iid, issue2.iid]).to_a })
.to contain_exactly(issue1, issue2) .to contain_exactly(issue1, issue2)
end end
...@@ -302,7 +302,7 @@ RSpec.describe Resolvers::IssuesResolver do ...@@ -302,7 +302,7 @@ RSpec.describe Resolvers::IssuesResolver do
create(:issue, project: another_project, iid: iid) create(:issue, project: another_project, iid: iid)
end end
expect(batch_sync { resolve_issues(iids: iids) }).to contain_exactly(issue1, issue2) expect(batch_sync { resolve_issues(iids: iids).to_a }).to contain_exactly(issue1, issue2)
end end
end end
end end
......
...@@ -42,7 +42,7 @@ RSpec.describe Resolvers::LabelsResolver do ...@@ -42,7 +42,7 @@ RSpec.describe Resolvers::LabelsResolver do
context 'without parent' do context 'without parent' do
it 'returns no labels' do it 'returns no labels' do
expect(resolve_labels(nil)).to eq(Label.none) expect(resolve_labels(nil)).to be_empty
end end
end end
...@@ -53,39 +53,27 @@ RSpec.describe Resolvers::LabelsResolver do ...@@ -53,39 +53,27 @@ RSpec.describe Resolvers::LabelsResolver do
# because :include_ancestor_groups, :include_descendant_groups, :only_group_labels default to false # because :include_ancestor_groups, :include_descendant_groups, :only_group_labels default to false
# the `nil` value would be equivalent to passing in `false` so just check for `nil` option # the `nil` value would be equivalent to passing in `false` so just check for `nil` option
where(:include_ancestor_groups, :include_descendant_groups, :only_group_labels, :search_term, :test) do # the expected result is wrapped in a lambda to get around the phase restrictions of RSpec::Parameterized
nil | nil | nil | nil | -> { expect(subject).to contain_exactly(label1, label2, subgroup_label1, subgroup_label2) } where(:include_ancestor_groups, :search_term, :expected_labels) do
nil | nil | true | nil | -> { expect(subject).to contain_exactly(label1, label2, subgroup_label1, subgroup_label2) } nil | nil | -> { [label1, label2, subgroup_label1, subgroup_label2] }
nil | true | nil | nil | -> { expect(subject).to contain_exactly(label1, label2, subgroup_label1, subgroup_label2, sub_subgroup_label1, sub_subgroup_label2) } false | nil | -> { [label1, label2, subgroup_label1, subgroup_label2] }
nil | true | true | nil | -> { expect(subject).to contain_exactly(label1, label2, subgroup_label1, subgroup_label2, sub_subgroup_label1, sub_subgroup_label2) } true | nil | -> { [label1, label2, group_label1, group_label2, subgroup_label1, subgroup_label2] }
true | nil | nil | nil | -> { expect(subject).to contain_exactly(label1, label2, group_label1, group_label2, subgroup_label1, subgroup_label2) } nil | 'new' | -> { [label2, subgroup_label2] }
true | nil | true | nil | -> { expect(subject).to contain_exactly(label1, label2, group_label1, group_label2, subgroup_label1, subgroup_label2) } false | 'new' | -> { [label2, subgroup_label2] }
true | true | nil | nil | -> { expect(subject).to contain_exactly(label1, label2, group_label1, group_label2, subgroup_label1, subgroup_label2, sub_subgroup_label1, sub_subgroup_label2) } true | 'new' | -> { [label2, group_label2, subgroup_label2] }
true | true | true | nil | -> { expect(subject).to contain_exactly(label1, label2, group_label1, group_label2, subgroup_label1, subgroup_label2, sub_subgroup_label1, sub_subgroup_label2) }
nil | nil | nil | 'new' | -> { expect(subject).to contain_exactly(label2, subgroup_label2) }
nil | nil | true | 'new' | -> { expect(subject).to contain_exactly(label2, subgroup_label2) }
nil | true | nil | 'new' | -> { expect(subject).to contain_exactly(label2, subgroup_label2, sub_subgroup_label2) }
nil | true | true | 'new' | -> { expect(subject).to contain_exactly(label2, subgroup_label2, sub_subgroup_label2) }
true | nil | nil | 'new' | -> { expect(subject).to contain_exactly(label2, group_label2, subgroup_label2) }
true | nil | true | 'new' | -> { expect(subject).to contain_exactly(label2, group_label2, subgroup_label2) }
true | true | nil | 'new' | -> { expect(subject).to contain_exactly(label2, group_label2, subgroup_label2, sub_subgroup_label2) }
true | true | true | 'new' | -> { expect(subject).to contain_exactly(label2, group_label2, subgroup_label2, sub_subgroup_label2) }
end end
with_them do with_them do
let(:params) do let(:params) do
{ {
include_ancestor_groups: include_ancestor_groups, include_ancestor_groups: include_ancestor_groups,
include_descendant_groups: include_descendant_groups,
only_group_labels: only_group_labels,
search_term: search_term search_term: search_term
} }
end end
subject { resolve_labels(project, params) } subject { resolve_labels(project, params) }
it { self.instance_exec(&test) } specify { expect(subject).to match_array(instance_exec(&expected_labels)) }
end end
end end
end end
......
...@@ -69,7 +69,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do ...@@ -69,7 +69,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do
it 'batch-resolves by target project full path and IIDS', :request_store do it 'batch-resolves by target project full path and IIDS', :request_store do
result = batch_sync(max_queries: queries_per_project) do result = batch_sync(max_queries: queries_per_project) do
resolve_mr(project, iids: [iid_1, iid_2]) resolve_mr(project, iids: [iid_1, iid_2]).to_a
end end
expect(result).to contain_exactly(merge_request_1, merge_request_2) expect(result).to contain_exactly(merge_request_1, merge_request_2)
......
...@@ -14,7 +14,7 @@ RSpec.describe Resolvers::ReleaseMilestonesResolver do ...@@ -14,7 +14,7 @@ RSpec.describe Resolvers::ReleaseMilestonesResolver do
describe '#resolve' do describe '#resolve' do
it "uses offset-pagination" do it "uses offset-pagination" do
expect(resolved).to be_a(::Gitlab::Graphql::Pagination::OffsetPaginatedRelation) expect(resolved).to be_a(::Gitlab::Graphql::Pagination::OffsetActiveRecordRelationConnection)
end end
it "includes the release's milestones in the returned OffsetActiveRecordRelationConnection" do it "includes the release's milestones in the returned OffsetActiveRecordRelationConnection" do
......
...@@ -649,16 +649,8 @@ module GraphqlHelpers ...@@ -649,16 +649,8 @@ module GraphqlHelpers
end end
end end
def execute_query(query_type) def execute_query(query_type, schema = empty_schema)
schema = Class.new(GraphQL::Schema) do schema.query(query_type)
use GraphQL::Pagination::Connections
use Gitlab::Graphql::Pagination::Connections
lazy_resolve ::Gitlab::Graphql::Lazy, :force
query(query_type)
end
schema.execute( schema.execute(
query_string, query_string,
context: { current_user: user }, context: { current_user: user },
...@@ -666,6 +658,15 @@ module GraphqlHelpers ...@@ -666,6 +658,15 @@ module GraphqlHelpers
) )
end end
def empty_schema
Class.new(GraphQL::Schema) do
use GraphQL::Pagination::Connections
use Gitlab::Graphql::Pagination::Connections
lazy_resolve ::Gitlab::Graphql::Lazy, :force
end
end
# A lookahead that selects everything # A lookahead that selects everything
def positive_lookahead def positive_lookahead
double(selects?: true).tap do |selection| double(selects?: true).tap do |selection|
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment