Merge branch 'qa-refactor-testcase-lint-script' into 'master'

Refactor E2E testcase linting

See merge request gitlab-org/gitlab!84218

.gitlab/ci/rules.gitlab-ci.yml

@@ -1373,6 +1373,10 @@
   rules:
     - changes: *code-backstage-qa-patterns
 
+.static-analysis:rules:ee-and-foss-qa:
+  rules:
+    - changes: *qa-patterns
+
 .static-analysis:rules:ee:
   rules:
     - <<: *if-not-ee

.gitlab/ci/static-analysis.gitlab-ci.yml

@@ -114,10 +114,27 @@ rubocop:
 qa:testcases:
   extends:
     - .static-analysis-base
-    - .rubocop-job-cache
-    - .static-analysis:rules:ee-and-foss
+    - .static-analysis:rules:ee-and-foss-qa
+  before_script:
+    - !reference [.default-before_script, before_script]
+    - cd qa/
+    - bundle_install_script
   script:
-    - run_timed_command "bundle exec rubocop qa/qa/specs/features/**/* --only QA/DuplicateTestcaseLink"
+    - run_timed_command "bundle exec bin/qa Test::Instance::All http://localhost:3000 --test-metadata-only"
+    - cd ..
+    - run_timed_command "./scripts/qa/testcases-check qa/tmp/test-metadata.json"
+  variables:
+    USE_BUNDLE_INSTALL: "false"
+    SETUP_DB: "false"
+    QA_EXPORT_TEST_METRICS: "false"
+    # Disable warnings in browserslist which can break on backports
+    # https://github.com/browserslist/browserslist/blob/a287ec6/node.js#L367-L384
+    BROWSERSLIST_IGNORE_OLD_DATA: "true"
+  artifacts:
+    expire_in: 31d
+    when: always
+    paths:
+      - qa/tmp/
 
 feature-flags-usage:
   extends:

.rubocop.yml

@@ -738,10 +738,6 @@ QA/SelectorUsage:
   Exclude:
     - 'spec/rubocop/**/*_spec.rb'
 
-QA/DuplicateTestcaseLink:
-  # this cop is executed in static-analysis.gitlab-ci.yml since it cannot be run in parallel
-  Enabled: false
-
 Performance/ActiveRecordSubtransactions:
   Exclude:
     - 'spec/**/*.rb'

qa/qa/scenario/bootable.rb

@@ -31,7 +31,7 @@ module QA
                 end
 
                 next
-              elsif opt.name == :count_examples_only
+              elsif opt.name == :count_examples_only || opt.name == :test_metadata_only
                 parser.on(opt.arg, opt.desc) do |value|
                   QA::Runtime::Env.dry_run = true
                   Runtime::Scenario.define(opt.name, value)

qa/qa/scenario/shared_attributes.rb

@@ -14,6 +14,7 @@ module QA
       attribute :parallel, '--parallel', 'Execute tests in parallel'
       attribute :loop, '--loop', 'Execute test repeatedly'
       attribute :count_examples_only, '--count-examples-only', 'Return the number of examples without running them'
+      attribute :test_metadata_only, '--test-metadata-only', 'Return all e2e test metadata without running them'
     end
   end
 end

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

@@ -155,7 +155,7 @@ module QA
       end
       # rubocop:enable RSpec/InstanceVariable
 
-      it "migrates large gitlab group via api" do
+      it "migrates large gitlab group via api", testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/358842' do
         start = Time.now
 
         # trigger import and log imported group path

qa/qa/specs/features/api/3_create/repository/commit_to_templated_project_spec.rb

@@ -10,7 +10,7 @@ module QA
         end
       end
 
-      it 'commits via the api' do
+      it 'commits via the api', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/357234' do
         expect do
           Resource::Repository::Commit.fabricate_via_api! do |commit|
             commit.project = project

qa/qa/specs/features/api/4_verify/remove_runner_spec.rb

@@ -16,7 +16,7 @@ module QA
       end
 
       # Removing a runner via the UI is covered by `spec/features/runners_spec.rb``
-      it 'removes the runner' do
+      it 'removes the runner', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/354828' do
         runners = nil
         expect { (runners = runner.list_of_runners(tag_list: runner_tags)).size }
           .to eventually_eq(1).within(max_duration: 10, sleep_interval: 1)

qa/qa/specs/features/browser_ui/4_verify/ci_variable/ui_variable_inheritable_when_forward_pipeline_variables_true_spec.rb

@@ -19,7 +19,7 @@ module QA
       it(
         'is inheritable when forward:pipeline_variables is true',
         :aggregate_failures,
-        test_case: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/358197'
+        testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/358197'
       ) do
         visit_job_page('child1', 'child1_job')
         verify_job_log_shows_variable_value

qa/qa/specs/features/browser_ui/4_verify/ci_variable/ui_variable_non_inheritable_when_forward_pipeline_variables_false_spec.rb

@@ -20,7 +20,7 @@ module QA
       it(
         'is not inheritable when forward:pipeline_variables is false',
         :aggregate_failures,
-        test_case: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/358199'
+        testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/358199'
       ) do
         visit_job_page('child1', 'child1_job')
         verify_job_log_does_not_show_variable_value
@@ -34,7 +34,7 @@ module QA
       it(
         'is not inheritable by default',
         :aggregate_failures,
-        test_case: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/358200'
+        testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/358200'
       ) do
         visit_job_page('child2', 'child2_job')
         verify_job_log_does_not_show_variable_value

qa/qa/specs/runner.rb

@@ -52,6 +52,8 @@ module QA
         tags_for_rspec
       end
 
+      # rubocop:disable Metrics/AbcSize
+      # rubocop:disable Metrics/CyclomaticComplexity
       def perform
         args = []
         args.push('--tty') if tty
@@ -89,12 +91,29 @@ module QA
           File.open(filename, 'w') { |f| f.write(total_examples) } if total_examples.to_i > 0
 
           $stdout.puts "Total examples in #{Runtime::Scenario.klass}: #{total_examples}#{total_examples.to_i > 0 ? ". Saved to file: #{filename}" : ''}"
+        elsif Runtime::Scenario.attributes[:test_metadata_only]
+          args.unshift('--dry-run')
+
+          output_file = Pathname.new(File.join(Runtime::Path.qa_root, 'tmp', 'test-metadata.json'))
+
+          RSpec.configure do |config|
+            config.add_formatter(QA::Support::JsonFormatter, output_file)
+            config.fail_if_no_examples = true
+          end
+
+          RSpec::Core::Runner.run(args.flatten, $stderr, $stdout) do |status|
+            abort if status.nonzero?
+          end
+
+          $stdout.puts "Saved to file: #{output_file}"
         else
           RSpec::Core::Runner.run(args.flatten, *DEFAULT_STD_ARGS).tap do |status|
             abort if status.nonzero?
           end
         end
       end
+      # rubocop:enable Metrics/AbcSize
+      # rubocop:enable Metrics/CyclomaticComplexity
 
       private
 

qa/spec/specs/runner_spec.rb

@@ -86,6 +86,41 @@ RSpec.describe QA::Specs::Runner do
       end
     end
 
+    context 'when test_metadata_only is set as an option' do
+      let(:rspec_config) { instance_double('RSpec::Core::Configuration') }
+      let(:output_file) { Pathname.new('/root/tmp/test-metadata.json') }
+
+      before do
+        QA::Runtime::Scenario.define(:test_metadata_only, true)
+        allow(RSpec).to receive(:configure).and_yield(rspec_config)
+        allow(rspec_config).to receive(:add_formatter)
+        allow(rspec_config).to receive(:fail_if_no_examples=)
+      end
+
+      it 'sets the `--dry-run` flag' do
+        expect_rspec_runner_arguments(['--dry-run', '--tag', '~orchestrated', '--tag', '~transient', '--tag', '~geo', *described_class::DEFAULT_TEST_PATH_ARGS], [$stderr, anything])
+
+        subject.perform
+      end
+
+      it 'configures json formatted output to file' do
+        allow(QA::Runtime::Path).to receive(:qa_root).and_return('/root')
+
+        expect(rspec_config).to receive(:add_formatter)
+          .with(QA::Support::JsonFormatter, output_file)
+        expect(rspec_config).to receive(:fail_if_no_examples=)
+          .with(true)
+
+        allow(RSpec::Core::Runner).to receive(:run).and_return(0)
+
+        subject.perform
+      end
+
+      after do
+        QA::Runtime::Scenario.attributes.delete(:test_metadata_only)
+      end
+    end
+
     context 'when tags are set' do
       subject { described_class.new.tap { |runner| runner.tags = %i[orchestrated github] } }
 

rubocop/cop/qa/duplicate_testcase_link.rb

-# frozen_string_literal: true
-
-module RuboCop
-  module Cop
-    module QA
-      # This cop checks for duplicate testcase links across e2e specs
-      #
-      # @example
-      #
-      #   # bad
-      #   it 'some test', testcase: '(...)/quality/test_cases/1892'
-      #   it 'another test, testcase: '(...)/quality/test_cases/1892'
-      #
-      #   # good
-      #   it 'some test', testcase: '(...)/quality/test_cases/1892'
-      #   it 'another test, testcase: '(...)/quality/test_cases/1894'
-      class DuplicateTestcaseLink < RuboCop::Cop::Cop
-        MESSAGE = "Don't reuse the same testcase link in different tests. Replace one of `%s`."
-
-        @testcase_set = Set.new
-
-        def_node_matcher :duplicate_testcase_link, <<~PATTERN
-          (block
-            (send nil? ...
-              ...
-              (hash
-                (pair
-                  (sym :testcase)
-                  (str $_))...)...)...)
-        PATTERN
-
-        def on_block(node)
-          duplicate_testcase_link(node) do |link|
-            break unless self.class.duplicate?(link)
-
-            add_offense(node, message: MESSAGE % link)
-          end
-        end
-
-        def self.duplicate?(link)
-          !@testcase_set.add?(link)
-        end
-      end
-    end
-  end
-end

rubocop/cop/qa/testcase_link_format.rb

-# frozen_string_literal: true
-
-require_relative '../../qa_helpers'
-
-module RuboCop
-  module Cop
-    module QA
-      # This cop checks for correct format of testcase links across e2e specs
-      #
-      # @example
-      #
-      #   # bad
-      #   it 'some test', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/557'
-      #   it 'another test, testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/2455'
-      #
-      #   # good
-      #   it 'some test', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/348312'
-      #   it 'another test, testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/348236'
-      class TestcaseLinkFormat < RuboCop::Cop::Cop
-        include QAHelpers
-
-        TESTCASE_FORMAT = %r{https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/\d+}.freeze
-        MESSAGE = "Testcase link format incorrect. Please link a test case from the GitLab project. See: https://docs.gitlab.com/ee/development/testing_guide/end_to_end/best_practices.html#link-a-test-to-its-test-case."
-
-        def_node_matcher :testcase_link_format, <<~PATTERN
-          (block
-            (send nil? ...
-              ...
-              (hash
-                (pair
-                  (sym :testcase)
-                  (str $_))...)...)...)
-        PATTERN
-
-        def on_block(node)
-          return unless in_qa_file?(node)
-
-          testcase_link_format(node) do |link|
-            add_offense(node, message: MESSAGE % link) unless TESTCASE_FORMAT =~ link
-          end
-        end
-      end
-    end
-  end
-end

scripts/qa/testcases-check

+#!/usr/bin/env ruby
+# frozen_string_literal: true
+
+require 'json'
+
+TESTCASE_FORMAT = %r{https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/\d+}.freeze
+
+testcases = []
+missing_testcases = []
+formatted_duplicates = []
+testcase_format_errors = []
+missing_message = %"\n*** The following tests are missing testcase links:\n\n%s\n"
+duplicate_message = %"\n*** The following tests have duplicate testcase links:\n\n%s"
+format_message = %"\n*** The following testcase links are incorrectly formatted:\n\n%s\n"
+
+test_metadata_file = ARGV.shift
+
+unless test_metadata_file
+  puts "usage: #{__FILE__} <test_metadata_file>"
+  exit 1
+end
+
+file = File.read(test_metadata_file)
+
+unless file =~ %r{.*\"examples\":\[\{\"id\"\:.*}
+  puts "\nRspec output did not match regex. Check test-metadata.json file.\n"
+  exit 1
+end
+
+puts "\nAnalyzing testcase data...\n"
+
+data_hash = JSON.parse(file)
+tests = data_hash['examples']
+
+tests.each do |test|
+  next if test['id'] =~ %r{.\/qa\/specs\/features\/sanity\/*}
+
+  if test['testcase']
+    testcases.push(["#{test['testcase']}", "#{test['id']} - #{test['full_description']}"])
+
+    unless TESTCASE_FORMAT =~ test['testcase']
+      testcase_format_errors.push(
+        <<~FORMAT_ERRORS
+           ==> #{test['testcase']} in file: #{test['id']} with title:
+               #{test['full_description']}
+        FORMAT_ERRORS
+      )
+    end
+  else
+    missing_testcases.push(" ==> #{test['id']} - #{test['full_description']}\n")
+  end
+end
+
+testcase_list = testcases.group_by {|testcase| testcase.shift}.transform_values(&:flatten)
+
+duplicates = testcase_list.select {|k, v| v.count > 1}
+
+unless duplicates.empty?
+  duplicates.each do |duplicate|
+    formatted_duplicates.append(
+      <<~DUPLICATES
+        Testcase link #{duplicate[0]} is used in too many tests:
+         ==> #{duplicate[1].join("\n ==> ")}\n
+      DUPLICATES
+    )
+  end
+end
+
+if formatted_duplicates.empty? && missing_testcases.empty? && testcase_format_errors.empty?
+  puts "\nNo errors found."
+else
+  puts "\n*** Testcase link violations detected! ***\n"
+  puts duplicate_message % formatted_duplicates.join("\n") unless formatted_duplicates.empty?
+  puts missing_message % missing_testcases.join("\n") unless missing_testcases.empty?
+  puts format_message % testcase_format_errors.join("\n") unless testcase_format_errors.empty?
+  puts "\n*** Please link a unique test case from the GitLab project for the errors listed above.\n"
+  puts "    See: https://docs.gitlab.com/ee/development/testing_guide/end_to_end/best_practices.html#link-a-test-to-its-test-case."
+  exit 1
+end

spec/rubocop/cop/qa/duplicate_testcase_link_spec.rb

-# frozen_string_literal: true
-
-require 'fast_spec_helper'
-
-require_relative '../../../../rubocop/cop/qa/duplicate_testcase_link'
-
-RSpec.describe RuboCop::Cop::QA::DuplicateTestcaseLink do
-  let(:source_file) { 'qa/page.rb' }
-
-  subject(:cop) { described_class.new }
-
-  context 'in a QA file' do
-    before do
-      allow(cop).to receive(:in_qa_file?).and_return(true)
-    end
-
-    it "registers an offense for a duplicate testcase link" do
-      expect_offense(<<-RUBY)
-        it 'some test', testcase: '/quality/test_cases/1892' do
-        end
-        it 'another test', testcase: '/quality/test_cases/1892' do
-        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't reuse the same testcase link in different tests. Replace one of `/quality/test_cases/1892`.
-        end
-      RUBY
-    end
-
-    it "doesnt offend if testcase link is unique" do
-      expect_no_offenses(<<-RUBY)
-        it 'some test', testcase: '/quality/test_cases/1893' do
-        end
-        it 'another test', testcase: '/quality/test_cases/1894' do
-        end
-      RUBY
-    end
-  end
-end

spec/rubocop/cop/qa/testcase_link_format_spec.rb

-# frozen_string_literal: true
-
-require 'fast_spec_helper'
-
-require_relative '../../../../rubocop/cop/qa/testcase_link_format'
-
-RSpec.describe RuboCop::Cop::QA::TestcaseLinkFormat do
-  let(:source_file) { 'qa/page.rb' }
-  let(:msg) { 'Testcase link format incorrect. Please link a test case from the GitLab project. See: https://docs.gitlab.com/ee/development/testing_guide/end_to_end/best_practices.html#link-a-test-to-its-test-case.' }
-
-  subject(:cop) { described_class.new }
-
-  context 'in a QA file' do
-    before do
-      allow(cop).to receive(:in_qa_file?).and_return(true)
-    end
-
-    it "registers an offense for a testcase link for an issue" do
-      node = "it 'another test', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/issues/557' do"
-
-      expect_offense(<<-RUBY, node: node, msg: msg)
-        %{node}
-        ^{node} %{msg}
-        end
-      RUBY
-    end
-
-    it "registers an offense for a testcase link for the wrong project" do
-      node = "it 'another test', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/2455' do"
-
-      expect_offense(<<-RUBY, node: node, msg: msg)
-        %{node}
-        ^{node} %{msg}
-        end
-      RUBY
-    end
-
-    it "doesnt offend if testcase link is correct" do
-      expect_no_offenses(<<-RUBY)
-        it 'some test', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/348312' do
-        end
-      RUBY
-    end
-  end
-end