Skip to content

Dont persist application settings in test env bis

Second try after !8573 (merged) / gitlab-ee!1056 got merged and introduced an issue in EE.

EE MR: gitlab-org/gitlab-ee!1098

What does this MR do?

This proposes another approach than !8532 (closed) to one of the problems reported in https://gitlab.com/gitlab-org/gitlab-ce/issues/24899#note_21207429.

Basically the idea is that we don't actually need a persisted ApplicationSetting record during the tests so we just disable the auto-creation of ApplicationSetting in CurrentSettings#ensure_application_settings!.

By simply returning ApplicationSetting.new(ApplicationSetting::DEFAULTS) (the defaults were moved to a constant because they were duplicated and not even the same between ApplicationSetting and CurrentSettings!), there are two wins:

  • we avoid calling ActiveRecord::Migrator.needs_migration?
  • we avoid calling ApplicationSetting.create_from_defaults

Are there points in the code the reviewer needs to double check?

There may be some failing specs but hopefully not too many.

Why was this MR needed?

This should reduce the number of DB queries made per spec example.

Todo

Merge request reports