From 5325c5febc9f78f5665e88a01c8a0668e9474fa8 Mon Sep 17 00:00:00 2001 From: Sahil Sharma Date: Tue, 7 Oct 2025 09:21:39 +0530 Subject: [PATCH 1/9] Display alert for CI variables when cache is not ready --- .../ci/pipeline_new/components/pipeline_variables_form.vue | 6 ++++++ app/graphql/types/project_type.rb | 3 +++ app/services/ci/list_config_variables_service.rb | 7 +++++++ 3 files changed, 16 insertions(+) diff --git a/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue b/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue index 7389f6b773ee11..5c0c5fca3cac0c 100644 --- a/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue +++ b/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue @@ -16,6 +16,7 @@ import { reportToSentry } from '~/ci/utils'; import InputsAdoptionBanner from '~/ci/common/pipeline_inputs/inputs_adoption_banner.vue'; import { fetchPolicies } from '~/lib/graphql'; import Markdown from '~/vue_shared/components/markdown/non_gfm_markdown.vue'; +import { createAlert } from '~/alert'; import filterVariables from '../utils/filter_variables'; import { CI_VARIABLE_TYPE_FILE, @@ -99,6 +100,11 @@ export default { }, error(error) { reportToSentry(this.$options.name, error); + + createAlert({ + message: error.message || s__('Pipeline|Failed to load CI configuration variables.'), + }); + this.ciConfigVariables = []; this.stopPollingAndPopulateForm(); }, diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index 14ebc91850492b..a5f98a13584bac 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -5,6 +5,7 @@ class ProjectType < BaseObject graphql_name 'Project' include ::Namespaces::DeletableHelper + include Gitlab::Graphql::Authorize::AuthorizeResource connection_type_class Types::CountableConnectionType @@ -1041,6 +1042,8 @@ def ci_config_variables(ref:) result.map do |var_key, var_config| { key: var_key, **var_config } end + rescue ::Ci::ListConfigVariablesService::CacheNotReadyError => e + raise_resource_not_available_error! e.message end def job(id:) diff --git a/app/services/ci/list_config_variables_service.rb b/app/services/ci/list_config_variables_service.rb index 3e1c0ce3ff328e..8d1c138bbb27f6 100644 --- a/app/services/ci/list_config_variables_service.rb +++ b/app/services/ci/list_config_variables_service.rb @@ -4,6 +4,8 @@ module Ci class ListConfigVariablesService < ::BaseService include ReactiveCaching + CacheNotReadyError = Class.new(StandardError) + self.reactive_cache_key = ->(service) { [service.class.name, service.id] } self.reactive_cache_work_type = :external_dependency self.reactive_cache_worker_finder = ->(id, *_args) { from_cache(id) } @@ -21,6 +23,11 @@ def execute(ref) # "ref" is not a enough for a cache key because the name is static but that branch can be changed any time sha = project.commit(ref).try(:sha) + unless within_reactive_cache_lifetime?(sha, ref) + raise CacheNotReadyError, + "Failed to retrieve CI configuration variables from cache." + end + with_reactive_cache(sha, ref) { |result| result } end -- GitLab From 8d62c8d639752851f4ca7b91a4a6b42a83a02837 Mon Sep 17 00:00:00 2001 From: Sahil Sharma Date: Tue, 7 Oct 2025 09:22:22 +0530 Subject: [PATCH 2/9] Add translation strings --- locale/gitlab.pot | 3 +++ 1 file changed, 3 insertions(+) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index c38132d48051de..1f29fd77b54d52 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -48973,6 +48973,9 @@ msgstr "" msgid "Pipeline|Failed" msgstr "" +msgid "Pipeline|Failed to load CI configuration variables." +msgstr "" + msgid "Pipeline|File" msgstr "" -- GitLab From 2dd4938eccdc77a23d04fc771252e81dc5b2d99e Mon Sep 17 00:00:00 2001 From: Sahil Sharma Date: Wed, 8 Oct 2025 07:17:34 +0530 Subject: [PATCH 3/9] Add tests to verify ListConfigVariablesService throwing error for missing cache --- .../ci/list_config_variables_service.rb | 6 ++-- .../ci/list_config_variables_service_spec.rb | 29 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/app/services/ci/list_config_variables_service.rb b/app/services/ci/list_config_variables_service.rb index 8d1c138bbb27f6..433eb3754c4ec7 100644 --- a/app/services/ci/list_config_variables_service.rb +++ b/app/services/ci/list_config_variables_service.rb @@ -23,12 +23,14 @@ def execute(ref) # "ref" is not a enough for a cache key because the name is static but that branch can be changed any time sha = project.commit(ref).try(:sha) - unless within_reactive_cache_lifetime?(sha, ref) + result = with_reactive_cache(sha, ref) { |result| result } + + if result.nil? && !within_reactive_cache_lifetime?(sha, ref) raise CacheNotReadyError, "Failed to retrieve CI configuration variables from cache." end - with_reactive_cache(sha, ref) { |result| result } + result end def calculate_reactive_cache(sha, ref) diff --git a/spec/services/ci/list_config_variables_service_spec.rb b/spec/services/ci/list_config_variables_service_spec.rb index 4c363412f3dfb8..7633784cf7e51e 100644 --- a/spec/services/ci/list_config_variables_service_spec.rb +++ b/spec/services/ci/list_config_variables_service_spec.rb @@ -307,4 +307,33 @@ expect(result).to be_nil end end + + context 'when cache is not within reactive cache lifetime' do + let(:ci_config) do + { + variables: { + KEY1: { value: 'val 1', description: 'description 1' } + }, + test: { + stage: 'test', + script: 'echo' + } + } + end + + before do + allow(service).to receive(:within_reactive_cache_lifetime?).and_return(false) + end + + it 'raises CacheNotReadyError' do + expect { result }.to raise_error(described_class::CacheNotReadyError) + end + + it 'raises error with expected message' do + expect { result }.to raise_error( + described_class::CacheNotReadyError, + 'Failed to retrieve CI configuration variables from cache.' + ) + end + end end -- GitLab From 9d7585e6d6c9a945227d8af92644a2c867f80e16 Mon Sep 17 00:00:00 2001 From: Sahil Sharma Date: Wed, 8 Oct 2025 07:36:42 +0530 Subject: [PATCH 4/9] Add test for project type graphql resolver for ci_config_variables field --- spec/graphql/types/project_type_spec.rb | 77 +++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index 1a98eeb3706baf..10f0bf9dcdae4f 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -1785,4 +1785,81 @@ end end end + + describe 'ci_config_variables field' do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + let(:ref) { project.default_branch } + + let(:query) do + %( + query { + project(fullPath: "#{project.full_path}") { + ciConfigVariables(ref: "#{ref}") { + key + value + description + } + } + } + ) + end + + subject { GitlabSchema.execute(query, context: { current_user: user }).as_json } + + before do + project.add_developer(user) + end + + context 'when cache is not ready' do + before do + allow_next_instance_of(::Ci::ListConfigVariablesService) do |service| + allow(service).to receive(:execute).and_raise( + ::Ci::ListConfigVariablesService::CacheNotReadyError, + 'Failed to retrieve CI configuration variables from cache.' + ) + end + end + + it 'returns GraphQL error with expected message' do + expect(subject['errors']).to be_present + expect(subject['errors'].first['message']).to eq('Failed to retrieve CI configuration variables from cache.') + expect(subject['data']['project']['ciConfigVariables']).to be_nil + end + + it 'includes error path information' do + expect(subject['errors'].first['path']).to eq(%w[project ciConfigVariables]) + end + end + + context 'when variables are successfully fetched' do + let(:ci_config_variables) do + { + KEY1: { value: 'val 1', description: 'description 1' }, + KEY2: { value: 'val 2', description: '' }, + KEY3: { value: 'val 3' } + } + end + + before do + allow_next_instance_of(::Ci::ListConfigVariablesService) do |service| + allow(service).to receive(:execute).and_return(ci_config_variables) + end + end + + it 'returns variables list' do + variables = subject.dig('data', 'project', 'ciConfigVariables') + + expect(variables).to contain_exactly( + { 'key' => 'KEY1', 'value' => 'val 1', 'description' => 'description 1' }, + { 'key' => 'KEY2', 'value' => 'val 2', 'description' => '' }, + { 'key' => 'KEY3', 'value' => 'val 3', 'description' => nil } + ) + end + + it 'does not return any errors' do + expect(subject['errors']).to be_nil + end + end + end end -- GitLab From a99af71bc22faffcef72fcda6458acdba6951c21 Mon Sep 17 00:00:00 2001 From: Sahil Sharma Date: Wed, 8 Oct 2025 07:58:09 +0530 Subject: [PATCH 5/9] Add frontend test to verify that alert is triggered on graphql error --- .../components/pipeline_variables_form_spec.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/spec/frontend/ci/pipeline_new/components/pipeline_variables_form_spec.js b/spec/frontend/ci/pipeline_new/components/pipeline_variables_form_spec.js index 689d10821f1636..b5acec9132f5a4 100644 --- a/spec/frontend/ci/pipeline_new/components/pipeline_variables_form_spec.js +++ b/spec/frontend/ci/pipeline_new/components/pipeline_variables_form_spec.js @@ -12,6 +12,9 @@ import { VARIABLE_TYPE } from '~/ci/pipeline_new/constants'; import InputsAdoptionBanner from '~/ci/common/pipeline_inputs/inputs_adoption_banner.vue'; import PipelineVariablesForm from '~/ci/pipeline_new/components/pipeline_variables_form.vue'; import Markdown from '~/vue_shared/components/markdown/non_gfm_markdown.vue'; +import { createAlert } from '~/alert'; + +jest.mock('~/alert'); Vue.use(VueApollo); jest.mock('~/ci/utils'); @@ -113,6 +116,8 @@ describe('PipelineVariablesForm', () => { }, }, }); + + jest.clearAllMocks(); }); it('displays the inputs adoption banner', async () => { @@ -243,6 +248,16 @@ describe('PipelineVariablesForm', () => { expect(reportToSentry).toHaveBeenCalledWith('PipelineVariablesForm', error); }); + + it('displays alert when GraphQL query fails', async () => { + const error = new Error('GraphQL error'); + await createComponent(); + wrapper.vm.$options.apollo.ciConfigVariables.error.call(wrapper.vm, error); + + expect(createAlert).toHaveBeenCalledWith({ + message: 'GraphQL error', + }); + }); }); describe('polling behavior', () => { -- GitLab From 97eec9513ec9b7563d9f844d0d8f90a6a192fe73 Mon Sep 17 00:00:00 2001 From: Sahil Sharma Date: Wed, 8 Oct 2025 18:29:13 +0530 Subject: [PATCH 6/9] Small tweaks to error messages --- .../ci/pipeline_new/components/pipeline_variables_form.vue | 2 +- app/services/ci/list_config_variables_service.rb | 2 +- locale/gitlab.pot | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue b/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue index 5c0c5fca3cac0c..5eae21903df241 100644 --- a/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue +++ b/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue @@ -102,7 +102,7 @@ export default { reportToSentry(this.$options.name, error); createAlert({ - message: error.message || s__('Pipeline|Failed to load CI configuration variables.'), + message: error.message || s__('Pipeline|Failed to retrieve CI/CD variables.'), }); this.ciConfigVariables = []; diff --git a/app/services/ci/list_config_variables_service.rb b/app/services/ci/list_config_variables_service.rb index 433eb3754c4ec7..a05db837846dee 100644 --- a/app/services/ci/list_config_variables_service.rb +++ b/app/services/ci/list_config_variables_service.rb @@ -27,7 +27,7 @@ def execute(ref) if result.nil? && !within_reactive_cache_lifetime?(sha, ref) raise CacheNotReadyError, - "Failed to retrieve CI configuration variables from cache." + "Failed to retrieve CI/CD variables from cache." end result diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 1f29fd77b54d52..0d00c0865e23ba 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -48973,7 +48973,7 @@ msgstr "" msgid "Pipeline|Failed" msgstr "" -msgid "Pipeline|Failed to load CI configuration variables." +msgid "Pipeline|Failed to retrieve CI/CD variables." msgstr "" msgid "Pipeline|File" -- GitLab From 513bc34b53780335157e678479c037873d6dc44b Mon Sep 17 00:00:00 2001 From: Sahil Sharma Date: Wed, 8 Oct 2025 21:45:29 +0530 Subject: [PATCH 7/9] Allow synchronous_reactive_cache helper to mock within_reactive_cache_lifetime as well --- spec/support/helpers/reactive_caching_helpers.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/support/helpers/reactive_caching_helpers.rb b/spec/support/helpers/reactive_caching_helpers.rb index 604abbe2edfd8e..b364772b58dffb 100644 --- a/spec/support/helpers/reactive_caching_helpers.rb +++ b/spec/support/helpers/reactive_caching_helpers.rb @@ -22,6 +22,8 @@ def synchronous_reactive_cache(subject) allow(subject).to receive(:with_reactive_cache) do |*args, &block| block.call(subject.calculate_reactive_cache(*args)) end + + allow(subject).to receive(:within_reactive_cache_lifetime?).and_return(true) end def read_reactive_cache(subject, *qualifiers) -- GitLab From ef54bb8b82bc4661d6869cc05bb660d7593ea8e9 Mon Sep 17 00:00:00 2001 From: Sahil Sharma Date: Tue, 14 Oct 2025 08:07:32 +0530 Subject: [PATCH 8/9] Throw error for ciConfigVariablesQuery on last polling attempt --- .../components/pipeline_variables_form.vue | 108 +++++++------ .../queries/ci_config_variables.graphql | 4 +- app/graphql/types/project_type.rb | 9 +- .../ci/list_config_variables_service.rb | 4 +- doc/api/graphql/reference/_index.md | 1 + .../pipeline_variables_form_spec.js | 142 +++++++++++++----- spec/graphql/types/project_type_spec.rb | 60 ++++++++ .../ci/list_config_variables_service_spec.rb | 36 ++--- 8 files changed, 256 insertions(+), 108 deletions(-) diff --git a/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue b/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue index 5eae21903df241..0466aefebfa3a9 100644 --- a/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue +++ b/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue @@ -78,39 +78,10 @@ export default { configVariablesWithDescription: {}, form: {}, maxPollTimeout: null, + pollingStartTime: null, + manualPollInterval: null, }; }, - apollo: { - ciConfigVariables: { - fetchPolicy: fetchPolicies.NO_CACHE, - query: ciConfigVariablesQuery, - variables() { - return { - fullPath: this.projectPath, - ref: this.refParam, - }; - }, - update({ project }) { - return project?.ciConfigVariables; - }, - result({ data }) { - if (data?.project?.ciConfigVariables) { - this.stopPollingAndPopulateForm(); - } - }, - error(error) { - reportToSentry(this.$options.name, error); - - createAlert({ - message: error.message || s__('Pipeline|Failed to retrieve CI/CD variables.'), - }); - - this.ciConfigVariables = []; - this.stopPollingAndPopulateForm(); - }, - pollInterval: CI_VARIABLES_POLLING_INTERVAL, - }, - }, computed: { descriptions() { return this.form[this.refParam]?.descriptions ?? {}; @@ -147,12 +118,11 @@ export default { refParam() { this.ciConfigVariables = null; this.clearTimeouts(); - this.$apollo.queries.ciConfigVariables.startPolling(CI_VARIABLES_POLLING_INTERVAL); - this.startMaxPollTimer(); + this.startManualPolling(); }, }, mounted() { - this.startMaxPollTimer(); + this.startManualPolling(); }, beforeDestroy() { this.clearTimeouts(); @@ -181,6 +151,11 @@ export default { clearTimeout(this.maxPollTimeout); this.maxPollTimeout = null; } + if (this.manualPollInterval) { + clearInterval(this.manualPollInterval); + this.manualPollInterval = null; + } + this.pollingStartTime = null; }, createListItemsFromVariableOptions(key) { const options = this.configVariablesWithDescription?.options?.[key] || []; @@ -266,19 +241,66 @@ export default { shouldShowValuesDropdown(key) { return this.configVariablesWithDescription.options[key]?.length > 1; }, - startMaxPollTimer() { - this.maxPollTimeout = setTimeout(() => { - if (this.ciConfigVariables === null) { - this.ciConfigVariables = []; - } - this.stopPollingAndPopulateForm(); - }, CI_VARIABLES_MAX_POLLING_TIME); - }, stopPollingAndPopulateForm() { this.clearTimeouts(); - this.$apollo.queries.ciConfigVariables.stopPolling(); this.populateForm(); }, + async executeQuery(failOnCacheMiss = false) { + try { + const result = await this.$apollo.query({ + query: ciConfigVariablesQuery, + variables: { + fullPath: this.projectPath, + ref: this.refParam, + failOnCacheMiss, + }, + fetchPolicy: fetchPolicies.NO_CACHE, + }); + + this.ciConfigVariables = result.data?.project?.ciConfigVariables; + + if (this.ciConfigVariables) { + this.stopPollingAndPopulateForm(); + return true; + } + + return false; + } catch (error) { + this.handleQueryError(error); + return true; + } + }, + handleQueryError(error) { + reportToSentry(this.$options.name, error); + createAlert({ + message: error.message || s__('Pipeline|Failed to retrieve CI/CD variables.'), + }); + this.ciConfigVariables = []; + this.stopPollingAndPopulateForm(); + }, + startManualPolling() { + const CI_VARIABLES_FINAL_ATTEMPT_THRESHOLD = + CI_VARIABLES_MAX_POLLING_TIME - CI_VARIABLES_POLLING_INTERVAL; + this.pollingStartTime = Date.now(); + + this.executeQuery(false); + + this.manualPollInterval = setInterval(async () => { + const pollingDuration = Date.now() - this.pollingStartTime; + const isLastAttempt = pollingDuration >= CI_VARIABLES_FINAL_ATTEMPT_THRESHOLD; + + const shouldStop = await this.executeQuery(isLastAttempt); + + if (shouldStop) { + this.clearTimeouts(); + } + }, CI_VARIABLES_POLLING_INTERVAL); + + this.maxPollTimeout = setTimeout(() => { + this.ciConfigVariables = this.ciConfigVariables || []; + this.stopPollingAndPopulateForm(); + }, CI_VARIABLES_MAX_POLLING_TIME); + }, }, }; diff --git a/app/assets/javascripts/ci/pipeline_new/graphql/queries/ci_config_variables.graphql b/app/assets/javascripts/ci/pipeline_new/graphql/queries/ci_config_variables.graphql index f93f5ad4f11a11..d6d1dea8498cd0 100644 --- a/app/assets/javascripts/ci/pipeline_new/graphql/queries/ci_config_variables.graphql +++ b/app/assets/javascripts/ci/pipeline_new/graphql/queries/ci_config_variables.graphql @@ -1,7 +1,7 @@ -query ciConfigVariables($fullPath: ID!, $ref: String!) { +query ciConfigVariables($fullPath: ID!, $ref: String!, $failOnCacheMiss: Boolean = false) { project(fullPath: $fullPath) { id - ciConfigVariables(ref: $ref) { + ciConfigVariables(ref: $ref, failOnCacheMiss: $failOnCacheMiss) { description key value diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index a5f98a13584bac..0a790e60a91aa6 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -35,6 +35,10 @@ def self.authorization_scopes authorize: :create_pipeline, experiment: { milestone: '15.3' }, description: 'CI/CD config variable.' do + argument :fail_on_cache_miss, GraphQL::Types::Boolean, + required: false, + default_value: false, + description: 'Whether to throw an error if cache is not ready.' argument :ref, GraphQL::Types::String, required: true, description: 'Ref.' @@ -1034,8 +1038,9 @@ def ci_pipeline_creation_inputs(ref:) response.payload[:inputs].all_inputs end - def ci_config_variables(ref:) - result = ::Ci::ListConfigVariablesService.new(object, context[:current_user]).execute(ref) + def ci_config_variables(ref:, fail_on_cache_miss: false) + result = ::Ci::ListConfigVariablesService.new(object, context[:current_user]).execute(ref, + fail_on_cache_miss: fail_on_cache_miss) return if result.nil? diff --git a/app/services/ci/list_config_variables_service.rb b/app/services/ci/list_config_variables_service.rb index a05db837846dee..7e46e425c5954b 100644 --- a/app/services/ci/list_config_variables_service.rb +++ b/app/services/ci/list_config_variables_service.rb @@ -19,13 +19,13 @@ def self.from_cache(id) new(project, user) end - def execute(ref) + def execute(ref, fail_on_cache_miss: false) # "ref" is not a enough for a cache key because the name is static but that branch can be changed any time sha = project.commit(ref).try(:sha) result = with_reactive_cache(sha, ref) { |result| result } - if result.nil? && !within_reactive_cache_lifetime?(sha, ref) + if result.nil? && fail_on_cache_miss raise CacheNotReadyError, "Failed to retrieve CI/CD variables from cache." end diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index c4f293dda8a162..86d411833c486c 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -39851,6 +39851,7 @@ Returns [`[CiConfigVariable!]`](#ciconfigvariable). | Name | Type | Description | | ---- | ---- | ----------- | +| `failOnCacheMiss` | [`Boolean`](#boolean) | Whether to throw an error if cache is not ready. | | `ref` | [`String!`](#string) | Ref. | ##### `Project.ciPipelineCreationInputs` diff --git a/spec/frontend/ci/pipeline_new/components/pipeline_variables_form_spec.js b/spec/frontend/ci/pipeline_new/components/pipeline_variables_form_spec.js index b5acec9132f5a4..a2170d4c8cd218 100644 --- a/spec/frontend/ci/pipeline_new/components/pipeline_variables_form_spec.js +++ b/spec/frontend/ci/pipeline_new/components/pipeline_variables_form_spec.js @@ -5,7 +5,6 @@ import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; -import { fetchPolicies } from '~/lib/graphql'; import { reportToSentry } from '~/ci/utils'; import ciConfigVariablesQuery from '~/ci/pipeline_new/graphql/queries/ci_config_variables.graphql'; import { VARIABLE_TYPE } from '~/ci/pipeline_new/constants'; @@ -15,6 +14,7 @@ import Markdown from '~/vue_shared/components/markdown/non_gfm_markdown.vue'; import { createAlert } from '~/alert'; jest.mock('~/alert'); +jest.useFakeTimers(); Vue.use(VueApollo); jest.mock('~/ci/utils'); @@ -224,78 +224,152 @@ describe('PipelineVariablesForm', () => { }); describe('query configuration', () => { - it('has correct apollo query configuration', async () => { - await createComponent(); - const { apollo } = wrapper.vm.$options; - - expect(apollo.ciConfigVariables.fetchPolicy).toBe(fetchPolicies.NO_CACHE); - expect(apollo.ciConfigVariables.query).toBe(ciConfigVariablesQuery); - }); - it('makes query with correct variables', async () => { await createComponent(); expect(mockCiConfigVariables).toHaveBeenCalledWith({ fullPath: defaultProvide.projectPath, ref: defaultProps.refParam, + failOnCacheMiss: false, }); }); - it('reports to sentry when query fails', async () => { + it('handles query errors in executeQuery method', async () => { const error = new Error('GraphQL error'); + const mockQuery = jest.fn().mockRejectedValue(error); + await createComponent(); - wrapper.vm.$options.apollo.ciConfigVariables.error.call(wrapper.vm, error); + wrapper.vm.$apollo.query = mockQuery; + + await wrapper.vm.executeQuery(false); expect(reportToSentry).toHaveBeenCalledWith('PipelineVariablesForm', error); + expect(createAlert).toHaveBeenCalledWith({ + message: 'GraphQL error', + }); + expect(wrapper.vm.ciConfigVariables).toEqual([]); }); - it('displays alert when GraphQL query fails', async () => { + it('handles query errors with failOnCacheMiss flag in executeQuery method', async () => { const error = new Error('GraphQL error'); + const mockQuery = jest.fn().mockRejectedValue(error); + await createComponent(); - wrapper.vm.$options.apollo.ciConfigVariables.error.call(wrapper.vm, error); + wrapper.vm.$apollo.query = mockQuery; - expect(createAlert).toHaveBeenCalledWith({ - message: 'GraphQL error', + const result = await wrapper.vm.executeQuery(true); + + expect(result).toBe(true); + expect(reportToSentry).toHaveBeenCalledWith('PipelineVariablesForm', error); + }); + + it('returns false when no ciConfigVariables received', async () => { + const mockQuery = jest.fn().mockResolvedValue({ + data: { project: { ciConfigVariables: null } }, + }); + + await createComponent(); + wrapper.vm.$apollo.query = mockQuery; + + const result = await wrapper.vm.executeQuery(false); + + expect(result).toBe(false); + }); + + it('returns true when ciConfigVariables received', async () => { + const mockQuery = jest.fn().mockResolvedValue({ + data: { project: { ciConfigVariables: configVariablesWithOptions } }, }); + + await createComponent(); + wrapper.vm.$apollo.query = mockQuery; + + const result = await wrapper.vm.executeQuery(false); + + expect(result).toBe(true); }); }); describe('polling behavior', () => { - it('configures Apollo with the correct polling interval', () => { - expect(PipelineVariablesForm.apollo.ciConfigVariables.pollInterval).toBe(2000); + it('starts manual polling with correct interval', async () => { + jest.spyOn(global, 'setInterval'); + jest.spyOn(global, 'setTimeout'); + + await createComponent(); + + wrapper.vm.startManualPolling(); + + expect(setInterval).toHaveBeenCalledWith(expect.any(Function), 2000); + expect(setTimeout).toHaveBeenCalledWith(expect.any(Function), 10000); + }); + + it('executes query immediately when starting manual polling', async () => { + const executeQuerySpy = jest.spyOn(PipelineVariablesForm.methods, 'executeQuery'); + + await createComponent(); + + expect(executeQuerySpy).toHaveBeenCalledWith(false); }); it('refetches and updates on ref change', async () => { await createComponent(); - wrapper.setProps({ refParam: 'new-ref-param' }); - await nextTick(); + const startManualPollingSpy = jest.spyOn(wrapper.vm, 'startManualPolling'); + + await wrapper.setProps({ refParam: 'refs/heads/new-feature' }); - expect(wrapper.vm.ciConfigVariables).toBe(null); + expect(startManualPollingSpy).toHaveBeenCalledTimes(1); + + await wrapper.setProps({ refParam: 'refs/heads/new-feature-1' }); + + expect(startManualPollingSpy).toHaveBeenCalledTimes(2); }); - it('sets ciConfigVariables to empty array on query error', async () => { + it('stops polling when data is received', async () => { + jest.spyOn(global, 'clearInterval'); + jest.spyOn(global, 'clearTimeout'); + + mockCiConfigVariables = jest + .fn() + .mockResolvedValueOnce({ + data: { project: { ciConfigVariables: null } }, + }) + .mockResolvedValueOnce({ + data: { project: { ciConfigVariables: configVariablesWithOptions } }, + }); + await createComponent(); - const error = new Error('GraphQL error'); - wrapper.vm.$options.apollo.ciConfigVariables.error.call(wrapper.vm, error); + jest.advanceTimersByTime(2000); + await waitForPromises(); - expect(wrapper.vm.ciConfigVariables).toEqual([]); - expect(reportToSentry).toHaveBeenCalledWith('PipelineVariablesForm', error); + expect(clearInterval).toHaveBeenCalled(); + expect(clearTimeout).toHaveBeenCalled(); }); - it('stops polling when data is received', async () => { - await createComponent({ configVariables: configVariablesWithOptions }); + it('clears all timeouts when polling stops due to successful query', async () => { + jest.spyOn(global, 'clearInterval'); + jest.spyOn(global, 'clearTimeout'); + + await createComponent(); + jest.spyOn(wrapper.vm, 'executeQuery').mockResolvedValue(true); + + jest.advanceTimersByTime(2000); + await waitForPromises(); - const stopPollingSpy = jest.spyOn( - wrapper.vm.$apollo.queries.ciConfigVariables, - 'stopPolling', - ); + expect(clearInterval).toHaveBeenCalled(); + expect(clearTimeout).toHaveBeenCalled(); + expect(wrapper.vm.manualPollInterval).toBe(null); + expect(wrapper.vm.pollingStartTime).toBe(null); + }); - const mockData = { data: { project: { ciConfigVariables: configVariablesWithOptions } } }; - wrapper.vm.$options.apollo.ciConfigVariables.result.call(wrapper.vm, mockData); + it('sets empty array when max poll timeout reached', async () => { + await createComponent(); - expect(stopPollingSpy).toHaveBeenCalled(); + jest.advanceTimersByTime(10000); + await waitForPromises(); + + expect(wrapper.vm.ciConfigVariables).toEqual([]); }); }); diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index 10f0bf9dcdae4f..bdb844d5869e9f 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -1861,5 +1861,65 @@ expect(subject['errors']).to be_nil end end + + context 'with fail_on_cache_miss argument' do + let(:fail_on_cache_miss) { nil } + let(:query) do + fail_on_cache_miss_arg = fail_on_cache_miss.nil? ? '' : ", failOnCacheMiss: #{fail_on_cache_miss}" + %( + query { + project(fullPath: "#{project.full_path}") { + ciConfigVariables(ref: "#{ref}"#{fail_on_cache_miss_arg}) { + key + value + description + } + } + } + ) + end + + context 'when service is called with fail_on_cache_miss parameter' do + before do + allow_next_instance_of(::Ci::ListConfigVariablesService) do |service| + allow(service).to receive(:execute).and_return({}) + end + end + + context 'with default value (not specified)' do + it 'calls service with fail_on_cache_miss: false' do + expect_next_instance_of(::Ci::ListConfigVariablesService) do |service| + expect(service).to receive(:execute).with(ref, fail_on_cache_miss: false) + end + + subject + end + end + + context 'with fail_on_cache_miss: false' do + let(:fail_on_cache_miss) { false } + + it 'calls service with fail_on_cache_miss: false' do + expect_next_instance_of(::Ci::ListConfigVariablesService) do |service| + expect(service).to receive(:execute).with(ref, fail_on_cache_miss: false) + end + + subject + end + end + + context 'with fail_on_cache_miss: true' do + let(:fail_on_cache_miss) { true } + + it 'calls service with fail_on_cache_miss: true' do + expect_next_instance_of(::Ci::ListConfigVariablesService) do |service| + expect(service).to receive(:execute).with(ref, fail_on_cache_miss: true) + end + + subject + end + end + end + end end end diff --git a/spec/services/ci/list_config_variables_service_spec.rb b/spec/services/ci/list_config_variables_service_spec.rb index 7633784cf7e51e..fc9f1f0b8607b1 100644 --- a/spec/services/ci/list_config_variables_service_spec.rb +++ b/spec/services/ci/list_config_variables_service_spec.rb @@ -306,34 +306,20 @@ expect(result).to be_nil end - end - context 'when cache is not within reactive cache lifetime' do - let(:ci_config) do - { - variables: { - KEY1: { value: 'val 1', description: 'description 1' } - }, - test: { - stage: 'test', - script: 'echo' - } - } - end - - before do - allow(service).to receive(:within_reactive_cache_lifetime?).and_return(false) - end + context 'when fail_on_cache_miss is true' do + subject(:result) { service.execute(ref, fail_on_cache_miss: true) } - it 'raises CacheNotReadyError' do - expect { result }.to raise_error(described_class::CacheNotReadyError) - end + it 'raises CacheNotReadyError' do + expect { result }.to raise_error(described_class::CacheNotReadyError) + end - it 'raises error with expected message' do - expect { result }.to raise_error( - described_class::CacheNotReadyError, - 'Failed to retrieve CI configuration variables from cache.' - ) + it 'raises error with expected message' do + expect { result }.to raise_error( + described_class::CacheNotReadyError, + 'Failed to retrieve CI/CD variables from cache.' + ) + end end end end -- GitLab From 78e74c2c20d890eba8dda8af1d0c51cac457baf7 Mon Sep 17 00:00:00 2001 From: Sahil Sharma Date: Tue, 14 Oct 2025 09:48:46 +0530 Subject: [PATCH 9/9] Implement fail_on_cache_miss in config_variables_spec --- .../api/graphql/ci/config_variables_spec.rb | 59 ++++++++++++++----- 1 file changed, 44 insertions(+), 15 deletions(-) diff --git a/spec/requests/api/graphql/ci/config_variables_spec.rb b/spec/requests/api/graphql/ci/config_variables_spec.rb index 6a9dcfee9667ad..03916d8a81b707 100644 --- a/spec/requests/api/graphql/ci/config_variables_spec.rb +++ b/spec/requests/api/graphql/ci/config_variables_spec.rb @@ -15,12 +15,13 @@ let(:service) { Ci::ListConfigVariablesService.new(project, user) } let(:ref) { project.default_branch } + let(:fail_on_cache_miss) { false } let(:query) do %( query { project(fullPath: "#{project.full_path}") { - ciConfigVariables(ref: "#{ref}") { + ciConfigVariables(ref: "#{ref}", failOnCacheMiss: #{fail_on_cache_miss}) { key value valueOptions @@ -40,19 +41,8 @@ end context 'when the cache is not empty' do - before do - synchronous_reactive_cache(service) - end - - it 'returns the CI variables for the config' do - expect(service) - .to receive(:execute) - .with(ref) - .and_call_original - - post_graphql(query, current_user: user) - - expect(graphql_data.dig('project', 'ciConfigVariables')).to contain_exactly( + let(:expected_ci_variables) do + [ { 'key' => 'KEY_VALUE_VAR', 'value' => 'value x', @@ -71,7 +61,32 @@ 'valueOptions' => ['env var value', 'env var value2'], 'description' => 'env var description' } - ) + ] + end + + shared_examples 'returns CI variables' do + it 'returns the CI variables for the config' do + expect(service) + .to receive(:execute) + .with(ref, fail_on_cache_miss: fail_on_cache_miss) + .and_call_original + + post_graphql(query, current_user: user) + + expect(graphql_data.dig('project', 'ciConfigVariables')).to match_array(expected_ci_variables) + end + end + + before do + synchronous_reactive_cache(service) + end + + it_behaves_like 'returns CI variables' + + context 'when failOnCacheMiss is true' do + let(:fail_on_cache_miss) { true } + + it_behaves_like 'returns CI variables' end end @@ -81,6 +96,20 @@ expect(graphql_data.dig('project', 'ciConfigVariables')).to be_nil end + + context 'when failOnCacheMiss is true' do + let(:fail_on_cache_miss) { true } + + it 'returns an error' do + post_graphql(query, current_user: user) + + expect(graphql_errors).to include( + a_hash_including( + 'message' => 'Failed to retrieve CI/CD variables from cache.' + ) + ) + end + end end end -- GitLab