Update CI job factories to create builds with consistent job definitions; create a job_definition stub helper for Rspec tests
The following discussion from !202317 (merged) should be addressed:
-
@mbobin started a discussion: (+5 comments) @lma-git wdyt if we are to introduce a module that would mutate the options/definition from the tests? Instead of stubbing or issue write requests directly to the database we could use this to modify the attributes that are stored in the metadata record.
module Ci::MetadataMutator def assign_options(job, options: {}, yaml_variables: {}) job.metadata.write_attribute(:config_options, options) if options.present? job.metadata.write_attribute(:yaml_variables, yaml_variables) if yaml_variables.present? return unless job.job_definition data = { options: options.presence, yaml_variables: yaml_variables.presence }.compact updated_config = job.job_definition.config.merge(data) # the definition will become immutable at some point # so we build a clone and assign it to the job if job.job_definition.persisted? processable.build_job_definition( # I don't know what it does to the to the instance record, I expect it to update the definition id, but ... config: updated_config, project_id: job.project_id, partition_id: job.partition_id ) else job.job_definition.write_attribute(:config, updated_config) end end def update_options(job, options: {}, yaml_variables: {}) assign_options(job, options: options, yaml_variables: yaml_variables) job.save! end endI haven't tested this to see if it actually works
😅
Or alternatively:
If we go with stubbing I think we should have a module that allows to stub consistently across all the test files which should also help us if we plan to do more refactoring in the area.
Further context
Here was the original motivation to rely on job factories and stubbing (!202317 (merged)):
We will eventually introduce the feature flag
stop_writing_builds_metadata(roll-out issue: #552065), which will prevent us from updating metadata attributes such asoptionsandyaml_variablesafter the job is created. These attributes should not be mutable because we will later populatejob_definitions(#551860 (closed)) and that table is insert-only.We currently mutate these attributes a lot in our spec tests so we need to move away from this approach. Moreover, we need to update the job factories to write to both
ci_builds_metadataandci_job_definitions. If a test mutatesoptions=directly, for example, the change will only update inmetadatabut notjob_definition. As such, the test will break since our code prioritizes reading fromjob_definitionbecause the FFread_from_new_ci_destinationsis default enabled in our specs.
However, this no-mutation approach may not be ideal per !202317 (comment 2709537430).
Possible strategies
(Moved here from #519972 (closed).)
For specs we should prefer (in order):
- Use factories to create builds with consistent job definitions (the job definition could be created in the factory with a sort of
find_or_createapproach). - Use stubs whenever possible instead of updating records, when
let_it_beis used. For this case we could introduce a spec helperstub_job_definitionto ensure data likeoptionsis stubbed correctly (e.g. using symbol keys consistently). - Use a new
unsafe_update_job_definitionspec helper if (for test efficiency) we need to update the records. This helper could useupdate_columnunder the hood.
Proposal
-
Update the CI job factories to create builds with consistent job definitions (instead of using associations).
-
Create a helper for stubbing job definitions/attributes so that they're stubbed consistently across files.
- Replace all instances where we currently do
allow(job).to receive(<attribute>).and_return(<value>), or similar, to use the stub helper.