Update Rspec job factories to write to ci_job_definitions; update tests to stop mutating options & yaml_variables
What does this MR do and why?
Context
We will eventually introduce the feature flag stop_writing_builds_metadata (roll-out issue: #552065 (closed)), which will prevent us from updating metadata attributes such as options and yaml_variables after the job is created. These attributes should not be mutable because we will later populate job_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_metadata and ci_job_definitions. If a test mutates options= directly, for example, the change will only update in metadata but not job_definition. As such, the test will break since our code prioritizes reading from job_definition because the FF read_from_new_ci_destinations is default enabled in our specs.
This MR
This MR helps to prepare for the introduction of the FF stop_writing_builds_metadata for options/yaml_variables. Details:
- Updates the job factory to write to both
ci_builds_metadataandci_job_definitions/ci_job_definition_instances.
- This more accurately represents the data state as we migrate to
ci_job_definitions.
- Updates
Ci::Metadatable#degenerate!to also destroy thejob_definition_instancerecord.
- Since this is just a small change, I included it to easily fix the degenerable job tests.
- Updates the tests to stop mutating
optionsoryaml_variables.
- We only need to address these two attributes for now because they're directly impacted by
Ci::Metadatable#read_metadata_attributewhich prioritizes reading fromjob_definitionin the tests. The other attributes will be addressed as part of #552057 (closed).
Test updates
I generally implemented one of 3 strategies to fix the tests:
- Set the value of
options/yaml_variablesdirectly in the factory on creation/build.
- This allows us to make changes to the tests in the least invasive way. It also ensures that the
optionsvalue follows the correct schema (when FFci_validate_config_optionsis removed) and that it's being read correctly according to theread_metadata_attributelogic. - In the future, we could consider refactoring the
createtests to usebuildorbuild_stubbed(and ensuring.valid?), but for this iteration we're just making the simplest changes.
-
If the object is only created once (e.g. with
let_it_be), or theoptions/yaml_variablesvalue can remain static in the context and option (1) would be impractical, then I stubbed the value withallow(x).to receive(y).and_return(z). -
For all other cases, the feature flag
read_from_new_ci_destinationsis stubbed asfalse.
References
- Resolves Step 2 of the implementation plan for Move getter/setter methods to `Ci::Metadatable`... (#552057 - closed)
MR acceptance checklist
Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Related to #552057 (closed)