Merge branch 'introduce-feature-flag-rspec-tag' into 'master'

Introduce New feature_flag RSpec Metadata Tag

See merge request gitlab-org/gitlab!83776

doc/development/testing_guide/end_to_end/best_practices.md

@@ -279,6 +279,9 @@ When you add a new test that requires administrator access, apply the RSpec meta
 
 When running tests locally or configuring a pipeline, the environment variable `QA_CAN_TEST_ADMIN_FEATURES` can be set to `false` to skip tests that have the `:requires_admin` tag.
 
+NOTE:
+If the _only_ action in the test that requires administrator access is to toggle a feature flag, please use the `feature_flag` tag instead. More details can be found in [testing with feature flags](feature_flags.md).
+
 ## Prefer `Commit` resource over `ProjectPush`
 
 In line with [using the API](#prefer-api-over-ui), use a `Commit` resource whenever possible.

doc/development/testing_guide/end_to_end/feature_flags.md

@@ -14,19 +14,45 @@ automatically authenticates as an administrator as long as you provide an approp
 token via `GITLAB_QA_ADMIN_ACCESS_TOKEN` (recommended), or provide `GITLAB_ADMIN_USERNAME`
 and `GITLAB_ADMIN_PASSWORD`.
 
-Please be sure to include the tag `:requires_admin` so that the test can be skipped in environments
-where administrator access is not available.
+## `feature_flag` RSpec tag
 
-WARNING:
-You are strongly advised to [enable feature flags only for a group, project, user](../../feature_flags/index.md#feature-actors),
-or [feature group](../../feature_flags/index.md#feature-groups). This makes it possible to
-test a feature in a shared environment without affecting other users.
+Please be sure to include the `feature_flag` tag so that the test can be skipped on the appropriate environments.
 
-For example, the code below would enable a feature flag named `:feature_flag_name` for the project
+**Optional metadata:**
+
+`name`
+
+- Format: `feature_flag: { name: 'feature_flag_name' }`
+- Used only for informational purposes at this time. It should be included to help quickly determine what 
+feature flag is under test.
+
+`scope`
+
+- Format: `feature_flag: { name: 'feature_flag_name', scope: :project }`
+- When `scope` is set to `:global`, the test will be **skipped on all live .com environments**. This is to avoid issues with feature flag changes affecting other tests or users on that environment.
+- When `scope` is set to any other value (such as `:project`, `:group` or `:user`), or if no `scope` is specified, the test will only be **skipped on canary and production**.
+This is due to the fact that admin access is not available there.
+
+**WARNING:** You are strongly advised to first try and [enable feature flags only for a group, project, user](../../feature_flags/index.md#feature-actors),
+or [feature group](../../feature_flags/index.md#feature-groups).
+
+- If a global feature flag must be used, it is strongly recommended to apply `scope: :global` to the `feature_flag` metadata. This is, however, left up to the SET's discretion to determine the level of risk. 
+  - For example, a test uses a global feature flag that only affects a small area of the application and is also needed to check for critical issues on live environments. 
+  In such a scenario, it would be riskier to skip running the test. For cases like this, `scope` can be left out of the metadata so that it can still run in live environments 
+  with admin access, such as staging.
+
+**Note on `requires_admin`:** This tag should still be applied if there are other actions within the test that require admin access that are unrelated to updating a 
+feature flag (ex: creating a user via the API).
+
+The code below would enable a feature flag named `:feature_flag_name` for the project
 created by the test:
 
 ```ruby
-RSpec.describe "with feature flag enabled", :requires_admin do
+RSpec.describe "with feature flag enabled", feature_flag: { 
+  name: 'feature_flag_name', 
+  scope: :project 
+  } do
+  
   let(:project) { Resource::Project.fabricate_via_api! }
 
   before do

doc/development/testing_guide/end_to_end/rspec_metadata_tests.md

@@ -12,9 +12,10 @@ This is a partial list of the [RSpec metadata](https://relishapp.com/rspec/rspec
 <!-- Please keep the tags in alphabetical order -->
 
 | Tag                         | Description                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                  |
-|-----|-------------|
+|-----------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | `:elasticsearch`            | The test requires an Elasticsearch service. It is used by the [instance-level scenario](https://gitlab.com/gitlab-org/gitlab-qa#definitions) [`Test::Integration::Elasticsearch`](https://gitlab.com/gitlab-org/gitlab/-/blob/72b62b51bdf513e2936301cb6c7c91ec27c35b4d/qa/qa/ee/scenario/test/integration/elasticsearch.rb) to include only tests that require Elasticsearch.                                                                                                                                |
 | `:except`                   | The test is to be run in their typical execution contexts _except_ as specified. See [test execution context selection](execution_context_selection.md) for more information.                                                                                                                                                                                                                                                                                                                                |
+| `:feature_flag`             | The test uses a feature flag and therefore requires an administrator account to run. When `scope` is set to `:global`, the test will be skipped on all live .com environments. Otherwise, it will be skipped only on Canary and Production. See [testing with feature flags](../../../development/testing_guide/end_to_end/feature_flags.md) for more details.                                                                                                                                               |
 | `:geo`                      | The test requires two GitLab Geo instances - a primary and a secondary - to be spun up.                                                                                                                                                                                                                                                                                                                                                                                                                      |
 | `:gitaly_cluster`           | The test runs against a GitLab instance where repositories are stored on redundant Gitaly nodes behind a Praefect node. All nodes are [separate containers](../../../administration/gitaly/praefect.md#requirements). Tests that use this tag have a longer setup time since there are three additional containers that need to be started.                                                                                                                                                                  |
 | `:github`                   | The test requires a GitHub personal access token.                                                                                                                                                                                                                                                                                                                                                                                                                                                            |
@@ -22,7 +23,7 @@ This is a partial list of the [RSpec metadata](https://relishapp.com/rspec/rspec
 | `:instance_saml`            | The test requires a GitLab instance that has SAML SSO enabled at the instance level. Interacts with an external SAML identity provider. Paired with the `:orchestrated` tag.                                                                                                                                                                                                                                                                                                                                 |
 | `:integrations`             | This aims to test the available [integrations](../../../user/project/integrations/overview.md#integrations-listing). The test requires Docker to be installed in the run context. It will provision the containers and can be run against a local instance or using the `gitlab-qa` scenario `Test::Integration::Integrations`                                                                                                                                                                               |
 | `:service_ping_disabled`    | The test interacts with the GitLab configuration service ping at the instance level to turn admin setting service ping checkbox on or off. This tag will have the test run only in the `service_ping_disabled` job and must be paired with the `:orchestrated` and `:requires_admin` tags.                                                                                                                                                                                                                   |
-| `:jira`           | The test requires a Jira Server. [GitLab-QA](https://gitlab.com/gitlab-org/gitlab-qa) provisions the Jira Server in a Docker container when the `Test::Integration::Jira` test scenario is run.
+| `:jira`                     | The test requires a Jira Server. [GitLab-QA](https://gitlab.com/gitlab-org/gitlab-qa) provisions the Jira Server in a Docker container when the `Test::Integration::Jira` test scenario is run.                                                                                                                                                                                                                                                                                                              |
 | `:kubernetes`               | The test includes a GitLab instance that is configured to be run behind an SSH tunnel, allowing a TLS-accessible GitLab. This test also includes provisioning of at least one Kubernetes cluster to test against. _This tag is often be paired with `:orchestrated`._                                                                                                                                                                                                                                        |
 | `:ldap_no_server`           | The test requires a GitLab instance to be configured to use LDAP. To be used with the `:orchestrated` tag. It does not spin up an LDAP server at orchestration time. Instead, it creates the LDAP server at runtime.                                                                                                                                                                                                                                                                                         |
 | `:ldap_no_tls`              | The test requires a GitLab instance to be configured to use an external LDAP server with TLS not enabled.                                                                                                                                                                                                                                                                                                                                                                                                    |
@@ -44,7 +45,7 @@ This is a partial list of the [RSpec metadata](https://relishapp.com/rspec/rspec
 | `:skip_live_env`            | The test is excluded when run against live deployed environments such as Staging, Canary, and Production.                                                                                                                                                                                                                                                                                                                                                                                                    |
 | `:skip_fips_env`            | The test is excluded when run against an environment in FIPS mode.                                                                                                                                                                                                                                                                                                                                                                                                                                           |
 | `:skip_signup_disabled`     | The test uses UI to sign up a new user and is skipped in any environment that does not allow new user registration via the UI.                                                                                                                                                                                                                                                                                                                                                                               |
-| `:smoke`          | The test belongs to the test suite which verifies basic functionality of a GitLab instance.|
+| `:smoke`                    | The test belongs to the test suite which verifies basic functionality of a GitLab instance.                                                                                                                                                                                                                                                                                                                                                                                                                  |
 | `:smtp`                     | The test requires a GitLab instance to be configured to use an SMTP server. Tests SMTP notification email delivery from GitLab by using MailHog.                                                                                                                                                                                                                                                                                                                                                             |
 | `:testcase`                 | The link to the test case issue in the [GitLab Project Test Cases](https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases).                                                                                                                                                                                                                                                                                                                                                                               |
 | `:transient`                | The test tests transient bugs. It is excluded by default.                                                                                                                                                                                                                                                                                                                                                                                                                                                    |

qa/qa/specs/features/api/1_manage/migration/gitlab_project_migration_common.rb

@@ -3,7 +3,11 @@
 module QA
   # Disable on live envs until bulk_import_projects toggle is on by default
   # Otherwise tests running in parallel can disable feature in the middle of other test
-  RSpec.shared_context 'with gitlab project migration', :requires_admin, :skip_live_env do
+  RSpec.shared_context 'with gitlab project migration', requires_admin: 'creates a user via API',
+    feature_flag: {
+      name: 'bulk_import_projects',
+      scope: :global
+    } do
     let(:source_project_with_readme) { false }
     let(:import_wait_duration) { { max_duration: 300, sleep_interval: 2 } }
     let(:admin_api_client) { Runtime::API::Client.as_admin }

qa/qa/specs/features/ee/browser_ui/2_plan/iterations/create_group_iteration_spec.rb

 # frozen_string_literal: true
 
 module QA
-  # TODO: Remove :requires_admin when the `Runtime::Feature.enable` method call is removed
-  RSpec.describe 'Plan', :requires_admin do
+  RSpec.describe 'Plan', feature_flag: { name: 'iteration_cadences', scope: :group } do
     describe 'Group Iterations' do
       include Support::Dates
 

qa/qa/specs/helpers/feature_flag.rb

+# frozen_string_literal: true
+
+require 'rspec/core'
+
+module QA
+  module Specs
+    module Helpers
+      module FeatureFlag
+        extend self
+
+        def skip_or_run_feature_flag_tests_or_contexts(example)
+          if example.metadata.key?(:feature_flag)
+            feature_flag_tag = example.metadata[:feature_flag]
+
+            global_feature_flag_message = 'Skipping on .com environments due to global feature flag usage'
+            feature_flag_message = 'Skipping on production due to feature flag usage'
+
+            if feature_flag_tag.is_a?(Hash) && feature_flag_tag[:scope] == :global
+              # Tests using a global feature flag will be skipped on live .com environments.
+              # This is to avoid flakiness with other tests running in parallel on the same environment
+              # as well as interfering with feature flag experimentation done by development groups.
+              example.metadata[:skip] = global_feature_flag_message if ContextSelector.dot_com?
+            else
+              # Tests using a feature flag scoped to an actor (ex: :project, :user, :group), or
+              # with no scope defined (such as in the case of a low risk global feature flag),
+              # will only be skipped in canary and production due to no admin account existing there.
+              example.metadata[:skip] = feature_flag_message if ContextSelector.context_matches?(:production)
+            end
+          end
+        end
+      end
+    end
+  end
+end

qa/qa/support/formatters/feature_flag_formatter.rb

+# frozen_string_literal: true
+
+module QA
+  module Support
+    module Formatters
+      class FeatureFlagFormatter < ::RSpec::Core::Formatters::BaseFormatter
+        include Specs::Helpers::FeatureFlag
+
+        ::RSpec::Core::Formatters.register(
+          self,
+          :example_group_started,
+          :example_started
+        )
+
+        # Starts example group
+        # @param [RSpec::Core::Notifications::GroupNotification] example_group_notification
+        # @return [void]
+        def example_group_started(example_group_notification)
+          group = example_group_notification.group
+
+          skip_or_run_feature_flag_tests_or_contexts(group)
+        end
+
+        # Starts example
+        # @param [RSpec::Core::Notifications::ExampleNotification] example_notification
+        # @return [void]
+        def example_started(example_notification)
+          example = example_notification.example
+
+          # if skip propagated from example_group, do not reset skip metadata
+          skip_or_run_feature_flag_tests_or_contexts(example) unless example.metadata[:skip]
+        end
+      end
+    end
+  end
+end

qa/spec/spec_helper.rb

@@ -20,6 +20,7 @@ RSpec.configure do |config|
 
   config.add_formatter QA::Support::Formatters::ContextFormatter
   config.add_formatter QA::Support::Formatters::QuarantineFormatter
+  config.add_formatter QA::Support::Formatters::FeatureFlagFormatter
   config.add_formatter QA::Support::Formatters::TestStatsFormatter if QA::Runtime::Env.export_metrics?
 
   config.before(:suite) do |suite|

qa/spec/specs/helpers/feature_flag_spec.rb

+# frozen_string_literal: true
+
+require 'rspec/core/sandbox'
+
+RSpec.describe QA::Specs::Helpers::FeatureFlag do
+  include QA::Support::Helpers::StubEnv
+  include QA::Specs::Helpers::RSpec
+
+  around do |ex|
+    RSpec::Core::Sandbox.sandboxed do |config|
+      config.add_formatter QA::Support::Formatters::ContextFormatter
+      config.add_formatter QA::Support::Formatters::QuarantineFormatter
+      config.add_formatter QA::Support::Formatters::FeatureFlagFormatter
+
+      # If there is an example-within-an-example, we want to make sure the inner example
+      # does not get a reference to the outer example (the real spec) if it calls
+      # something like `pending`
+      config.before(:context) { RSpec.current_example = nil }
+
+      config.color_mode = :off
+
+      ex.run
+    end
+  end
+
+  describe '.skip_or_run_feature_flag_tests_or_contexts' do
+    shared_examples 'runs with given feature flag metadata' do |metadata|
+      it do
+        group = describe_successfully 'Feature flag test', feature_flag: metadata do
+          it('passes') {}
+        end
+
+        expect(group.examples.first.execution_result.status).to eq(:passed)
+      end
+    end
+
+    shared_examples 'skips with given feature flag metadata' do |metadata|
+      it do
+        group = describe_successfully 'Feature flag test', feature_flag: metadata do
+          it('is skipped') {}
+        end
+
+        expect(group.examples.first.execution_result.status).to eq(:pending)
+      end
+    end
+
+    context 'when run on staging' do
+      before(:context) do
+        QA::Runtime::Scenario.define(:gitlab_address, 'https://staging.gitlab.com')
+      end
+
+      context 'when no scope is defined' do
+        it_behaves_like 'runs with given feature flag metadata', { name: 'no_scope_ff' }
+
+        it 'is skipped if quarantine tag is also applied' do
+          group = describe_successfully 'Feature flag with no scope', feature_flag: { name: 'quarantine_with_ff' }, quarantine: {
+            issue: 'https://gitlab.com/test-group/test/-/issues/123',
+            type: 'bug'
+          } do
+            it('is skipped') {}
+          end
+
+          expect(group.examples.first.execution_result.status).to eq(:pending)
+        end
+      end
+
+      it_behaves_like 'runs with given feature flag metadata', { name: 'actor_ff', scope: :project }
+
+      it_behaves_like 'skips with given feature flag metadata', { name: 'global_ff', scope: :global }
+
+      context 'when should be skipped in a specific job' do
+        before do
+          stub_env('CI_JOB_NAME', 'job-to-skip')
+        end
+
+        it 'is skipped for that job' do
+          group = describe_successfully 'Test should be skipped', feature_flag: { name: 'skip_job_ff' }, except: { job: 'job-to-skip' } do
+            it('does not run on staging in specified job') {}
+          end
+
+          expect(group.examples.first.execution_result.status).to eq(:pending)
+        end
+      end
+
+      context 'when should only run in a specific job' do
+        before do
+          stub_env('CI_JOB_NAME', 'job-to-run')
+        end
+
+        it 'is run for that job' do
+          group = describe_successfully 'Test should run', feature_flag: { name: 'run_job_ff' }, only: { job: 'job-to-run' } do
+            it('runs on staging in specified job') {}
+          end
+
+          expect(group.examples.first.execution_result.status).to eq(:passed)
+        end
+
+        it 'skips if test is set to only run in a job different from current CI job' do
+          group = describe_successfully 'Test should be skipped', feature_flag: { name: 'skip_job_ff' }, only: { job: 'other-job' } do
+            it('does not run on staging in specified job') {}
+          end
+
+          expect(group.examples.first.execution_result.status).to eq(:pending)
+        end
+      end
+    end
+
+    context 'when run on production' do
+      before(:context) do
+        QA::Runtime::Scenario.define(:gitlab_address, 'https://gitlab.com')
+      end
+
+      context 'when no scope is defined' do
+        it_behaves_like 'skips with given feature flag metadata', { name: 'no_scope_ff' }
+
+        context 'for only one test in the example group' do
+          it 'only skips specified test and runs all others' do
+            group = describe_successfully 'Feature flag set for one test' do
+              it('is skipped', feature_flag: { name: 'single_test_ff' }) {}
+              it('passes') {}
+            end
+
+            expect(group.examples[0].execution_result.status).to eq(:pending)
+            expect(group.examples[1].execution_result.status).to eq(:passed)
+          end
+        end
+      end
+
+      it_behaves_like 'skips with given feature flag metadata', { name: 'actor_ff', scope: :project }
+
+      it_behaves_like 'skips with given feature flag metadata', { name: 'global_ff', scope: :global }
+    end
+
+    # The nightly package job, for example, does not run against a live environment with
+    # a defined gitlab_address. In this case, feature_flag tag logic can be safely ignored
+    context 'when run without a gitlab address specified' do
+      before(:context) do
+        QA::Runtime::Scenario.define(:gitlab_address, nil)
+      end
+
+      it_behaves_like 'runs with given feature flag metadata', { name: 'no_scope_ff' }
+
+      it_behaves_like 'runs with given feature flag metadata', { name: 'actor_ff', scope: :project }
+
+      it_behaves_like 'runs with given feature flag metadata', { name: 'global_ff', scope: :global }
+    end
+  end
+end