[go: up one dir, main page]

Skip to content

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
    end

    I 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 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.

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):

  1. Use factories to create builds with consistent job definitions (the job definition could be created in the factory with a sort of find_or_create approach).
  2. Use stubs whenever possible instead of updating records, when let_it_be is used. For this case we could introduce a spec helper stub_job_definition to ensure data like options is stubbed correctly (e.g. using symbol keys consistently).
  3. Use a new unsafe_update_job_definition spec helper if (for test efficiency) we need to update the records. This helper could use update_column under the hood.

Proposal

  1. Update the CI job factories to create builds with consistent job definitions (instead of using associations).

  2. 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.
Edited by Leaminn Ma