diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2024-01-04 15:19:41 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2024-01-04 15:19:41 +0300 |
commit | ac72b79188a14a28eafe55d32641f9939cf5d9c4 (patch) | |
tree | d6f6f349fb30017a600ebdee07b832889615978e /spec/requests/api/graphql | |
parent | 8f89276d8498f45459bca67493eccd1bdf055330 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec/requests/api/graphql')
-rw-r--r-- | spec/requests/api/graphql/ci/runner_spec.rb | 158 | ||||
-rw-r--r-- | spec/requests/api/graphql/ci/runners_spec.rb | 183 |
2 files changed, 176 insertions, 165 deletions
diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index 8262640b283..1b6948d0380 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -876,107 +876,95 @@ RSpec.describe 'Query.runner(id)', :freeze_time, feature_category: :fleet_visibi end describe 'Query limits' do - def runner_query(runner) - <<~SINGLE - runner(id: "#{runner.to_global_id}") { - #{all_graphql_fields_for('CiRunner', excluded: excluded_fields)} - createdBy { - id - username - webPath - webUrl - } - groups { - nodes { - id - path - fullPath - webUrl - } - } - projects { - nodes { - id - path - fullPath - webUrl - } - } - ownerProject { - id - path - fullPath - webUrl - } + let_it_be(:user2) { another_admin } + let_it_be(:user3) { create(:user) } + let_it_be(:tag_list) { %w[n_plus_1_test some_tag] } + let_it_be(:args) do + { current_user: user, token: { personal_access_token: create(:personal_access_token, user: user) } } + end + + let_it_be(:runner1) { create(:ci_runner, tag_list: tag_list, creator: user) } + let_it_be(:runner2) do + create(:ci_runner, :group, groups: [group], tag_list: tag_list, creator: user) + end + + let_it_be(:runner3) do + create(:ci_runner, :project, projects: [project1], tag_list: tag_list, creator: user) + end + + let(:single_discrete_runners_query) do + multiple_discrete_runners_query([]) + end + + let(:runner_fragment) do + <<~QUERY + #{all_graphql_fields_for('CiRunner', excluded: excluded_fields)} + createdBy { + id + username + webPath + webUrl } - SINGLE + QUERY end - let(:active_project_runner2) { create(:ci_runner, :project) } - let(:active_group_runner2) { create(:ci_runner, :group) } + # Exclude fields that are already hardcoded above (or tested separately), + # and also some fields from deeper objects which are problematic: + # - createdBy: Known N+1 issues, but only on exotic fields which we don't normally use + # - ownerProject.pipeline: Needs arguments (iid or sha) + # - project.productAnalyticsState: Can be requested only for 1 Project(s) at a time. + let(:excluded_fields) { %w[createdBy jobs pipeline productAnalyticsState] } + + it 'avoids N+1 queries', :use_sql_query_cache do + discrete_runners_control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + post_graphql(single_discrete_runners_query, **args) + end + + additional_runners = setup_additional_records + + expect do + post_graphql(multiple_discrete_runners_query(additional_runners), **args) - # Exclude fields that are already hardcoded above - let(:excluded_fields) { %w[createdBy jobs groups projects ownerProject] } + raise StandardError, flattened_errors if graphql_errors # Ensure any error in query causes test to fail + end.not_to exceed_query_limit(discrete_runners_control) + end - let(:single_query) do + def runner_query(runner, nr) <<~QUERY - { - instance_runner1: #{runner_query(active_instance_runner)} - group_runner1: #{runner_query(active_group_runner)} - project_runner1: #{runner_query(active_project_runner)} + runner#{nr}: runner(id: "#{runner.to_global_id}") { + #{runner_fragment} } QUERY end - let(:double_query) do + def multiple_discrete_runners_query(additional_runners) <<~QUERY { - instance_runner1: #{runner_query(active_instance_runner)} - instance_runner2: #{runner_query(inactive_instance_runner)} - group_runner1: #{runner_query(active_group_runner)} - group_runner2: #{runner_query(active_group_runner2)} - project_runner1: #{runner_query(active_project_runner)} - project_runner2: #{runner_query(active_project_runner2)} + #{runner_query(runner1, 1)} + #{runner_query(runner2, 2)} + #{runner_query(runner3, 3)} + #{additional_runners.each_with_index.map { |r, i| runner_query(r, 4 + i) }.join("\n")} } QUERY end - it 'does not execute more queries per runner', :aggregate_failures, quarantine: "https://gitlab.com/gitlab-org/gitlab/-/issues/391442" do - # warm-up license cache and so on: - personal_access_token = create(:personal_access_token, user: user) - args = { current_user: user, token: { personal_access_token: personal_access_token } } - post_graphql(double_query, **args) - - control = ActiveRecord::QueryRecorder.new { post_graphql(single_query, **args) } - - personal_access_token = create(:personal_access_token, user: another_admin) - args = { current_user: another_admin, token: { personal_access_token: personal_access_token } } - expect { post_graphql(double_query, **args) }.not_to exceed_query_limit(control) - - expect(graphql_data.count).to eq 6 - expect(graphql_data).to match( - a_hash_including( - 'instance_runner1' => a_graphql_entity_for(active_instance_runner), - 'instance_runner2' => a_graphql_entity_for(inactive_instance_runner), - 'group_runner1' => a_graphql_entity_for( - active_group_runner, - groups: { 'nodes' => contain_exactly(a_graphql_entity_for(group)) } - ), - 'group_runner2' => a_graphql_entity_for( - active_group_runner2, - groups: { 'nodes' => active_group_runner2.groups.map { |g| a_graphql_entity_for(g) } } - ), - 'project_runner1' => a_graphql_entity_for( - active_project_runner, - projects: { 'nodes' => active_project_runner.projects.map { |p| a_graphql_entity_for(p) } }, - owner_project: a_graphql_entity_for(active_project_runner.projects[0]) - ), - 'project_runner2' => a_graphql_entity_for( - active_project_runner2, - projects: { 'nodes' => active_project_runner2.projects.map { |p| a_graphql_entity_for(p) } }, - owner_project: a_graphql_entity_for(active_project_runner2.projects[0]) - ) - )) + def setup_additional_records + # Add more runners (including owned by other users) + runner4 = create(:ci_runner, tag_list: tag_list + %w[tag1 tag2], creator: user2) + runner5 = create(:ci_runner, :group, groups: [create(:group)], tag_list: tag_list + %w[tag2 tag3], creator: user3) + # Add one more project to runner + runner3.assign_to(create(:project)) + + # Add more runner managers (including to existing runners) + runner_manager1 = create(:ci_runner_machine, runner: runner1) + create(:ci_runner_machine, runner: runner1) + create(:ci_runner_machine, runner: runner2, system_xid: runner_manager1.system_xid) + create(:ci_runner_machine, runner: runner3) + create(:ci_runner_machine, runner: runner4, version: '16.4.1') + create(:ci_runner_machine, runner: runner5, version: '16.4.0', system_xid: runner_manager1.system_xid) + create(:ci_runner_machine, runner: runner3) + + [runner4, runner5] end end diff --git a/spec/requests/api/graphql/ci/runners_spec.rb b/spec/requests/api/graphql/ci/runners_spec.rb index 0fe14bef778..189106fae7b 100644 --- a/spec/requests/api/graphql/ci/runners_spec.rb +++ b/spec/requests/api/graphql/ci/runners_spec.rb @@ -18,22 +18,34 @@ RSpec.describe 'Query.runners', feature_category: :fleet_visibility do let(:fields) do <<~QUERY nodes { - #{all_graphql_fields_for('CiRunner', excluded: %w[createdBy ownerProject])} - createdBy { - username - webPath - webUrl - } - ownerProject { - id - path - fullPath - webUrl - } + #{all_graphql_fields_for('CiRunner', excluded: excluded_fields)} } QUERY end + let(:query) do + %( + query { + runners { + #{fields} + } + } + ) + end + + # Exclude fields from deeper objects which are problematic: + # - ownerProject.pipeline: Needs arguments (iid or sha) + # - project.productAnalyticsState: Can be requested only for 1 Project(s) at a time. + let(:excluded_fields) { %w[pipeline productAnalyticsState] } + + it 'returns expected runners' do + post_graphql(query, current_user: current_user) + + expect(runners_graphql_data['nodes']).to contain_exactly( + *Ci::Runner.all.map { |expected_runner| a_graphql_entity_for(expected_runner) } + ) + end + context 'with filters' do shared_examples 'a working graphql query returning expected runners' do it_behaves_like 'a working graphql query' do @@ -49,31 +61,6 @@ RSpec.describe 'Query.runners', feature_category: :fleet_visibility do *Array(expected_runners).map { |expected_runner| a_graphql_entity_for(expected_runner) } ) end - - it 'does not execute more queries per runner', :aggregate_failures do - # warm-up license cache and so on: - personal_access_token = create(:personal_access_token, user: current_user) - args = { current_user: current_user, token: { personal_access_token: personal_access_token } } - post_graphql(query, **args) - expect(graphql_data_at(:runners, :nodes)).not_to be_empty - - admin2 = create(:admin) - personal_access_token = create(:personal_access_token, user: admin2) - args = { current_user: admin2, token: { personal_access_token: personal_access_token } } - control = ActiveRecord::QueryRecorder.new { post_graphql(query, **args) } - - runner2 = create(:ci_runner, :instance, version: '14.0.0', tag_list: %w[tag5 tag6], creator: admin2) - runner3 = create(:ci_runner, :project, version: '14.0.1', projects: [project], tag_list: %w[tag3 tag8], - creator: current_user) - - create(:ci_build, :failed, runner: runner2) - create(:ci_runner_machine, runner: runner2, version: '16.4.1') - - create(:ci_build, :failed, runner: runner3) - create(:ci_runner_machine, runner: runner3, version: '16.4.0') - - expect { post_graphql(query, **args) }.not_to exceed_query_limit(control) - end end context 'when filtered on type and status' do @@ -179,52 +166,88 @@ RSpec.describe 'Query.runners', feature_category: :fleet_visibility do end end end + end - context 'without filters' do - context 'with managers requested for multiple runners' do - let(:fields) do - <<~QUERY - nodes { - managers { - nodes { - #{all_graphql_fields_for('CiRunnerManager', max_depth: 1)} - } - } - } - QUERY - end + describe 'Runner query limits' do + let_it_be(:user) { create(:user, :admin) } + let_it_be(:user2) { create(:user) } + let_it_be(:user3) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project) } + let_it_be(:tag_list) { %w[n_plus_1_test some_tag] } + let_it_be(:args) do + { current_user: user, token: { personal_access_token: create(:personal_access_token, user: user) } } + end - let(:query) do - %( - query { - runners { - #{fields} - } - } - ) - end + let_it_be(:runner1) { create(:ci_runner, tag_list: tag_list, creator: user) } + let_it_be(:runner2) do + create(:ci_runner, :group, groups: [group], tag_list: tag_list, creator: user) + end - it 'does not execute more queries per runner', :aggregate_failures do - # warm-up license cache and so on: - personal_access_token = create(:personal_access_token, user: current_user) - args = { current_user: current_user, token: { personal_access_token: personal_access_token } } - post_graphql(query, **args) - expect(graphql_data_at(:runners, :nodes)).not_to be_empty - - admin2 = create(:admin) - personal_access_token = create(:personal_access_token, user: admin2) - args = { current_user: admin2, token: { personal_access_token: personal_access_token } } - control = ActiveRecord::QueryRecorder.new { post_graphql(query, **args) } - - create(:ci_runner, :instance, :with_runner_manager, version: '14.0.0', tag_list: %w[tag5 tag6], - creator: admin2) - create(:ci_runner, :project, :with_runner_manager, version: '14.0.1', projects: [project], - tag_list: %w[tag3 tag8], - creator: current_user) - - expect { post_graphql(query, **args) }.not_to exceed_query_limit(control) - end - end + let_it_be(:runner3) do + create(:ci_runner, :project, projects: [project], tag_list: tag_list, creator: user) + end + + let(:runner_fragment) do + <<~QUERY + #{all_graphql_fields_for('CiRunner', excluded: excluded_fields)} + createdBy { + id + username + webPath + webUrl + } + QUERY + end + + # Exclude fields that are already hardcoded above (or tested separately), + # and also some fields from deeper objects which are problematic: + # - createdBy: Known N+1 issues, but only on exotic fields which we don't normally use + # - ownerProject.pipeline: Needs arguments (iid or sha) + # - project.productAnalyticsState: Can be requested only for 1 Project(s) at a time. + let(:excluded_fields) { %w[createdBy jobs pipeline productAnalyticsState] } + + let(:runners_query) do + <<~QUERY + { + runners { + nodes { #{runner_fragment} } + } + } + QUERY + end + + it 'avoids N+1 queries', :use_sql_query_cache do + personal_access_token = create(:personal_access_token, user: user) + args = { current_user: user, token: { personal_access_token: personal_access_token } } + + runners_control = ActiveRecord::QueryRecorder.new(skip_cached: false) { post_graphql(runners_query, **args) } + + setup_additional_records + + expect { post_graphql(runners_query, **args) }.not_to exceed_query_limit(runners_control) + end + + def setup_additional_records + # Add more runners (including owned by other users) + runner4 = create(:ci_runner, tag_list: tag_list + %w[tag1 tag2], creator: user2) + runner5 = create(:ci_runner, :group, groups: [create(:group)], tag_list: tag_list + %w[tag2 tag3], creator: user3) + # Add one more project to runner + runner3.assign_to(create(:project)) + + # Add more runner managers (including to existing runners) + runner_manager1 = create(:ci_runner_machine, runner: runner1) + create(:ci_runner_machine, runner: runner1) + create(:ci_runner_machine, runner: runner2, system_xid: runner_manager1.system_xid) + create(:ci_runner_machine, runner: runner3) + create(:ci_runner_machine, runner: runner4, version: '16.4.1') + create(:ci_runner_machine, runner: runner5, version: '16.4.0', system_xid: runner_manager1.system_xid) + create(:ci_runner_machine, runner: runner3) + + create(:ci_build, :failed, runner: runner4) + create(:ci_build, :failed, runner: runner5) + + [runner4, runner5] end end |