From 9f9bf2139d70ea19bca1160bbf8a32901c4f2154 Mon Sep 17 00:00:00 2001 From: Payton Burdette Date: Thu, 26 Jun 2025 17:20:36 -0400 Subject: [PATCH 01/22] Return bridges and builds for project jobs Return all job types builds or bridges for the project jobs feature. Changelog: changed --- app/finders/ci/jobs_finder.rb | 14 +++++++++++--- app/graphql/resolvers/project_jobs_resolver.rb | 14 +++++++------- app/models/ci/runner.rb | 1 + 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/app/finders/ci/jobs_finder.rb b/app/finders/ci/jobs_finder.rb index c46311a40c06a1..7f4611c606d5ef 100644 --- a/app/finders/ci/jobs_finder.rb +++ b/app/finders/ci/jobs_finder.rb @@ -11,7 +11,7 @@ def initialize(current_user:, pipeline: nil, project: nil, runner: nil, params: @runner = runner @params = params @type = type - raise ArgumentError 'type must be a subclass of Ci::Processable' unless type < ::Ci::Processable + raise ArgumentError 'type must be Ci::Processable or subclass' unless type <= ::Ci::Processable end def execute @@ -105,9 +105,9 @@ def use_runner_type_filter? end def filter_by_with_artifacts(builds) - return builds.with_any_artifacts if params[:with_artifacts] + return builds unless params[:with_artifacts] - builds + builds.with_any_artifacts end def filter_by_statuses!(builds) @@ -123,6 +123,14 @@ def jobs_by_type(relation, type) relation.builds when ::Ci::Bridge.name relation.bridges + when ::Ci::Processable.name + if params[:with_artifacts] + # When filtering by artifacts, only return builds since bridges don't have artifacts + relation.builds + else + # Return both builds and bridges when not filtering by artifacts + relation.processables + end else raise ArgumentError, "finder does not support #{type} type" end diff --git a/app/graphql/resolvers/project_jobs_resolver.rb b/app/graphql/resolvers/project_jobs_resolver.rb index adc4fbac3b9231..3faa38da8ac929 100644 --- a/app/graphql/resolvers/project_jobs_resolver.rb +++ b/app/graphql/resolvers/project_jobs_resolver.rb @@ -33,13 +33,14 @@ class ProjectJobsResolver < BaseResolver def resolve_with_lookahead(**args) filter_by_name = Feature.enabled?(:populate_and_use_build_names_table, project) && args[:name].to_s.present? filter_by_sources = args[:sources].present? + job_params = { + scope: args[:statuses], with_artifacts: args[:with_artifacts], + skip_ordering: filter_by_sources + } - jobs = ::Ci::JobsFinder.new( - current_user: current_user, project: project, params: { - scope: args[:statuses], with_artifacts: args[:with_artifacts], - skip_ordering: filter_by_sources - } - ).execute + jobs = ::Ci::JobsFinder + .new(current_user: current_user, project: project, params: job_params, type: ::Ci::Processable) + .execute # These job filters are currently exclusive with each other if filter_by_name @@ -66,7 +67,6 @@ def resolve_with_lookahead(**args) def preloads { previous_stage_jobs_or_needs: [:needs, :pipeline], - artifacts: [:job_artifacts], pipeline: [:user], build_source: [:source] } diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index d7e2095acc9e27..143b30c89baf0a 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -96,6 +96,7 @@ class Runner < Ci::ApplicationRecord has_many :runner_managers, inverse_of: :runner has_many :builds has_many :running_builds, inverse_of: :runner + has_many :processables, class_name: 'Ci::Processable' has_many :runner_projects, inverse_of: :runner, autosave: true, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :projects, through: :runner_projects, disable_joins: true has_many :runner_namespaces, inverse_of: :runner, autosave: true -- GitLab From dd3056e452eb8c6d3a48ffa63e26a9e2f080506e Mon Sep 17 00:00:00 2001 From: Payton Burdette Date: Fri, 27 Jun 2025 10:24:35 -0400 Subject: [PATCH 02/22] Only filter builds not bridges --- app/graphql/resolvers/project_jobs_resolver.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/graphql/resolvers/project_jobs_resolver.rb b/app/graphql/resolvers/project_jobs_resolver.rb index 3faa38da8ac929..4a120d47c3db8f 100644 --- a/app/graphql/resolvers/project_jobs_resolver.rb +++ b/app/graphql/resolvers/project_jobs_resolver.rb @@ -38,8 +38,11 @@ def resolve_with_lookahead(**args) skip_ordering: filter_by_sources } + # Determine the correct job type based on filtering needs + job_type = filter_by_name || filter_by_sources ? ::Ci::Build : ::Ci::Processable + jobs = ::Ci::JobsFinder - .new(current_user: current_user, project: project, params: job_params, type: ::Ci::Processable) + .new(current_user: current_user, project: project, params: job_params, type: job_type) .execute # These job filters are currently exclusive with each other -- GitLab From 4bd2a2790d0f55482a967a2cf20b1a44f5f52f10 Mon Sep 17 00:00:00 2001 From: Payton Burdette Date: Fri, 27 Jun 2025 12:08:30 -0400 Subject: [PATCH 03/22] Add resolver test coverage --- .../resolvers/project_jobs_resolver_spec.rb | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/spec/graphql/resolvers/project_jobs_resolver_spec.rb b/spec/graphql/resolvers/project_jobs_resolver_spec.rb index f1f24be4129593..5fd27fef7624ba 100644 --- a/spec/graphql/resolvers/project_jobs_resolver_spec.rb +++ b/spec/graphql/resolvers/project_jobs_resolver_spec.rb @@ -85,6 +85,13 @@ expect(parsed_response).to match_array([a_graphql_entity_for(pending_build)]) end end + + context 'with bridge' do + let_it_be(:bridge_job) { create(:ci_bridge, :success, name: 'Bridge Job', pipeline: pipeline) } + + it { is_expected.to include(successful_build, pending_build) } + it { is_expected.not_to include(bridge_job) } + end end context 'when filtering by build name' do @@ -131,6 +138,48 @@ end end end + + context 'with bridge' do + let_it_be(:bridge_with_same_name) { create(:ci_bridge, :success, name: 'Build Three', pipeline: pipeline) } + + it { is_expected.to include(failed_build, pending_build) } + it { is_expected.not_to include(bridge_with_same_name) } + end + end + + context 'when filtering by artifacts' do + let_it_be(:build_with_artifacts) { create(:ci_build, :success, :artifacts, name: 'Build With Artifacts', pipeline: pipeline) } + let_it_be(:build_without_artifacts) { create(:ci_build, :success, name: 'Build Without Artifacts', pipeline: pipeline) } + let_it_be(:bridge_job) { create(:ci_bridge, :success, name: 'Bridge Job', pipeline: pipeline) } + + context 'with with_artifacts: true' do + let(:args) { { with_artifacts: true } } + + it { is_expected.to include(build_with_artifacts) } + it { is_expected.not_to include(build_without_artifacts, bridge_job) } + end + + context 'with with_artifacts: false' do + let(:args) { { with_artifacts: false } } + + it { is_expected.to include(build_with_artifacts, build_without_artifacts, bridge_job) } + end + end + + context 'when returning both builds and bridges' do + let_it_be(:successful_bridge) { create(:ci_bridge, :success, name: 'Bridge Job', pipeline: pipeline) } + let_it_be(:failed_bridge) { create(:ci_bridge, :failed, name: 'Failed Bridge', pipeline: pipeline) } + + context 'without any filters' do + it { is_expected.to include(successful_build, successful_build_two, failed_build, pending_build, successful_bridge, failed_bridge) } + end + + context 'with status filter' do + let(:args) { { statuses: [Types::Ci::JobStatusEnum.coerce_isolated_input('SUCCESS')] } } + + it { is_expected.to include(successful_build, successful_build_two, successful_bridge) } + it { is_expected.not_to include(failed_build, pending_build, failed_bridge) } + end end end -- GitLab From 46605f58b693f13258a281a87ac0036423e7ce58 Mon Sep 17 00:00:00 2001 From: Payton Burdette Date: Fri, 27 Jun 2025 14:37:03 -0400 Subject: [PATCH 04/22] Add finder test coverage --- spec/finders/ci/jobs_finder_spec.rb | 46 +++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/spec/finders/ci/jobs_finder_spec.rb b/spec/finders/ci/jobs_finder_spec.rb index c69d32f679d1cd..15ba998694653c 100644 --- a/spec/finders/ci/jobs_finder_spec.rb +++ b/spec/finders/ci/jobs_finder_spec.rb @@ -464,4 +464,50 @@ end end end + + context 'when type is Ci::Processable' do + let_it_be(:bridge_job) { create(:ci_bridge, :success, pipeline: pipeline, name: 'bridge') } + let_it_be(:build_job_with_artifacts) do + create(:ci_build, :success, pipeline: pipeline, name: 'build_with_artifacts') + end + + let_it_be(:artifact) { create(:ci_job_artifact, job: build_job_with_artifacts) } + + subject do + described_class.new(current_user: user, project: project, params: params, type: ::Ci::Processable).execute + end + + context 'with user being project maintainer' do + before do + project.add_maintainer(user) + end + + context 'when with_artifacts is true' do + let(:params) { { with_artifacts: true } } + + it 'returns only builds with artifacts, not bridges' do + expect(subject).to match_array([build_job_with_artifacts]) + expect(subject).not_to include(bridge_job) + end + end + + context 'when with_artifacts is false' do + let(:params) { { with_artifacts: false } } + + it 'returns both builds and bridges' do + expect(subject).to include(bridge_job, build_job_with_artifacts) + end + end + end + + context 'with user being project guest' do + before do + project.add_guest(user) + end + + it 'returns no jobs' do + expect(subject).to be_empty + end + end + end end -- GitLab From de65a0d10883878cb18c9a0bbb38480ce6de0488 Mon Sep 17 00:00:00 2001 From: Payton Burdette Date: Fri, 27 Jun 2025 14:39:31 -0400 Subject: [PATCH 05/22] Add model test coverage --- spec/models/ci/runner_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 13bbdbfb81c2e7..91e25b46119b42 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -18,6 +18,7 @@ it { is_expected.to have_many(:builds) } it { is_expected.to have_one(:last_build).class_name('Ci::Build') } it { is_expected.to have_many(:running_builds).inverse_of(:runner) } + it { is_expected.to have_many(:processables).class_name('Ci::Processable') } it { is_expected.to have_many(:runner_projects).inverse_of(:runner) } it { is_expected.to have_many(:projects).through(:runner_projects) } -- GitLab From eed2eb276bc1e4291999f79c64a93789975f4f76 Mon Sep 17 00:00:00 2001 From: Payton Burdette Date: Mon, 30 Jun 2025 09:23:26 -0400 Subject: [PATCH 06/22] Reduce jobs_by_type complexitiy --- app/finders/ci/jobs_finder.rb | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/app/finders/ci/jobs_finder.rb b/app/finders/ci/jobs_finder.rb index 7f4611c606d5ef..cc79adeb872296 100644 --- a/app/finders/ci/jobs_finder.rb +++ b/app/finders/ci/jobs_finder.rb @@ -124,17 +124,21 @@ def jobs_by_type(relation, type) when ::Ci::Bridge.name relation.bridges when ::Ci::Processable.name - if params[:with_artifacts] - # When filtering by artifacts, only return builds since bridges don't have artifacts - relation.builds - else - # Return both builds and bridges when not filtering by artifacts - relation.processables - end + processables_with_artifact_filter(relation) else raise ArgumentError, "finder does not support #{type} type" end end + + def processables_with_artifact_filter(relation) + if params[:with_artifacts] + # When filtering by artifacts, only return builds since bridges don't have artifacts + relation.builds + else + # Return both builds and bridges when not filtering by artifacts + relation.processables + end + end end end -- GitLab From 51faa1c5649dc5fc7935c537f1bfd925f612edef Mon Sep 17 00:00:00 2001 From: Payton Burdette Date: Mon, 30 Jun 2025 11:35:31 -0400 Subject: [PATCH 07/22] Add bridge job badge --- app/assets/javascripts/ci/constants.js | 2 ++ .../ci/jobs_page/components/job_cells/job_cell.vue | 8 +++++++- .../ci/jobs_page/graphql/queries/get_jobs.query.graphql | 1 + .../ci/jobs_page/components/job_cells/job_cell_spec.js | 2 ++ 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/ci/constants.js b/app/assets/javascripts/ci/constants.js index 07bf90910f6e67..9d3131d594a051 100644 --- a/app/assets/javascripts/ci/constants.js +++ b/app/assets/javascripts/ci/constants.js @@ -21,6 +21,8 @@ export const ICONS = { SUCCESS: 'success', }; +export const BRIDGE_KIND = 'BRIDGE'; + export const FAILED_STATUS = 'failed'; export const PASSED_STATUS = 'passed'; export const MANUAL_STATUS = 'manual'; diff --git a/app/assets/javascripts/ci/jobs_page/components/job_cells/job_cell.vue b/app/assets/javascripts/ci/jobs_page/components/job_cells/job_cell.vue index af7b011ba06eed..53591aaf692e99 100644 --- a/app/assets/javascripts/ci/jobs_page/components/job_cells/job_cell.vue +++ b/app/assets/javascripts/ci/jobs_page/components/job_cells/job_cell.vue @@ -3,7 +3,7 @@ import { GlBadge, GlIcon, GlLink, GlTooltipDirective } from '@gitlab/ui'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { s__ } from '~/locale'; import LinkCell from '~/ci/runner/components/cells/link_cell.vue'; -import { SUCCESS_STATUS } from '../../../constants'; +import { SUCCESS_STATUS, BRIDGE_KIND } from '../../../constants'; export default { iconSize: 12, @@ -72,6 +72,9 @@ export default { jobStuck() { return this.job?.stuck; }, + isBridgeJob() { + return this.job?.kind === BRIDGE_KIND; + }, }, }; @@ -150,6 +153,9 @@ export default { {{ s__('Job|manual') }} + + {{ s__('Job|bridge') }} + diff --git a/app/assets/javascripts/ci/jobs_page/graphql/queries/get_jobs.query.graphql b/app/assets/javascripts/ci/jobs_page/graphql/queries/get_jobs.query.graphql index 36cf33a712509d..486298c6fc84ff 100644 --- a/app/assets/javascripts/ci/jobs_page/graphql/queries/get_jobs.query.graphql +++ b/app/assets/javascripts/ci/jobs_page/graphql/queries/get_jobs.query.graphql @@ -91,6 +91,7 @@ query getJobs( updateBuild cancelBuild } + kind } } } diff --git a/spec/frontend/ci/jobs_page/components/job_cells/job_cell_spec.js b/spec/frontend/ci/jobs_page/components/job_cells/job_cell_spec.js index 9665e25e2ace44..992e6c8a58275e 100644 --- a/spec/frontend/ci/jobs_page/components/job_cells/job_cell_spec.js +++ b/spec/frontend/ci/jobs_page/components/job_cells/job_cell_spec.js @@ -136,12 +136,14 @@ describe('Job Cell', () => { ${'trigger-token-job-badge'} | ${'trigger token'} ${'fail-job-badge'} | ${'allowed to fail'} ${'delayed-job-badge'} | ${'delayed'} + ${'bridge-job-badge'} | ${'bridge'} `('displays the static $text badge', ({ testId, text }) => { createComponent({ manualJob: true, triggered: true, allowFailure: true, scheduledAt: '2021-03-09T14:58:50+00:00', + kind: 'BRIDGE', }); expect(findBadgeById(testId).exists()).toBe(true); -- GitLab From 4e079dfb9040d5d2ef0ba002e362214433b1194b Mon Sep 17 00:00:00 2001 From: Payton Burdette Date: Mon, 30 Jun 2025 11:36:37 -0400 Subject: [PATCH 08/22] Regenerate gitlab.pot file --- locale/gitlab.pot | 3 +++ 1 file changed, 3 insertions(+) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 22f14a680154f9..9cd375f43f1375 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -35630,6 +35630,9 @@ msgstr "" msgid "Job|allowed to fail" msgstr "" +msgid "Job|bridge" +msgstr "" + msgid "Job|delayed" msgstr "" -- GitLab From aa66be91fa57b1bfc159890ffc4daa841f544728 Mon Sep 17 00:00:00 2001 From: Payton Burdette Date: Tue, 1 Jul 2025 10:16:14 -0400 Subject: [PATCH 09/22] Update badge text --- .../ci/jobs_page/components/job_cells/job_cell.vue | 6 +++--- locale/gitlab.pot | 6 +++--- .../ci/jobs_page/components/job_cells/job_cell_spec.js | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/ci/jobs_page/components/job_cells/job_cell.vue b/app/assets/javascripts/ci/jobs_page/components/job_cells/job_cell.vue index 53591aaf692e99..ec66c2052e0da3 100644 --- a/app/assets/javascripts/ci/jobs_page/components/job_cells/job_cell.vue +++ b/app/assets/javascripts/ci/jobs_page/components/job_cells/job_cell.vue @@ -72,7 +72,7 @@ export default { jobStuck() { return this.job?.stuck; }, - isBridgeJob() { + isTriggerJob() { return this.job?.kind === BRIDGE_KIND; }, }, @@ -153,8 +153,8 @@ export default { {{ s__('Job|manual') }} - - {{ s__('Job|bridge') }} + + {{ s__('Job|trigger job') }} diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9cd375f43f1375..0d92c257291721 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -35630,15 +35630,15 @@ msgstr "" msgid "Job|allowed to fail" msgstr "" -msgid "Job|bridge" -msgstr "" - msgid "Job|delayed" msgstr "" msgid "Job|manual" msgstr "" +msgid "Job|trigger job" +msgstr "" + msgid "Job|trigger token" msgstr "" diff --git a/spec/frontend/ci/jobs_page/components/job_cells/job_cell_spec.js b/spec/frontend/ci/jobs_page/components/job_cells/job_cell_spec.js index 992e6c8a58275e..507e4f78313491 100644 --- a/spec/frontend/ci/jobs_page/components/job_cells/job_cell_spec.js +++ b/spec/frontend/ci/jobs_page/components/job_cells/job_cell_spec.js @@ -136,7 +136,7 @@ describe('Job Cell', () => { ${'trigger-token-job-badge'} | ${'trigger token'} ${'fail-job-badge'} | ${'allowed to fail'} ${'delayed-job-badge'} | ${'delayed'} - ${'bridge-job-badge'} | ${'bridge'} + ${'trigger-job-badge'} | ${'trigger job'} `('displays the static $text badge', ({ testId, text }) => { createComponent({ manualJob: true, -- GitLab From b9ac20cb17eb436e02f8e3ffa41aed93fe7a4174 Mon Sep 17 00:00:00 2001 From: Payton Burdette Date: Thu, 3 Jul 2025 10:44:40 -0400 Subject: [PATCH 10/22] Address reviewer feedback --- app/finders/ci/jobs_finder.rb | 5 +++-- app/graphql/resolvers/project_jobs_resolver.rb | 11 ++++++++++- app/models/ci/runner.rb | 1 - spec/models/ci/runner_spec.rb | 1 - 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/app/finders/ci/jobs_finder.rb b/app/finders/ci/jobs_finder.rb index cc79adeb872296..61de0d8f988934 100644 --- a/app/finders/ci/jobs_finder.rb +++ b/app/finders/ci/jobs_finder.rb @@ -11,7 +11,9 @@ def initialize(current_user:, pipeline: nil, project: nil, runner: nil, params: @runner = runner @params = params @type = type + raise ArgumentError 'type must be Ci::Processable or subclass' unless type <= ::Ci::Processable + raise ArgumentError, 'runner parameter can only be used with Ci::Build type' if runner && type != ::Ci::Build end def execute @@ -130,12 +132,11 @@ def jobs_by_type(relation, type) end end + # When filtering by artifacts, only builds have artifacts def processables_with_artifact_filter(relation) if params[:with_artifacts] - # When filtering by artifacts, only return builds since bridges don't have artifacts relation.builds else - # Return both builds and bridges when not filtering by artifacts relation.processables end end diff --git a/app/graphql/resolvers/project_jobs_resolver.rb b/app/graphql/resolvers/project_jobs_resolver.rb index 4a120d47c3db8f..af3069fa312e83 100644 --- a/app/graphql/resolvers/project_jobs_resolver.rb +++ b/app/graphql/resolvers/project_jobs_resolver.rb @@ -68,11 +68,20 @@ def resolve_with_lookahead(**args) private def preloads - { + base_preloads = { previous_stage_jobs_or_needs: [:needs, :pipeline], pipeline: [:user], build_source: [:source] } + + # Only include artifacts preload when we're using builds + base_preloads[:artifacts] = [:job_artifacts] if should_preload_artifacts? + + base_preloads + end + + def should_preload_artifacts? + @job_type == ::Ci::Build end end end diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 143b30c89baf0a..d7e2095acc9e27 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -96,7 +96,6 @@ class Runner < Ci::ApplicationRecord has_many :runner_managers, inverse_of: :runner has_many :builds has_many :running_builds, inverse_of: :runner - has_many :processables, class_name: 'Ci::Processable' has_many :runner_projects, inverse_of: :runner, autosave: true, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :projects, through: :runner_projects, disable_joins: true has_many :runner_namespaces, inverse_of: :runner, autosave: true diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 91e25b46119b42..13bbdbfb81c2e7 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -18,7 +18,6 @@ it { is_expected.to have_many(:builds) } it { is_expected.to have_one(:last_build).class_name('Ci::Build') } it { is_expected.to have_many(:running_builds).inverse_of(:runner) } - it { is_expected.to have_many(:processables).class_name('Ci::Processable') } it { is_expected.to have_many(:runner_projects).inverse_of(:runner) } it { is_expected.to have_many(:projects).through(:runner_projects) } -- GitLab From 26c5dddb05c3250263523ddb1d0f295bf8193648 Mon Sep 17 00:00:00 2001 From: Payton Burdette Date: Thu, 3 Jul 2025 15:19:00 -0400 Subject: [PATCH 11/22] Fix preloads with_artifacts param --- app/graphql/resolvers/project_jobs_resolver.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/graphql/resolvers/project_jobs_resolver.rb b/app/graphql/resolvers/project_jobs_resolver.rb index af3069fa312e83..f042c3503bc22b 100644 --- a/app/graphql/resolvers/project_jobs_resolver.rb +++ b/app/graphql/resolvers/project_jobs_resolver.rb @@ -31,6 +31,8 @@ class ProjectJobsResolver < BaseResolver alias_method :project, :object def resolve_with_lookahead(**args) + @resolver_args = args + filter_by_name = Feature.enabled?(:populate_and_use_build_names_table, project) && args[:name].to_s.present? filter_by_sources = args[:sources].present? job_params = { @@ -81,7 +83,7 @@ def preloads end def should_preload_artifacts? - @job_type == ::Ci::Build + @resolver_args&.dig(:with_artifacts) || @job_type == ::Ci::Build end end end -- GitLab From 74c4b5efd2dbd310bb2c565313dec58c61f242ba Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Mon, 7 Jul 2025 12:06:50 +0200 Subject: [PATCH 12/22] Add ff return_bridges_and_builds --- app/finders/ci/jobs_finder.rb | 27 +++++++++- .../resolvers/project_jobs_resolver.rb | 51 +++++++++++++++---- .../return_bridges_and_builds.yml | 10 ++++ 3 files changed, 75 insertions(+), 13 deletions(-) create mode 100644 config/feature_flags/gitlab_com_derisk/return_bridges_and_builds.yml diff --git a/app/finders/ci/jobs_finder.rb b/app/finders/ci/jobs_finder.rb index 61de0d8f988934..e8da381e81e6c2 100644 --- a/app/finders/ci/jobs_finder.rb +++ b/app/finders/ci/jobs_finder.rb @@ -12,8 +12,12 @@ def initialize(current_user:, pipeline: nil, project: nil, runner: nil, params: @params = params @type = type - raise ArgumentError 'type must be Ci::Processable or subclass' unless type <= ::Ci::Processable - raise ArgumentError, 'runner parameter can only be used with Ci::Build type' if runner && type != ::Ci::Build + if Feature.enabled?(:return_bridges_and_builds, project) + raise ArgumentError 'type must be Ci::Processable or subclass' unless type <= ::Ci::Processable + raise ArgumentError, 'runner parameter can only be used with Ci::Build type' if runner && type != ::Ci::Build + else + raise ArgumentError 'type must be a subclass of Ci::Processable' unless type < ::Ci::Processable + end end def execute @@ -120,6 +124,14 @@ def filter_by_statuses!(builds) end def jobs_by_type(relation, type) + if Feature.enabled?(:return_bridges_and_builds, project) + jobs_by_type_with_handling_processables(relation, type) + else + jobs_by_type_without_handling_processables(relation, type) + end + end + + def jobs_by_type_with_handling_processables(relation, type) case type.name when ::Ci::Build.name relation.builds @@ -132,6 +144,17 @@ def jobs_by_type(relation, type) end end + def jobs_by_type_without_handling_processables(relation, type) + case type.name + when ::Ci::Build.name + relation.builds + when ::Ci::Bridge.name + relation.bridges + else + raise ArgumentError, "finder does not support #{type} type" + end + end + # When filtering by artifacts, only builds have artifacts def processables_with_artifact_filter(relation) if params[:with_artifacts] diff --git a/app/graphql/resolvers/project_jobs_resolver.rb b/app/graphql/resolvers/project_jobs_resolver.rb index f042c3503bc22b..fed629435e7773 100644 --- a/app/graphql/resolvers/project_jobs_resolver.rb +++ b/app/graphql/resolvers/project_jobs_resolver.rb @@ -35,18 +35,27 @@ def resolve_with_lookahead(**args) filter_by_name = Feature.enabled?(:populate_and_use_build_names_table, project) && args[:name].to_s.present? filter_by_sources = args[:sources].present? - job_params = { - scope: args[:statuses], with_artifacts: args[:with_artifacts], - skip_ordering: filter_by_sources - } - - # Determine the correct job type based on filtering needs - job_type = filter_by_name || filter_by_sources ? ::Ci::Build : ::Ci::Processable - - jobs = ::Ci::JobsFinder - .new(current_user: current_user, project: project, params: job_params, type: job_type) - .execute + jobs = if Feature.enabled?(:return_bridges_and_builds, project) + job_params = { + scope: args[:statuses], with_artifacts: args[:with_artifacts], + skip_ordering: filter_by_sources + } + + # Determine the correct job type based on filtering needs + job_type = filter_by_name || filter_by_sources ? ::Ci::Build : ::Ci::Processable + + ::Ci::JobsFinder + .new(current_user: current_user, project: project, params: job_params, type: job_type) + .execute + else + ::Ci::JobsFinder.new( + current_user: current_user, project: project, params: { + scope: args[:statuses], with_artifacts: args[:with_artifacts], + skip_ordering: filter_by_sources + } + ).execute + end # These job filters are currently exclusive with each other if filter_by_name jobs = ::Ci::BuildNameFinder.new( @@ -69,7 +78,17 @@ def resolve_with_lookahead(**args) private + # Will be removed with the removal of ff return_bridges_and_builds def preloads + if Feature.enabled?(:return_bridges_and_builds, project) + preloads_new + else + preloads_old + end + end + + # Will be changed to 'preloads' with the removal of ff return_bridges_and_builds + def preloads_new base_preloads = { previous_stage_jobs_or_needs: [:needs, :pipeline], pipeline: [:user], @@ -82,6 +101,16 @@ def preloads base_preloads end + # Will be removed with the removal of ff return_bridges_and_builds + def preloads_old + { + previous_stage_jobs_or_needs: [:needs, :pipeline], + artifacts: [:job_artifacts], + pipeline: [:user], + build_source: [:source] + } + end + def should_preload_artifacts? @resolver_args&.dig(:with_artifacts) || @job_type == ::Ci::Build end diff --git a/config/feature_flags/gitlab_com_derisk/return_bridges_and_builds.yml b/config/feature_flags/gitlab_com_derisk/return_bridges_and_builds.yml new file mode 100644 index 00000000000000..2d47bf63895bed --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/return_bridges_and_builds.yml @@ -0,0 +1,10 @@ +--- +name: return_bridges_and_builds +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/379141 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/195869 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/553943 +milestone: '18.2' +group: group::pipeline execution +type: gitlab_com_derisk +default_enabled: false -- GitLab From 9d23ca3858415ecb4cbc3b30bf4a81ef0d7456be Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Mon, 7 Jul 2025 12:13:26 +0200 Subject: [PATCH 13/22] Add guard clause for not always defined methods --- app/finders/ci/jobs_finder.rb | 1 + ee/app/finders/ee/ci/jobs_finder.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/app/finders/ci/jobs_finder.rb b/app/finders/ci/jobs_finder.rb index e8da381e81e6c2..47ae3fb96b0e36 100644 --- a/app/finders/ci/jobs_finder.rb +++ b/app/finders/ci/jobs_finder.rb @@ -101,6 +101,7 @@ def filter_by_scope(builds) def filter_by_runner_types(builds) return builds unless use_runner_type_filter? + return builds unless builds.respond_to?(:with_runner_type) builds.with_runner_type(params[:runner_type]) end diff --git a/ee/app/finders/ee/ci/jobs_finder.rb b/ee/app/finders/ee/ci/jobs_finder.rb index 377bdc82adee10..170627f67023cb 100644 --- a/ee/app/finders/ee/ci/jobs_finder.rb +++ b/ee/app/finders/ee/ci/jobs_finder.rb @@ -23,6 +23,7 @@ def use_runner_type_filter? def filter_by_failure_reason(builds) return builds unless use_failure_reason_filter? + return builds unless builds.respond_to?(:recently_failed_on_instance_runner) builds.recently_failed_on_instance_runner(params[:failure_reason]) # currently limited to instance runners end -- GitLab From 8499d8a9961c5cf351a774c7fdd98a54343ad99b Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Mon, 7 Jul 2025 16:09:01 +0200 Subject: [PATCH 14/22] Add tests for disabled feature flag --- app/finders/ci/jobs_finder.rb | 4 +-- .../resolvers/project_jobs_resolver.rb | 20 ------------- ee/spec/finders/ee/ci/jobs_finder_spec.rb | 29 +++++++++++++++++++ spec/finders/ci/jobs_finder_spec.rb | 18 ++++++++++-- .../resolvers/project_jobs_resolver_spec.rb | 17 +++++++++++ 5 files changed, 64 insertions(+), 24 deletions(-) diff --git a/app/finders/ci/jobs_finder.rb b/app/finders/ci/jobs_finder.rb index 47ae3fb96b0e36..5dd054dfd1d5a8 100644 --- a/app/finders/ci/jobs_finder.rb +++ b/app/finders/ci/jobs_finder.rb @@ -13,10 +13,10 @@ def initialize(current_user:, pipeline: nil, project: nil, runner: nil, params: @type = type if Feature.enabled?(:return_bridges_and_builds, project) - raise ArgumentError 'type must be Ci::Processable or subclass' unless type <= ::Ci::Processable + raise ArgumentError, 'type must be Ci::Processable or subclass' unless type <= ::Ci::Processable raise ArgumentError, 'runner parameter can only be used with Ci::Build type' if runner && type != ::Ci::Build else - raise ArgumentError 'type must be a subclass of Ci::Processable' unless type < ::Ci::Processable + raise ArgumentError, 'type must be a subclass of Ci::Processable' unless type < ::Ci::Processable end end diff --git a/app/graphql/resolvers/project_jobs_resolver.rb b/app/graphql/resolvers/project_jobs_resolver.rb index fed629435e7773..b3d4f62dd5df88 100644 --- a/app/graphql/resolvers/project_jobs_resolver.rb +++ b/app/graphql/resolvers/project_jobs_resolver.rb @@ -78,17 +78,7 @@ def resolve_with_lookahead(**args) private - # Will be removed with the removal of ff return_bridges_and_builds def preloads - if Feature.enabled?(:return_bridges_and_builds, project) - preloads_new - else - preloads_old - end - end - - # Will be changed to 'preloads' with the removal of ff return_bridges_and_builds - def preloads_new base_preloads = { previous_stage_jobs_or_needs: [:needs, :pipeline], pipeline: [:user], @@ -101,16 +91,6 @@ def preloads_new base_preloads end - # Will be removed with the removal of ff return_bridges_and_builds - def preloads_old - { - previous_stage_jobs_or_needs: [:needs, :pipeline], - artifacts: [:job_artifacts], - pipeline: [:user], - build_source: [:source] - } - end - def should_preload_artifacts? @resolver_args&.dig(:with_artifacts) || @job_type == ::Ci::Build end diff --git a/ee/spec/finders/ee/ci/jobs_finder_spec.rb b/ee/spec/finders/ee/ci/jobs_finder_spec.rb index f538f821b8fb3c..a1aa42cb78a5ab 100644 --- a/ee/spec/finders/ee/ci/jobs_finder_spec.rb +++ b/ee/spec/finders/ee/ci/jobs_finder_spec.rb @@ -130,6 +130,35 @@ end end end + + context 'when feature flag :return_bridges_and_builds is disabled' do + before do + stub_feature_flags(return_bridges_and_builds: false) + end + + context 'when build type is invalid' do + let(:type) { String } + let(:runner_performance_insights) { true } + + subject(:finder_execute) { described_class.new(current_user: current_user, type: type).execute } + + it 'raises an ArgumentError for invalid type' do + expect { finder_execute }.to raise_error(ArgumentError, /type must be a subclass of Ci::Processable/) + end + end + + context 'when build type is valid' do + let(:runner_performance_insights) { true } + + subject(:finder_execute) do + described_class.new(current_user: current_user, runner: instance_runner).execute + end + + it 'raises an ArgumentError for invalid type' do + expect(finder_execute.ids).not_to be_empty + end + end + end end end diff --git a/spec/finders/ci/jobs_finder_spec.rb b/spec/finders/ci/jobs_finder_spec.rb index 15ba998694653c..a1333f40771102 100644 --- a/spec/finders/ci/jobs_finder_spec.rb +++ b/spec/finders/ci/jobs_finder_spec.rb @@ -178,8 +178,22 @@ project.add_maintainer(user) end - it 'returns jobs for the specified project' do - expect(subject).to match_array([successful_job]) + context 'when type is invalid' do + let(:unsupported_type) do + Class.new(::Ci::Processable) do + def self.name + "UnsupportedCustomType" + end + end + end + + subject do + described_class.new(current_user: user, project: project, params: params, type: unsupported_type) + end + + it 'raises an error' do + expect { subject.execute }.to raise_error(ArgumentError, /finder does not support/) + end end context 'when artifacts are present for some jobs' do diff --git a/spec/graphql/resolvers/project_jobs_resolver_spec.rb b/spec/graphql/resolvers/project_jobs_resolver_spec.rb index 5fd27fef7624ba..189e9773595b53 100644 --- a/spec/graphql/resolvers/project_jobs_resolver_spec.rb +++ b/spec/graphql/resolvers/project_jobs_resolver_spec.rb @@ -181,6 +181,23 @@ it { is_expected.not_to include(failed_build, pending_build, failed_bridge) } end end + + context 'when returning just builds' do + before do + stub_feature_flags(return_bridges_and_builds: false) + end + + context 'without any filters' do + it { is_expected.to include(successful_build, successful_build_two, failed_build, pending_build) } + end + + context 'with status filter' do + let(:args) { { statuses: [Types::Ci::JobStatusEnum.coerce_isolated_input('SUCCESS')] } } + + it { is_expected.to include(successful_build, successful_build_two) } + it { is_expected.not_to include(failed_build, pending_build) } + end + end end context 'with unauthorized user' do -- GitLab From a9e27b3a8aff814c97e5973134c6508f70d5e124 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Mon, 7 Jul 2025 16:28:43 +0200 Subject: [PATCH 15/22] Fix job type --- app/graphql/resolvers/project_jobs_resolver.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/graphql/resolvers/project_jobs_resolver.rb b/app/graphql/resolvers/project_jobs_resolver.rb index b3d4f62dd5df88..7c4bbcae44bded 100644 --- a/app/graphql/resolvers/project_jobs_resolver.rb +++ b/app/graphql/resolvers/project_jobs_resolver.rb @@ -43,10 +43,10 @@ def resolve_with_lookahead(**args) } # Determine the correct job type based on filtering needs - job_type = filter_by_name || filter_by_sources ? ::Ci::Build : ::Ci::Processable + @job_type = filter_by_name || filter_by_sources ? ::Ci::Build : ::Ci::Processable ::Ci::JobsFinder - .new(current_user: current_user, project: project, params: job_params, type: job_type) + .new(current_user: current_user, project: project, params: job_params, type: @job_type) .execute else ::Ci::JobsFinder.new( -- GitLab From 3c67c87f4fd450d4b98b555e125231a577dc86e3 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 9 Jul 2025 19:15:00 +0200 Subject: [PATCH 16/22] Change back preloads --- app/graphql/resolvers/project_jobs_resolver.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/graphql/resolvers/project_jobs_resolver.rb b/app/graphql/resolvers/project_jobs_resolver.rb index 7c4bbcae44bded..0e965916e2374c 100644 --- a/app/graphql/resolvers/project_jobs_resolver.rb +++ b/app/graphql/resolvers/project_jobs_resolver.rb @@ -79,16 +79,12 @@ def resolve_with_lookahead(**args) private def preloads - base_preloads = { + { previous_stage_jobs_or_needs: [:needs, :pipeline], pipeline: [:user], + artifacts: [:job_artifacts], build_source: [:source] } - - # Only include artifacts preload when we're using builds - base_preloads[:artifacts] = [:job_artifacts] if should_preload_artifacts? - - base_preloads end def should_preload_artifacts? -- GitLab From 83edfd7f902e505388be68a4496fc2817c415db8 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 9 Jul 2025 19:26:22 +0200 Subject: [PATCH 17/22] Use try instead of respond to --- app/finders/ci/jobs_finder.rb | 18 +++++------------- app/graphql/resolvers/project_jobs_resolver.rb | 4 ---- ee/app/finders/ee/ci/jobs_finder.rb | 4 ++-- 3 files changed, 7 insertions(+), 19 deletions(-) diff --git a/app/finders/ci/jobs_finder.rb b/app/finders/ci/jobs_finder.rb index 5dd054dfd1d5a8..fa523aca67aa04 100644 --- a/app/finders/ci/jobs_finder.rb +++ b/app/finders/ci/jobs_finder.rb @@ -101,9 +101,8 @@ def filter_by_scope(builds) def filter_by_runner_types(builds) return builds unless use_runner_type_filter? - return builds unless builds.respond_to?(:with_runner_type) - builds.with_runner_type(params[:runner_type]) + builds.try(:with_runner_type, params[:runner_type]) || builds end # Overridden in EE @@ -114,7 +113,7 @@ def use_runner_type_filter? def filter_by_with_artifacts(builds) return builds unless params[:with_artifacts] - builds.with_any_artifacts + builds.try(:with_any_artifacts) || builds end def filter_by_statuses!(builds) @@ -133,13 +132,15 @@ def jobs_by_type(relation, type) end def jobs_by_type_with_handling_processables(relation, type) + return relation.builds if params[:with_artifacts] + case type.name when ::Ci::Build.name relation.builds when ::Ci::Bridge.name relation.bridges when ::Ci::Processable.name - processables_with_artifact_filter(relation) + relation.processables else raise ArgumentError, "finder does not support #{type} type" end @@ -155,15 +156,6 @@ def jobs_by_type_without_handling_processables(relation, type) raise ArgumentError, "finder does not support #{type} type" end end - - # When filtering by artifacts, only builds have artifacts - def processables_with_artifact_filter(relation) - if params[:with_artifacts] - relation.builds - else - relation.processables - end - end end end diff --git a/app/graphql/resolvers/project_jobs_resolver.rb b/app/graphql/resolvers/project_jobs_resolver.rb index 0e965916e2374c..5c6d17da4e2fed 100644 --- a/app/graphql/resolvers/project_jobs_resolver.rb +++ b/app/graphql/resolvers/project_jobs_resolver.rb @@ -86,9 +86,5 @@ def preloads build_source: [:source] } end - - def should_preload_artifacts? - @resolver_args&.dig(:with_artifacts) || @job_type == ::Ci::Build - end end end diff --git a/ee/app/finders/ee/ci/jobs_finder.rb b/ee/app/finders/ee/ci/jobs_finder.rb index 170627f67023cb..30f506a19d459d 100644 --- a/ee/app/finders/ee/ci/jobs_finder.rb +++ b/ee/app/finders/ee/ci/jobs_finder.rb @@ -23,9 +23,9 @@ def use_runner_type_filter? def filter_by_failure_reason(builds) return builds unless use_failure_reason_filter? - return builds unless builds.respond_to?(:recently_failed_on_instance_runner) - builds.recently_failed_on_instance_runner(params[:failure_reason]) # currently limited to instance runners + # currently limited to instance runners + builds.try(:recently_failed_on_instance_runner, params[:failure_reason]) || builds end def use_failure_reason_filter? -- GitLab From d9f6fde3bac317e1802671837125574572a39b7d Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 9 Jul 2025 19:37:16 +0200 Subject: [PATCH 18/22] Add briges scope --- app/models/project.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/project.rb b/app/models/project.rb index 8b3608a2e50850..0d5ef0c73a0075 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -453,6 +453,7 @@ def with_developer_access has_many :pending_builds, class_name: 'Ci::PendingBuild' has_many :builds, class_name: 'Ci::Build', inverse_of: :project has_many :processables, class_name: 'Ci::Processable', inverse_of: :project + has_many :bridges, class_name: 'Ci::Bridge', inverse_of: :project has_many :build_trace_chunks, class_name: 'Ci::BuildTraceChunk', through: :builds, source: :trace_chunks, dependent: :restrict_with_error has_many :build_report_results, class_name: 'Ci::BuildReportResult', inverse_of: :project has_many :job_artifacts, class_name: 'Ci::JobArtifact', dependent: :restrict_with_error -- GitLab From 1d8eec906992b33d652919d9436f21b72b844a07 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 10 Jul 2025 09:23:51 +0200 Subject: [PATCH 19/22] Add bridges to all_models --- spec/lib/gitlab/import_export/all_models.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index c9e06d98091b90..896245ded90066 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -573,6 +573,7 @@ container_repositories: - project - name project: +- bridges - instance_runner_monthly_usages - hosted_runner_monthly_usages - catalog_resource -- GitLab From 1bec048b85311ced677c2c16bcc5c2fd46c219cb Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 10 Jul 2025 15:37:54 +0200 Subject: [PATCH 20/22] Simplify job type determination --- .../resolvers/project_jobs_resolver.rb | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/app/graphql/resolvers/project_jobs_resolver.rb b/app/graphql/resolvers/project_jobs_resolver.rb index 5c6d17da4e2fed..06aad57aa204a2 100644 --- a/app/graphql/resolvers/project_jobs_resolver.rb +++ b/app/graphql/resolvers/project_jobs_resolver.rb @@ -36,26 +36,22 @@ def resolve_with_lookahead(**args) filter_by_name = Feature.enabled?(:populate_and_use_build_names_table, project) && args[:name].to_s.present? filter_by_sources = args[:sources].present? - jobs = if Feature.enabled?(:return_bridges_and_builds, project) - job_params = { - scope: args[:statuses], with_artifacts: args[:with_artifacts], - skip_ordering: filter_by_sources - } - - # Determine the correct job type based on filtering needs - @job_type = filter_by_name || filter_by_sources ? ::Ci::Build : ::Ci::Processable + # Determine the correct job type based on filtering needs + job_type = if Feature.enabled?(:return_bridges_and_builds, project) + filter_by_name || filter_by_sources ? ::Ci::Build : ::Ci::Processable + else + ::Ci::Build + end + + job_params = { + scope: args[:statuses], with_artifacts: args[:with_artifacts], + skip_ordering: filter_by_sources + } - ::Ci::JobsFinder - .new(current_user: current_user, project: project, params: job_params, type: @job_type) + jobs = ::Ci::JobsFinder + .new(current_user: current_user, project: project, params: job_params, type: job_type) .execute - else - ::Ci::JobsFinder.new( - current_user: current_user, project: project, params: { - scope: args[:statuses], with_artifacts: args[:with_artifacts], - skip_ordering: filter_by_sources - } - ).execute - end + # These job filters are currently exclusive with each other if filter_by_name jobs = ::Ci::BuildNameFinder.new( -- GitLab From 8b27cd0f051aeb85b227f5425408586c08b1be57 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Sat, 12 Jul 2025 01:03:13 +0200 Subject: [PATCH 21/22] Fix spec name --- app/graphql/resolvers/project_jobs_resolver.rb | 2 -- ee/spec/finders/ee/ci/jobs_finder_spec.rb | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/app/graphql/resolvers/project_jobs_resolver.rb b/app/graphql/resolvers/project_jobs_resolver.rb index 06aad57aa204a2..1717e8522fd07c 100644 --- a/app/graphql/resolvers/project_jobs_resolver.rb +++ b/app/graphql/resolvers/project_jobs_resolver.rb @@ -31,8 +31,6 @@ class ProjectJobsResolver < BaseResolver alias_method :project, :object def resolve_with_lookahead(**args) - @resolver_args = args - filter_by_name = Feature.enabled?(:populate_and_use_build_names_table, project) && args[:name].to_s.present? filter_by_sources = args[:sources].present? diff --git a/ee/spec/finders/ee/ci/jobs_finder_spec.rb b/ee/spec/finders/ee/ci/jobs_finder_spec.rb index a1aa42cb78a5ab..3b036f171c8989 100644 --- a/ee/spec/finders/ee/ci/jobs_finder_spec.rb +++ b/ee/spec/finders/ee/ci/jobs_finder_spec.rb @@ -154,7 +154,7 @@ described_class.new(current_user: current_user, runner: instance_runner).execute end - it 'raises an ArgumentError for invalid type' do + it 'finds the correct builds' do expect(finder_execute.ids).not_to be_empty end end -- GitLab From 932c3cf8eca7bf5bcda329566af1cf31fa3003fe Mon Sep 17 00:00:00 2001 From: Payton Burdette Date: Mon, 14 Jul 2025 12:43:33 -0400 Subject: [PATCH 22/22] Fix naming and spec --- app/finders/ci/jobs_finder.rb | 8 +++--- ee/spec/finders/ee/ci/jobs_finder_spec.rb | 31 +++++------------------ 2 files changed, 11 insertions(+), 28 deletions(-) diff --git a/app/finders/ci/jobs_finder.rb b/app/finders/ci/jobs_finder.rb index fa523aca67aa04..bfdb9f30420b0d 100644 --- a/app/finders/ci/jobs_finder.rb +++ b/app/finders/ci/jobs_finder.rb @@ -125,13 +125,13 @@ def filter_by_statuses!(builds) def jobs_by_type(relation, type) if Feature.enabled?(:return_bridges_and_builds, project) - jobs_by_type_with_handling_processables(relation, type) + jobs_by_type_with_processables(relation, type) else - jobs_by_type_without_handling_processables(relation, type) + jobs_by_type_without_processables(relation, type) end end - def jobs_by_type_with_handling_processables(relation, type) + def jobs_by_type_with_processables(relation, type) return relation.builds if params[:with_artifacts] case type.name @@ -146,7 +146,7 @@ def jobs_by_type_with_handling_processables(relation, type) end end - def jobs_by_type_without_handling_processables(relation, type) + def jobs_by_type_without_processables(relation, type) case type.name when ::Ci::Build.name relation.builds diff --git a/ee/spec/finders/ee/ci/jobs_finder_spec.rb b/ee/spec/finders/ee/ci/jobs_finder_spec.rb index 3b036f171c8989..e409aab61bbff4 100644 --- a/ee/spec/finders/ee/ci/jobs_finder_spec.rb +++ b/ee/spec/finders/ee/ci/jobs_finder_spec.rb @@ -131,32 +131,15 @@ end end - context 'when feature flag :return_bridges_and_builds is disabled' do - before do - stub_feature_flags(return_bridges_and_builds: false) - end - - context 'when build type is invalid' do - let(:type) { String } - let(:runner_performance_insights) { true } + context 'when recently_failed_on_instance_runner method is not available' do + let(:params) { { failure_reason: :runner_system_failure, runner_type: %w[instance_type] } } + let(:runner_performance_insights) { true } - subject(:finder_execute) { described_class.new(current_user: current_user, type: type).execute } - - it 'raises an ArgumentError for invalid type' do - expect { finder_execute }.to raise_error(ArgumentError, /type must be a subclass of Ci::Processable/) - end - end + it 'falls back to original builds' do + expect(::Ci::Build).to receive(:recently_failed_on_instance_runner) + .with(:runner_system_failure).and_return(nil) - context 'when build type is valid' do - let(:runner_performance_insights) { true } - - subject(:finder_execute) do - described_class.new(current_user: current_user, runner: instance_runner).execute - end - - it 'finds the correct builds' do - expect(finder_execute.ids).not_to be_empty - end + expect(finder_execute.ids).to contain_exactly(job1.id, job2.id, job7.id, job3.id, job4.id, job5.id, job6.id) end end end -- GitLab