From cfe215de319aa84872571a481781794ed91d7686 Mon Sep 17 00:00:00 2001
From: Chad Woolley <cwoolley@gitlab.com>
Date: Thu, 25 Jul 2024 12:35:42 +0000
Subject: [PATCH] Apply 1 suggestion(s) to 1 file(s)
Co-authored-by: Marcin Sedlak-Jakubowski <msedlakjakubowski@gitlab.com>
---
.../testing_guide/best_practices.md | 7 ++
.../llm/chain/answers/streamed_json_spec.rb | 3 +-
.../lib/system_check/app/search_check_spec.rb | 3 +-
scripts/run-fast-specs.sh | 97 +++++++++++++++++++
spec/bin/sidekiq_cluster_spec.rb | 2 +-
.../diagnostic_reports/uploader_smoke_spec.rb | 2 +-
spec/dot_gitlab_ci/rules_spec.rb | 5 +-
.../linter/inline_javascript_spec.rb | 2 +-
.../ci/config/entry/include/rules_spec.rb | 3 +-
.../gitlab/graphql/known_operations_spec.rb | 3 +-
spec/lib/gitlab/i18n/pluralization_spec.rb | 3 +-
.../redis_cluster_validator_spec.rb | 4 +-
.../pagination/offset_header_builder_spec.rb | 3 +-
spec/lib/remote_development/settings_spec.rb | 2 +-
.../concerns/super_sidebar_panel_spec.rb | 3 +-
.../lib/glfm/update_example_snapshots_spec.rb | 2 +-
.../lib/glfm/update_specification_spec.rb | 2 +-
17 files changed, 130 insertions(+), 16 deletions(-)
create mode 100755 scripts/run-fast-specs.sh
diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md
index be19b1f674f4..e76fd4433058 100644
--- a/doc/development/testing_guide/best_practices.md
+++ b/doc/development/testing_guide/best_practices.md
@@ -871,6 +871,13 @@ To verify that code and its specs are well-isolated from Rails, run the spec
individually via `bin/rspec`. Don't use `bin/spring rspec` as it loads
`spec_helper` automatically.
+#### Maintaining fast_spec_helper specs
+
+There is a utility script `scripts/run-fast-specs.sh` which can be used to run
+all specs which use `fast_spec_helper`, in various ways. This script is useful
+to help identify `fast_spec_helper` specs which have problems, such as not
+running successfully in isolation. See the script for more details.
+
### `subject` and `let` variables
The GitLab RSpec suite has made extensive use of `let`(along with its strict, non-lazy
diff --git a/ee/spec/lib/gitlab/llm/chain/answers/streamed_json_spec.rb b/ee/spec/lib/gitlab/llm/chain/answers/streamed_json_spec.rb
index 7dfa14bab385..10a0a15942cf 100644
--- a/ee/spec/lib/gitlab/llm/chain/answers/streamed_json_spec.rb
+++ b/ee/spec/lib/gitlab/llm/chain/answers/streamed_json_spec.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
-require 'fast_spec_helper'
+# require 'fast_spec_helper' -- this no longer runs under fast_spec_helper
+require 'spec_helper'
RSpec.describe Gitlab::Llm::Chain::Answers::StreamedJson, feature_category: :duo_chat do
describe "#next_chunk" do
diff --git a/ee/spec/lib/system_check/app/search_check_spec.rb b/ee/spec/lib/system_check/app/search_check_spec.rb
index 2e033b09ffb8..b507e2597f01 100644
--- a/ee/spec/lib/system_check/app/search_check_spec.rb
+++ b/ee/spec/lib/system_check/app/search_check_spec.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
-require 'fast_spec_helper'
+# require 'fast_spec_helper' -- this no longer runs under fast_spec_helper
+require 'spec_helper'
require 'rspec-parameterized'
RSpec.describe SystemCheck::App::SearchCheck do
diff --git a/scripts/run-fast-specs.sh b/scripts/run-fast-specs.sh
new file mode 100755
index 000000000000..c46da331ef73
--- /dev/null
+++ b/scripts/run-fast-specs.sh
@@ -0,0 +1,97 @@
+#!/usr/bin/env bash
+
+# shellcheck disable=SC2059
+
+BCyan='\033[1;36m'
+BRed='\033[1;31m'
+BGreen='\033[1;32m'
+BBlue='\033[1;34m'
+Color_Off='\033[0m'
+
+set -o errexit
+set -o pipefail
+trap onexit_err ERR
+
+# Exit handling
+function onexit_err() {
+ local exit_status=${1:-$?}
+ printf "\nâŒâŒâŒ ${BRed}Run of fast_spec_helper specs failed!${Color_Off} âŒâŒâŒ\n"
+ exit "${exit_status}"
+}
+
+function print_start_message {
+ trap onexit_err ERR
+
+ printf "${BCyan}\nStarting run of fast_spec_helper specs...${Color_Off}\n"
+}
+
+function run_fast_spec_helper_specs {
+ trap onexit_err ERR
+
+ printf "\n\n${BBlue}Running specs which use fast_spec_helper and also run fast...${Color_Off}\n\n"
+
+ # TIP: Add '--format=documentation --order=stable' when debugging flaky interaction between specs
+ bin/rspec --tag=~uses_fast_spec_helper_but_runs_slow "${fast_spec_helper_specs[@]}"
+}
+
+function run_fast_spec_helper_specs_which_are_slow {
+ trap onexit_err ERR
+
+ printf "\n\n${BBlue}Running specs which use fast_spec_helper but actually run slow...${Color_Off}\n\n"
+
+ # NOTE: fails_if_sidekiq_not_configured tag is used to skip specs which require special local config for sidekiq
+ bin/rspec --tag=uses_fast_spec_helper_but_runs_slow --tag=~fails_if_sidekiq_not_configured "${fast_spec_helper_specs[@]}"
+}
+
+function run_all_fast_spec_helper_specs_individually {
+ trap onexit_err ERR
+
+ printf "\n\n${BBlue}INDIVIDUALLY running all specs which use fast_spec_helper to ensure they run in isolation (this will be SLOW! Set SPEC_FILE_TO_START_AT to skip known-good spec files)...${Color_Off}\n\n"
+
+ # NOTE: Override `SPEC_FILE_TO_START_AT` to the relative path of a specific file in order to skip specs which are already known to be passing
+ SPEC_FILE_TO_START_AT="${SPEC_FILE_TO_START_AT:-${fast_spec_helper_specs[0]}}"
+
+ for spec_file in "${fast_spec_helper_specs[@]}"; do
+ if [ "${spec_file}" = "${SPEC_FILE_TO_START_AT}" ]; then
+ printf "${BBlue}Starting individual spec runs at SPEC_FILE_TO_START_AT: '${SPEC_FILE_TO_START_AT}'${Color_Off}\n\n"
+ START_RUNNING=true
+ fi
+
+ if [ -n "${START_RUNNING}" ]; then
+ printf "${BBlue}Running spec '${spec_file}' individually:${Color_Off}\n"
+ bin/rspec --tag=~fails_if_sidekiq_not_configured "$spec_file"
+ fi
+ done
+}
+
+function print_success_message {
+ trap onexit_err ERR
+
+ printf "\n✅✅✅ ${BGreen}All executed fast_spec_helper specs passed successfully!${Color_Off} ✅✅✅\n"
+}
+
+function main {
+ trap onexit_err ERR
+
+ # cd to gitlab root directory
+ cd "$(dirname "${BASH_SOURCE[0]}")"/..
+
+ # See https://github.com/koalaman/shellcheck/wiki/SC2207 for more context on this approach of creating an array
+ fast_spec_helper_specs=()
+ while IFS='' read -r line; do
+ fast_spec_helper_specs+=("$line")
+ done < <(git grep -l -E '^require .fast_spec_helper' -- '**/*_spec.rb')
+
+ print_start_message
+
+ if [ -n "${RUN_ALL_FAST_SPECS_INDIVIDUALLY}" ]; then
+ run_all_fast_spec_helper_specs_individually
+ else
+ run_fast_spec_helper_specs
+ run_fast_spec_helper_specs_which_are_slow
+ fi
+
+ print_success_message
+}
+
+main "$@"
diff --git a/spec/bin/sidekiq_cluster_spec.rb b/spec/bin/sidekiq_cluster_spec.rb
index 3379fee12632..2e0c1f67a952 100644
--- a/spec/bin/sidekiq_cluster_spec.rb
+++ b/spec/bin/sidekiq_cluster_spec.rb
@@ -4,7 +4,7 @@
require 'shellwords'
require 'rspec-parameterized'
-RSpec.describe 'bin/sidekiq-cluster', :aggregate_failures do
+RSpec.describe 'bin/sidekiq-cluster', :uses_fast_spec_helper_but_runs_slow, :fails_if_sidekiq_not_configured, :aggregate_failures do
using RSpec::Parameterized::TableSyntax
let(:root) { File.expand_path('../..', __dir__) }
diff --git a/spec/commands/diagnostic_reports/uploader_smoke_spec.rb b/spec/commands/diagnostic_reports/uploader_smoke_spec.rb
index 1ec4eb407d8b..e9d157bda08b 100644
--- a/spec/commands/diagnostic_reports/uploader_smoke_spec.rb
+++ b/spec/commands/diagnostic_reports/uploader_smoke_spec.rb
@@ -5,7 +5,7 @@
# We need to capture pid from Process.spawn and then clean up by killing the process, which requires instance variables.
# rubocop: disable RSpec/InstanceVariable
-RSpec.describe 'bin/diagnostic-reports-uploader' do
+RSpec.describe 'bin/diagnostic-reports-uploader', :uses_fast_spec_helper_but_runs_slow do
# This is a smoke test for 'bin/diagnostic-reports-uploader'.
# We intend to run this binary with `ruby bin/diagnostic-reports-uploader`, without preloading the entire Rails app.
# Also, we use inline gemfile, to avoid pulling full Gemfile from the main app into memory.
diff --git a/spec/dot_gitlab_ci/rules_spec.rb b/spec/dot_gitlab_ci/rules_spec.rb
index 85a72fa32adf..d5892b5a42ac 100644
--- a/spec/dot_gitlab_ci/rules_spec.rb
+++ b/spec/dot_gitlab_ci/rules_spec.rb
@@ -1,6 +1,9 @@
# frozen_string_literal: true
-require 'fast_spec_helper'
+# NOTE: Do not remove the parentheses from this require statement!
+# They are necessary so it doesn't match the regex in `scripts/run-fast-specs.sh`,
+# and make the "fast" portion of that suite run slow.
+require('fast_spec_helper') # NOTE: Do not remove the parentheses from this require statement!
PatternsList = Struct.new(:name, :patterns)
diff --git a/spec/haml_lint/linter/inline_javascript_spec.rb b/spec/haml_lint/linter/inline_javascript_spec.rb
index fb35bb682473..9d00e3ab9d4a 100644
--- a/spec/haml_lint/linter/inline_javascript_spec.rb
+++ b/spec/haml_lint/linter/inline_javascript_spec.rb
@@ -7,7 +7,7 @@
require_relative '../../../haml_lint/linter/inline_javascript'
-RSpec.describe HamlLint::Linter::InlineJavaScript do # rubocop:disable RSpec/FilePath
+RSpec.describe HamlLint::Linter::InlineJavaScript, :uses_fast_spec_helper_but_runs_slow do # rubocop:disable RSpec/FilePath
using RSpec::Parameterized::TableSyntax
include_context 'linter'
diff --git a/spec/lib/gitlab/ci/config/entry/include/rules_spec.rb b/spec/lib/gitlab/ci/config/entry/include/rules_spec.rb
index 503020e2202c..c8e53082c93c 100644
--- a/spec/lib/gitlab/ci/config/entry/include/rules_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/include/rules_spec.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
-require 'fast_spec_helper'
+# require 'fast_spec_helper' -- this no longer runs under fast_spec_helper
+require 'spec_helper'
require_dependency 'active_model'
RSpec.describe Gitlab::Ci::Config::Entry::Include::Rules, feature_category: :pipeline_composition do
diff --git a/spec/lib/gitlab/graphql/known_operations_spec.rb b/spec/lib/gitlab/graphql/known_operations_spec.rb
index acb85bc737b6..601cc10ef032 100644
--- a/spec/lib/gitlab/graphql/known_operations_spec.rb
+++ b/spec/lib/gitlab/graphql/known_operations_spec.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
-require 'fast_spec_helper'
+# require 'fast_spec_helper' -- this no longer runs under fast_spec_helper
+require 'spec_helper'
require 'rspec-parameterized'
RSpec.describe Gitlab::Graphql::KnownOperations do
diff --git a/spec/lib/gitlab/i18n/pluralization_spec.rb b/spec/lib/gitlab/i18n/pluralization_spec.rb
index 6bfc500c8b89..4aab6daa21e9 100644
--- a/spec/lib/gitlab/i18n/pluralization_spec.rb
+++ b/spec/lib/gitlab/i18n/pluralization_spec.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
-require 'fast_spec_helper'
+# require 'fast_spec_helper' -- this no longer runs under fast_spec_helper
+require 'spec_helper'
require 'rspec-parameterized'
require 'rails/version'
require 'gettext_i18n_rails'
diff --git a/spec/lib/gitlab/instrumentation/redis_cluster_validator_spec.rb b/spec/lib/gitlab/instrumentation/redis_cluster_validator_spec.rb
index ea5a32a25ff1..679006f00c3c 100644
--- a/spec/lib/gitlab/instrumentation/redis_cluster_validator_spec.rb
+++ b/spec/lib/gitlab/instrumentation/redis_cluster_validator_spec.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
-require 'fast_spec_helper'
-require 'support/helpers/rails_helpers'
+# require 'fast_spec_helper' -- this no longer runs under fast_spec_helper
+require 'spec_helper'
require 'rspec-parameterized'
RSpec.describe Gitlab::Instrumentation::RedisClusterValidator, feature_category: :scalability do
diff --git a/spec/lib/gitlab/pagination/offset_header_builder_spec.rb b/spec/lib/gitlab/pagination/offset_header_builder_spec.rb
index f43216702a84..dc964ad502f0 100644
--- a/spec/lib/gitlab/pagination/offset_header_builder_spec.rb
+++ b/spec/lib/gitlab/pagination/offset_header_builder_spec.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
-require 'fast_spec_helper'
+# require 'fast_spec_helper' -- this no longer runs under fast_spec_helper
+require 'spec_helper'
RSpec.describe Gitlab::Pagination::OffsetHeaderBuilder do
let(:request) { double(url: 'http://localhost') }
diff --git a/spec/lib/remote_development/settings_spec.rb b/spec/lib/remote_development/settings_spec.rb
index 1d95f85bf719..ef11e716a424 100644
--- a/spec/lib/remote_development/settings_spec.rb
+++ b/spec/lib/remote_development/settings_spec.rb
@@ -2,7 +2,7 @@
require 'fast_spec_helper'
-RSpec.describe RemoteDevelopment::Settings, feature_category: :remote_development do
+RSpec.describe RemoteDevelopment::Settings, :rd_fast, feature_category: :remote_development do
let(:response_hash) { { settings: { some_setting: 42 }, status: :success } }
let(:get_settings_args) { { requested_setting_names: [:some_setting], options: { some_option: 42 } } }
diff --git a/spec/lib/sidebars/concerns/super_sidebar_panel_spec.rb b/spec/lib/sidebars/concerns/super_sidebar_panel_spec.rb
index e0c05379a9e0..0536d0e67d32 100644
--- a/spec/lib/sidebars/concerns/super_sidebar_panel_spec.rb
+++ b/spec/lib/sidebars/concerns/super_sidebar_panel_spec.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
-require 'fast_spec_helper'
+# require 'fast_spec_helper' -- this no longer runs under fast_spec_helper
+require 'spec_helper'
RSpec.describe Sidebars::Concerns::SuperSidebarPanel, feature_category: :navigation do
let(:menu_class_foo) { Class.new(Sidebars::Menu) }
diff --git a/spec/scripts/lib/glfm/update_example_snapshots_spec.rb b/spec/scripts/lib/glfm/update_example_snapshots_spec.rb
index 82a398ea204a..41466995ead3 100644
--- a/spec/scripts/lib/glfm/update_example_snapshots_spec.rb
+++ b/spec/scripts/lib/glfm/update_example_snapshots_spec.rb
@@ -28,7 +28,7 @@
# Also, the textual content of the individual fixture file entries is also crafted to help
# indicate which scenarios which they are covering.
# rubocop:disable RSpec/MultipleMemoizedHelpers
-RSpec.describe Glfm::UpdateExampleSnapshots, '#process', feature_category: :team_planning do
+RSpec.describe Glfm::UpdateExampleSnapshots, '#process', :uses_fast_spec_helper_but_runs_slow, feature_category: :team_planning do
subject { described_class.new }
# GLFM input files
diff --git a/spec/scripts/lib/glfm/update_specification_spec.rb b/spec/scripts/lib/glfm/update_specification_spec.rb
index 6b9ba8e59be7..753623979f26 100644
--- a/spec/scripts/lib/glfm/update_specification_spec.rb
+++ b/spec/scripts/lib/glfm/update_specification_spec.rb
@@ -27,7 +27,7 @@
# should run in sub-second time when the Spring pre-loader is used. This allows
# logic which is not directly related to the slow sub-processes to be TDD'd with a
# very rapid feedback cycle.
-RSpec.describe Glfm::UpdateSpecification, '#process', feature_category: :team_planning do
+RSpec.describe Glfm::UpdateSpecification, '#process', :uses_fast_spec_helper_but_runs_slow, feature_category: :team_planning do
include NextInstanceOf
subject { described_class.new }
--
GitLab