Merge branch 'acunskis-context-listener' into 'master'

E2E: Move quarantine and context selection logic to formatter

See merge request gitlab-org/gitlab!67423

qa/qa.rb

@@ -632,7 +632,9 @@ module QA
 
     module Helpers
       autoload :ContextSelector, 'qa/specs/helpers/context_selector'
+      autoload :ContextFormatter, 'qa/specs/helpers/context_formatter'
       autoload :Quarantine, 'qa/specs/helpers/quarantine'
+      autoload :QuarantineFormatter, 'qa/specs/helpers/quarantine_formatter'
       autoload :RSpec, 'qa/specs/helpers/rspec'
     end
   end
@@ -680,6 +682,7 @@ module QA
     autoload :WaitForRequests, 'qa/support/wait_for_requests'
     autoload :OTP, 'qa/support/otp'
     autoload :SSH, 'qa/support/ssh'
+    autoload :AllureMetadataFormatter, 'qa/support/allure_metadata_formatter.rb'
   end
 end
 

qa/qa/runtime/allure_report.rb

@@ -67,25 +67,8 @@ module QA
         # @return [void]
         def configure_rspec
           RSpec.configure do |config|
-            config.formatter = AllureRspecFormatter
-
-            config.after do |example|
-              next if example.attempts && example.attempts > 0
-
-              testcase = example.metadata[:testcase]
-              example.tms('Testcase', testcase) if testcase
-
-              quarantine_issue = example.metadata.dig(:quarantine, :issue)
-              example.issue('Quarantine issue', quarantine_issue) if quarantine_issue
-
-              spec_file = example.file_path.split('/').last
-              example.issue(
-                'Failure issues',
-                "https://gitlab.com/gitlab-org/gitlab/-/issues?scope=all&state=opened&search=#{spec_file}"
-              )
-
-              example.add_link(name: "Job(#{Env.ci_job_name})", url: Env.ci_job_url) if Env.running_in_ci?
-            end
+            config.add_formatter(AllureRspecFormatter)
+            config.add_formatter(QA::Support::AllureMetadataFormatter)
           end
         end
 

qa/qa/specs/features/api/1_manage/import_large_github_repo_spec.rb

@@ -96,9 +96,6 @@ module QA
       end
 
       after do |example|
-        # skip saving data if example is skipped or failed before import finished
-        next if example.pending?
-
         user.remove_via_api!
         next unless defined?(@import_time)
 

qa/qa/specs/features/ee/browser_ui/3_create/web_ide/web_terminal_spec.rb

 # frozen_string_literal: true
 
 module QA
-  # This test was quarantined because relative URL isn't supported
-  # See https://gitlab.com/gitlab-org/gitlab/issues/13833
-  RSpec.describe 'Create', :runner, :quarantine do
+  RSpec.describe(
+    'Create',
+    :runner,
+    quarantine: {
+      only: { job: 'relative_url' },
+      issue: 'https://gitlab.com/gitlab-org/gitlab/issues/13833',
+      type: :bug
+    }
+  ) do
     describe 'Web IDE web terminal' do
       before do
         project = Resource::Project.fabricate_via_api! do |project|

qa/qa/specs/helpers/context_formatter.rb

+# frozen_string_literal: true
+
+require 'rspec/core'
+require "rspec/core/formatters/base_formatter"
+
+module QA
+  module Specs
+    module Helpers
+      class ContextFormatter < ::RSpec::Core::Formatters::BaseFormatter
+        include ContextSelector
+
+        ::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)
+          set_skip_metadata(example_group_notification.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
+          set_skip_metadata(example_notification.example) unless example.metadata[:skip]
+        end
+
+        private
+
+        # Skip example_group or example
+        #
+        # @param [<RSpec::Core::ExampleGroup, RSpec::Core::Example>] example
+        # @return [void]
+        def set_skip_metadata(example)
+          return skip_only(example.metadata) if example.metadata.key?(:only)
+          return skip_except(example.metadata) if example.metadata.key?(:except)
+        end
+
+        # Skip based on 'only' condition
+        #
+        # @param [Hash] metadata
+        # @return [void]
+        def skip_only(metadata)
+          return if context_matches?(metadata[:only])
+
+          metadata[:skip] = 'Test is not compatible with this environment or pipeline'
+        end
+
+        # Skip based on 'except' condition
+        #
+        # @param [Hash] metadata
+        # @return [void]
+        def skip_except(metadata)
+          return unless except?(metadata[:except])
+
+          metadata[:skip] = 'Test is excluded in this job'
+        end
+      end
+    end
+  end
+end

qa/qa/specs/helpers/context_selector.rb

@@ -8,18 +8,6 @@ module QA
       module ContextSelector
         extend self
 
-        def configure_rspec
-          ::RSpec.configure do |config|
-            config.before do |example|
-              if example.metadata.key?(:only)
-                skip('Test is not compatible with this environment or pipeline') unless ContextSelector.context_matches?(example.metadata[:only])
-              elsif example.metadata.key?(:except)
-                skip('Test is excluded in this job') if ContextSelector.except?(example.metadata[:except])
-              end
-            end
-          end
-        end
-
         def except?(*options)
           return false if Runtime::Env.ci_job_name.blank? && options.any? { |o| o.is_a?(Hash) && o[:job].present? }
           return false if Runtime::Env.ci_project_name.blank? && options.any? { |o| o.is_a?(Hash) && o[:pipeline].present? }

qa/qa/specs/helpers/quarantine.rb

@@ -10,26 +10,6 @@ module QA
 
         extend self
 
-        def configure_rspec
-          ::RSpec.configure do |config|
-            config.before(:context, :quarantine) do
-              Quarantine.skip_or_run_quarantined_contexts(config.inclusion_filter.rules, self.class)
-            end
-
-            config.before do |example|
-              Quarantine.skip_or_run_quarantined_tests_or_contexts(config.inclusion_filter.rules, example)
-            end
-          end
-        end
-
-        # Skip the entire context if a context is quarantined. This avoids running
-        # before blocks unnecessarily.
-        def skip_or_run_quarantined_contexts(filters, example)
-          return unless example.metadata.key?(:quarantine)
-
-          skip_or_run_quarantined_tests_or_contexts(filters, example)
-        end
-
         # Skip tests in quarantine unless we explicitly focus on them.
         def skip_or_run_quarantined_tests_or_contexts(filters, example)
           if filters.key?(:quarantine)
@@ -43,19 +23,19 @@ module QA
             # running that ldap test as well because of the :quarantine metadata.
             # We could use an exclusion filter, but this way the test report will list
             # the quarantined tests when they're not run so that we're aware of them
-            skip("Only running tests tagged with :quarantine and any of #{included_filters.keys}") if should_skip_when_focused?(example.metadata, included_filters)
-          else
-            if example.metadata.key?(:quarantine)
-              quarantine_tag = example.metadata[:quarantine]
-
-              if quarantine_tag.is_a?(Hash) && quarantine_tag&.key?(:only)
-                # If the :quarantine hash contains :only, we respect that.
-                # For instance `quarantine: { only: { subdomain: :staging } }` will only quarantine the test when it runs against staging.
-                return unless ContextSelector.context_matches?(quarantine_tag[:only])
-              end
+            if should_skip_when_focused?(example.metadata, included_filters)
+              example.metadata[:skip] = "Only running tests tagged with :quarantine and any of #{included_filters.keys}"
+            end
+          elsif example.metadata.key?(:quarantine)
+            quarantine_tag = example.metadata[:quarantine]
 
-              skip(quarantine_message(quarantine_tag))
+            if quarantine_tag.is_a?(Hash) && quarantine_tag&.key?(:only) && !ContextSelector.context_matches?(quarantine_tag[:only])
+              # If the :quarantine hash contains :only, we respect that.
+              # For instance `quarantine: { only: { subdomain: :staging } }` will only quarantine the test when it runs against staging.
+              return
             end
+
+            example.metadata[:skip] = quarantine_message(quarantine_tag)
           end
         end
 
@@ -64,7 +44,7 @@ module QA
         end
 
         def quarantine_message(quarantine_tag)
-          quarantine_message = %w(In quarantine)
+          quarantine_message = %w[In quarantine]
           quarantine_message << case quarantine_tag
                                 when String
                                   ": #{quarantine_tag}"

qa/qa/specs/helpers/quarantine_formatter.rb

+# frozen_string_literal: true
+
+require 'rspec/core'
+require "rspec/core/formatters/base_formatter"
+
+module QA
+  module Specs
+    module Helpers
+      class QuarantineFormatter < ::RSpec::Core::Formatters::BaseFormatter
+        include Quarantine
+
+        ::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_quarantined_tests_or_contexts(filters, 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_quarantined_tests_or_contexts(filters, example) unless example.metadata[:skip]
+        end
+
+        private
+
+        def filters
+          @filters ||= ::RSpec.configuration.inclusion_filter.rules
+        end
+      end
+    end
+  end
+end

qa/qa/specs/helpers/rspec.rb

@@ -19,8 +19,10 @@ module QA
         # expanding into the global state
         # See: https://github.com/rspec/rspec-core/issues/2603
         def describe_successfully(*args, &describe_body)
+          reporter = ::RSpec.configuration.reporter
+
           example_group = RSpec.describe(*args, &describe_body)
-          ran_successfully = example_group.run RaiseOnFailuresReporter
+          ran_successfully = example_group.run reporter
           expect(ran_successfully).to eq true
           example_group
         end

qa/qa/support/allure_metadata_formatter.rb

+# frozen_string_literal: true
+
+require 'rspec/core'
+require "rspec/core/formatters/base_formatter"
+
+module QA
+  module Support
+    class AllureMetadataFormatter < ::RSpec::Core::Formatters::BaseFormatter
+      ::RSpec::Core::Formatters.register(
+        self,
+        :example_started
+      )
+
+      # Starts example
+      # @param [RSpec::Core::Notifications::ExampleNotification] example_notification
+      # @return [void]
+      def example_started(example_notification)
+        example = example_notification.example
+
+        testcase = example.metadata[:testcase]
+        example.tms('Testcase', testcase) if testcase
+
+        quarantine_issue = example.metadata.dig(:quarantine, :issue)
+        example.issue('Quarantine issue', quarantine_issue) if quarantine_issue
+
+        spec_file = example.file_path.split('/').last
+        example.issue(
+          'Failure issues',
+          "https://gitlab.com/gitlab-org/gitlab/-/issues?scope=all&state=opened&search=#{spec_file}"
+        )
+        return unless Runtime::Env.running_in_ci?
+
+        example.add_link(name: "Job(#{Runtime::Env.ci_job_name})", url: Runtime::Env.ci_job_url)
+      end
+    end
+  end
+end

qa/spec/spec_helper.rb

@@ -26,8 +26,8 @@ Dir[::File.join(__dir__, "support/shared_examples/*.rb")].sort.each { |f| requir
 RSpec.configure do |config|
   config.include ::Matchers
 
-  QA::Specs::Helpers::Quarantine.configure_rspec
-  QA::Specs::Helpers::ContextSelector.configure_rspec
+  config.add_formatter QA::Specs::Helpers::ContextFormatter
+  config.add_formatter QA::Specs::Helpers::QuarantineFormatter
 
   config.before do |example|
     QA::Runtime::Logger.debug("\nStarting test: #{example.full_description}\n")

qa/spec/specs/allure_report_spec.rb

@@ -5,21 +5,7 @@ require 'allure-rspec'
 describe QA::Runtime::AllureReport do
   include Helpers::StubENV
 
-  let(:rspec_config) { double('RSpec::Core::Configuration', 'formatter=': nil, after: nil) }
-  let(:rspec_example) do
-    double(
-      'RSpec::Core::Example',
-      tms: nil,
-      issue: nil,
-      add_link: nil,
-      attempts: 0,
-      file_path: 'file/path/spec.rb',
-      metadata: {
-        testcase: 'testcase',
-        quarantine: { issue: 'issue' }
-      }
-    )
-  end
+  let(:rspec_config) { double('RSpec::Core::Configuration', 'add_formatter': nil, after: nil) }
 
   let(:png_path) { 'png_path' }
   let(:html_path) { 'html_path' }
@@ -36,7 +22,6 @@ describe QA::Runtime::AllureReport do
 
     allow(AllureRspec).to receive(:configure).and_yield(allure_config)
     allow(RSpec).to receive(:configure).and_yield(rspec_config)
-    allow(rspec_config).to receive(:after).and_yield(rspec_example)
     allow(Capybara::Screenshot).to receive(:after_save_screenshot).and_yield(png_path)
     allow(Capybara::Screenshot).to receive(:after_save_html).and_yield(html_path)
   end
@@ -62,12 +47,10 @@ describe QA::Runtime::AllureReport do
     let(:png_file) { 'png-file' }
     let(:html_file) { 'html-file' }
     let(:ci_job) { 'ee:relative 5' }
-    let(:ci_job_url) { 'url' }
 
     before do
       stub_env('CI', 'true')
       stub_env('CI_JOB_NAME', ci_job)
-      stub_env('CI_JOB_URL', ci_job_url)
 
       allow(Allure).to receive(:add_attachment)
       allow(File).to receive(:open).with(png_path) { png_file }
@@ -85,20 +68,9 @@ describe QA::Runtime::AllureReport do
       end
     end
 
-    it 'adds rspec formatter' do
-      expect(rspec_config).to have_received(:formatter=).with(AllureRspecFormatter)
-    end
-
-    it 'configures after block' do
-      aggregate_failures do
-        expect(rspec_example).to have_received(:tms).with('Testcase', 'testcase')
-        expect(rspec_example).to have_received(:issue).with('Quarantine issue', 'issue')
-        expect(rspec_example).to have_received(:add_link).with(name: "Job(#{ci_job})", url: ci_job_url)
-        expect(rspec_example).to have_received(:issue).with(
-          'Failure issues',
-          'https://gitlab.com/gitlab-org/gitlab/-/issues?scope=all&state=opened&search=spec.rb'
-        )
-      end
+    it 'adds rspec and metadata formatter' do
+      expect(rspec_config).to have_received(:add_formatter).with(AllureRspecFormatter).ordered
+      expect(rspec_config).to have_received(:add_formatter).with(QA::Support::AllureMetadataFormatter).ordered
     end
 
     it 'configures screenshot saving' do

qa/spec/specs/helpers/context_selector_spec.rb

@@ -2,29 +2,25 @@
 
 require 'rspec/core/sandbox'
 
-RSpec.configure do |c|
-  c.around do |ex|
+RSpec.describe QA::Specs::Helpers::ContextSelector do
+  include Helpers::StubENV
+  include QA::Specs::Helpers::RSpec
+
+  around do |ex|
+    QA::Runtime::Scenario.define(:gitlab_address, 'https://staging.gitlab.com')
+
     RSpec::Core::Sandbox.sandboxed do |config|
+      config.formatter = QA::Specs::Helpers::ContextFormatter
+
       # 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
-end
-
-RSpec.describe QA::Specs::Helpers::ContextSelector do
-  include Helpers::StubENV
-  include QA::Specs::Helpers::RSpec
-
-  before do
-    QA::Runtime::Scenario.define(:gitlab_address, 'https://staging.gitlab.com')
-    described_class.configure_rspec
-  end
 
   describe '.context_matches?' do
     it 'returns true when url has .com' do
@@ -104,7 +100,6 @@ RSpec.describe QA::Specs::Helpers::ContextSelector do
     context 'with different environment set' do
       before do
         QA::Runtime::Scenario.define(:gitlab_address, 'https://gitlab.com')
-        described_class.configure_rspec
       end
 
       it 'does not run against production' do
@@ -239,7 +234,6 @@ RSpec.describe QA::Specs::Helpers::ContextSelector do
     context 'without CI_PROJECT_NAME set' do
       before do
         stub_env('CI_PROJECT_NAME', nil)
-        described_class.configure_rspec
       end
 
       it 'runs on any pipeline' do
@@ -273,7 +267,6 @@ RSpec.describe QA::Specs::Helpers::ContextSelector do
     context 'when a pipeline triggered from the default branch runs in gitlab-qa' do
       before do
         stub_env('CI_PROJECT_NAME', 'gitlab-qa')
-        described_class.configure_rspec
       end
 
       it 'runs on default branch pipelines' do
@@ -310,7 +303,6 @@ RSpec.describe QA::Specs::Helpers::ContextSelector do
     context 'with CI_PROJECT_NAME set' do
       before do
         stub_env('CI_PROJECT_NAME', 'NIGHTLY')
-        described_class.configure_rspec
       end
 
       it 'runs on designated pipeline' do
@@ -353,7 +345,6 @@ RSpec.describe QA::Specs::Helpers::ContextSelector do
     context 'without CI_JOB_NAME set' do
       before do
         stub_env('CI_JOB_NAME', nil)
-        described_class.configure_rspec
       end
 
       context 'when excluding contexts' do
@@ -396,7 +387,6 @@ RSpec.describe QA::Specs::Helpers::ContextSelector do
     context 'with CI_JOB_NAME set' do
       before do
         stub_env('CI_JOB_NAME', 'ee:instance-image')
-        described_class.configure_rspec
       end
 
       context 'when excluding contexts' do

qa/spec/specs/helpers/quarantine_spec.rb

@@ -2,9 +2,14 @@
 
 require 'rspec/core/sandbox'
 
-RSpec.configure do |c|
-  c.around do |ex|
+RSpec.describe QA::Specs::Helpers::Quarantine do
+  include Helpers::StubENV
+  include QA::Specs::Helpers::RSpec
+
+  around do |ex|
     RSpec::Core::Sandbox.sandboxed do |config|
+      config.formatter = QA::Specs::Helpers::QuarantineFormatter
+
       # 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`
@@ -15,18 +20,9 @@ RSpec.configure do |c|
       ex.run
     end
   end
-end
-
-RSpec.describe QA::Specs::Helpers::Quarantine do
-  include Helpers::StubENV
-  include QA::Specs::Helpers::RSpec
 
   describe '.skip_or_run_quarantined_contexts' do
     context 'with no tag focused' do
-      before do
-        described_class.configure_rspec
-      end
-
       it 'skips before hooks of quarantined contexts' do
         executed_hooks = []
 
@@ -66,7 +62,6 @@ RSpec.describe QA::Specs::Helpers::Quarantine do
 
     context 'with :quarantine focused' do
       before do
-        described_class.configure_rspec
         RSpec.configure do |c|
           c.filter_run :quarantine
         end
@@ -110,10 +105,6 @@ RSpec.describe QA::Specs::Helpers::Quarantine do
 
   describe '.skip_or_run_quarantined_tests_or_contexts' do
     context 'with no tag focused' do
-      before do
-        described_class.configure_rspec
-      end
-
       it 'skips quarantined tests' do
         group = describe_successfully do
           it('is pending', :quarantine) {}
@@ -135,7 +126,6 @@ RSpec.describe QA::Specs::Helpers::Quarantine do
       context 'with environment set' do
         before do
           QA::Runtime::Scenario.define(:gitlab_address, 'https://staging.gitlab.com')
-          described_class.configure_rspec
         end
 
         context 'no pipeline specified' do
@@ -168,7 +158,6 @@ RSpec.describe QA::Specs::Helpers::Quarantine do
           shared_examples 'skipped in project' do |project|
             before do
               stub_env('CI_PROJECT_NAME', project)
-              described_class.configure_rspec
             end
 
             it "is skipped in #{project}" do
@@ -209,7 +198,6 @@ RSpec.describe QA::Specs::Helpers::Quarantine do
 
     context 'with :quarantine focused' do
       before do
-        described_class.configure_rspec
         RSpec.configure do |c|
           c.filter_run :quarantine
         end
@@ -234,7 +222,6 @@ RSpec.describe QA::Specs::Helpers::Quarantine do
 
     context 'with a non-quarantine tag focused' do
       before do
-        described_class.configure_rspec
         RSpec.configure do |c|
           c.filter_run :foo
         end
@@ -277,7 +264,6 @@ RSpec.describe QA::Specs::Helpers::Quarantine do
 
     context 'with :quarantine and non-quarantine tags focused' do
       before do
-        described_class.configure_rspec
         RSpec.configure do |c|
           c.filter_run :foo, :bar, :quarantine
         end

qa/spec/support/allure_metadata_formatter_spec.rb

+# frozen_string_literal: true
+
+describe QA::Support::AllureMetadataFormatter do
+  include Helpers::StubENV
+
+  let(:formatter) { described_class.new(StringIO.new) }
+
+  let(:rspec_example_notification) { double('RSpec::Core::Notifications::ExampleNotification', example: rspec_example) }
+  let(:rspec_example) do
+    double(
+      'RSpec::Core::Example',
+      tms: nil,
+      issue: nil,
+      add_link: nil,
+      attempts: 0,
+      file_path: 'file/path/spec.rb',
+      metadata: {
+        testcase: 'testcase',
+        quarantine: { issue: 'issue' }
+      }
+    )
+  end
+
+  let(:ci_job) { 'ee:relative 5' }
+  let(:ci_job_url) { 'url' }
+
+  before do
+    stub_env('CI', 'true')
+    stub_env('CI_JOB_NAME', ci_job)
+    stub_env('CI_JOB_URL', ci_job_url)
+  end
+
+  it "adds additional data to report" do
+    formatter.example_started(rspec_example_notification)
+
+    aggregate_failures do
+      expect(rspec_example).to have_received(:tms).with('Testcase', 'testcase')
+      expect(rspec_example).to have_received(:issue).with('Quarantine issue', 'issue')
+      expect(rspec_example).to have_received(:add_link).with(name: "Job(#{ci_job})", url: ci_job_url)
+      expect(rspec_example).to have_received(:issue).with(
+        'Failure issues',
+        'https://gitlab.com/gitlab-org/gitlab/-/issues?scope=all&state=opened&search=spec.rb'
+      )
+    end
+  end
+end