Merge branch 'jobs-to-rerun-failed-tests-from-results-fix-rules' into 'master'

Setup and test stage jobs detect and run failures from previous pipeline

See merge request gitlab-org/gitlab!72977

.gitignore

@@ -74,6 +74,7 @@ eslint-report.html
 /.gitlab_kas_secret
 /webpack-report/
 /crystalball/
+/test_results/
 /deprecations/
 /knapsack/
 /rspec_flaky/

.gitlab/ci/rails.gitlab-ci.yml

@@ -885,5 +885,24 @@ fail-pipeline-early:
     - install_gitlab_gem
   script:
     - fail_pipeline_early
+
+rspec rspec-pg12-rerun-previous-failed-tests:
+  extends:
+    - .rspec-base-pg12
+    - .rails:rules:rerun-previous-failed-tests
+  stage: test
+  needs: ["setup-test-env", "compile-test-assets", "detect-previous-failed-tests"]
+  script:
+    - !reference [.base-script, script]
+    - rspec_rerun_previous_failed_tests tmp/previous_failed_tests/rspec_failed_files.txt
+
+rspec rspec-ee-pg12-rerun-previous-failed-tests:
+  extends:
+    - "rspec rspec-pg12-rerun-previous-failed-tests"
+    - .rspec-ee-base-pg12
+  script:
+    - !reference [.base-script, script]
+    - rspec_rerun_previous_failed_tests tmp/previous_failed_tests/rspec_ee_failed_files.txt
+
 # EE: Canonical MR pipelines
 ##################################################

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

@@ -1198,6 +1198,18 @@
     - changes: *code-backstage-patterns
     - <<: *if-merge-request-labels-run-all-rspec
 
+.rails:rules:detect-previous-failed-tests:
+  rules:
+    - <<: *if-merge-request-labels-run-all-rspec
+    - <<: *if-merge-request
+      changes: *code-backstage-patterns
+
+.rails:rules:rerun-previous-failed-tests:
+  rules:
+    - <<: *if-merge-request-labels-run-all-rspec
+    - <<: *if-merge-request
+      changes: *code-backstage-patterns
+
 .rails:rules:rspec-foss-impact:
   rules:
     - <<: *if-not-ee

.gitlab/ci/setup.gitlab-ci.yml

@@ -102,6 +102,23 @@ detect-tests as-if-foss:
   before_script:
     - '[ "$FOSS_ONLY" = "1" ] && rm -rf ee/ qa/spec/ee/ qa/qa/specs/features/ee/ qa/qa/ee/ qa/qa/ee.rb'
 
+detect-previous-failed-tests:
+  extends:
+    - .detect-test-base
+    - .rails:rules:detect-previous-failed-tests
+  variables:
+    PREVIOUS_FAILED_TESTS_DIR: tmp/previous_failed_tests/
+    RSPEC_PG_REGEX: /rspec .+ pg12( .+)?/
+    RSPEC_EE_PG_REGEX: /rspec-ee .+ pg12( .+)?/
+  script:
+    - source ./scripts/utils.sh
+    - source ./scripts/rspec_helpers.sh
+    - retrieve_previous_failed_tests ${PREVIOUS_FAILED_TESTS_DIR} "${RSPEC_PG_REGEX}" "${RSPEC_EE_PG_REGEX}"
+  artifacts:
+    expire_in: 7d
+    paths:
+      - ${PREVIOUS_FAILED_TESTS_DIR}
+
 add-jh-folder:
   extends: .setup:rules:add-jh-folder
   image: ${GITLAB_DEPENDENCY_PROXY}alpine:edge

scripts/api/default_options.rb

@@ -9,3 +9,10 @@ module API
     endpoint: ENV['CI_API_V4_URL'] || 'https://gitlab.com/api/v4'
   }.freeze
 end
+
+module Host
+  DEFAULT_OPTIONS = {
+    instance_base_url: ENV['CI_SERVER_URL'],
+    mr_id: ENV['CI_MERGE_REQUEST_ID']
+  }.freeze
+end

scripts/failed_tests.rb

+#!/usr/bin/env ruby
+# frozen_string_literal: true
+
+require 'optparse'
+require 'fileutils'
+require 'uri'
+require 'json'
+require 'set'
+
+class FailedTests
+  def initialize(options)
+    @filename = options.delete(:previous_tests_report_path)
+    @output_directory = options.delete(:output_directory)
+    @rspec_pg_regex = options.delete(:rspec_pg_regex)
+    @rspec_ee_pg_regex = options.delete(:rspec_ee_pg_regex)
+  end
+
+  def output_failed_test_files
+    create_output_dir
+
+    failed_files_for_suite_collection.each do |suite_collection_name, suite_collection_files|
+      failed_test_files = suite_collection_files.map { |filepath| filepath.delete_prefix('./') }.join(' ')
+
+      output_file = File.join(output_directory, "#{suite_collection_name}_failed_files.txt")
+
+      File.open(output_file, 'w') do |file|
+        file.write(failed_test_files)
+      end
+    end
+  end
+
+  def failed_files_for_suite_collection
+    suite_map.each_with_object(Hash.new { |h, k| h[k] = Set.new }) do |(suite_collection_name, suite_collection_regex), hash|
+      failed_suites.each do |suite|
+        hash[suite_collection_name].merge(failed_files(suite)) if suite['name'] =~ suite_collection_regex
+      end
+    end
+  end
+
+  def suite_map
+    @suite_map ||= {
+      rspec: rspec_pg_regex,
+      rspec_ee: rspec_ee_pg_regex,
+      jest: /jest/
+    }
+  end
+
+  private
+
+  attr_reader :filename, :output_directory, :rspec_pg_regex, :rspec_ee_pg_regex
+
+  def file_contents
+    @file_contents ||= begin
+      File.read(filename)
+    rescue Errno::ENOENT
+      '{}'
+    end
+  end
+
+  def file_contents_as_json
+    @file_contents_as_json ||= begin
+      JSON.parse(file_contents)
+    rescue JSON::ParserError
+      {}
+    end
+  end
+
+  def failed_suites
+    return [] unless file_contents_as_json['suites']
+
+    file_contents_as_json['suites'].select { |suite| suite['failed_count'] > 0 }
+  end
+
+  def failed_files(suite)
+    return [] unless suite
+
+    suite['test_cases'].each_with_object([]) do |failure_hash, failed_cases|
+      failed_cases << failure_hash['file'] if failure_hash['status'] == 'failed'
+    end
+  end
+
+  def create_output_dir
+    return if File.directory?(output_directory)
+
+    puts 'Creating output directory...'
+    FileUtils.mkdir_p(output_directory)
+  end
+end
+
+if $0 == __FILE__
+  options = {
+    previous_tests_report_path: 'test_results/previous/test_reports.json',
+    output_directory: 'tmp/previous_failed_tests/',
+    rspec_pg_regex: /rspec .+ pg12( .+)?/,
+    rspec_ee_pg_regex: /rspec-ee .+ pg12( .+)?/
+  }
+
+  OptionParser.new do |opts|
+    opts.on("-p", "--previous-tests-report-path PREVIOUS_TESTS_REPORT_PATH", String, "Path of the file listing previous test failures") do |value|
+      options[:previous_tests_report_path] = value
+    end
+
+    opts.on("-o", "--output-directory OUTPUT_DIRECTORY", String, "Output directory for failed test files") do |value|
+      options[:output_directory] = value
+    end
+
+    opts.on("--rspec-pg-regex RSPEC_PG_REGEX", Regexp, "Regex to use when finding matching RSpec jobs") do |value|
+      options[:rspec_pg_regex] = value
+    end
+
+    opts.on("--rspec-ee-pg-regex RSPEC_EE_PG_REGEX", Regexp, "Regex to use when finding matching RSpec EE jobs") do |value|
+      options[:rspec_ee_pg_regex] = value
+    end
+
+    opts.on("-h", "--help", "Prints this help") do
+      puts opts
+      exit
+    end
+  end.parse!
+
+  FailedTests.new(options).output_failed_test_files
+end

scripts/pipeline_test_report_builder.rb

+#!/usr/bin/env ruby
+# frozen_string_literal: true
+
+require 'optparse'
+require 'time'
+require 'fileutils'
+require 'uri'
+require 'cgi'
+require 'net/http'
+require 'json'
+require_relative 'api/default_options'
+
+# Request list of pipelines for MR
+# https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab/merge_requests/69053/pipelines
+# Find latest failed pipeline
+# Retrieve list of failed builds for test stage in pipeline
+# https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab/pipelines/363788864/jobs/?scope=failed
+# Retrieve test reports for these builds
+# https://gitlab.com/gitlab-org/gitlab/-/pipelines/363788864/tests/suite.json?build_ids[]=1555608749
+# Push into expected format for failed tests
+class PipelineTestReportBuilder
+  def initialize(options)
+    @project = options.delete(:project)
+    @mr_id = options.delete(:mr_id) || Host::DEFAULT_OPTIONS[:mr_id]
+    @instance_base_url = options.delete(:instance_base_url) || Host::DEFAULT_OPTIONS[:instance_base_url]
+    @output_file_path = options.delete(:output_file_path)
+  end
+
+  def test_report_for_latest_pipeline
+    build_test_report_json_for_pipeline(previous_pipeline)
+  end
+
+  def execute
+    if output_file_path
+      FileUtils.mkdir_p(File.dirname(output_file_path))
+    end
+
+    File.open(output_file_path, 'w') do |file|
+      file.write(test_report_for_latest_pipeline)
+    end
+  end
+
+  private
+
+  attr_reader :project, :mr_id, :instance_base_url, :output_file_path
+
+  def project_api_base_url
+    "#{instance_base_url}/api/v4/projects/#{CGI.escape(project)}"
+  end
+
+  def project_base_url
+    "#{instance_base_url}/#{project}"
+  end
+
+  def previous_pipeline
+    # Top of the list will always be the current pipeline
+    # Second from top will be the previous pipeline
+    pipelines_for_mr.sort_by { |a| -Time.parse(a['created_at']).to_i }[1]
+  end
+
+  def pipelines_for_mr
+    fetch("#{project_api_base_url}/merge_requests/#{mr_id}/pipelines")
+  end
+
+  def failed_builds_for_pipeline(pipeline_id)
+    fetch("#{project_api_base_url}/pipelines/#{pipeline_id}/jobs?scope=failed&per_page=100")
+  end
+
+  # Method uses the test suite endpoint to gather test results for a particular build.
+  # Here we request individual builds, even though it is possible to supply multiple build IDs.
+  # The reason for this; it is possible to lose the job context and name when requesting multiple builds.
+  # Please see for more info: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69053#note_709939709
+  def test_report_for_build(pipeline_id, build_id)
+    fetch("#{project_base_url}/-/pipelines/#{pipeline_id}/tests/suite.json?build_ids[]=#{build_id}")
+  end
+
+  def build_test_report_json_for_pipeline(pipeline)
+    # empty file if no previous failed pipeline
+    return {}.to_json if pipeline.nil? || pipeline['status'] != 'failed'
+
+    test_report = {}
+
+    puts "Discovered last failed pipeline (#{pipeline['id']}) for MR!#{mr_id}"
+
+    failed_builds_for_test_stage = failed_builds_for_pipeline(pipeline['id']).select do |failed_build|
+      failed_build['stage'] == 'test'
+    end
+
+    puts "#{failed_builds_for_test_stage.length} failed builds in test stage found..."
+
+    if failed_builds_for_test_stage.any?
+      test_report['suites'] ||= []
+
+      failed_builds_for_test_stage.each do |failed_build|
+        test_report['suites'] << test_report_for_build(pipeline['id'], failed_build['id'])
+      end
+    end
+
+    test_report.to_json
+  end
+
+  def fetch(uri_str)
+    uri = URI(uri_str)
+
+    puts "URL: #{uri}"
+
+    request = Net::HTTP::Get.new(uri)
+
+    body = ''
+
+    Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http|
+      http.request(request) do |response|
+        case response
+        when Net::HTTPSuccess
+          body = response.read_body
+        else
+          raise "Unexpected response: #{response.value}"
+        end
+      end
+    end
+
+    JSON.parse(body)
+  end
+end
+
+if $0 == __FILE__
+  options = Host::DEFAULT_OPTIONS.dup
+
+  OptionParser.new do |opts|
+    opts.on("-p", "--project PROJECT", String, "Project where to find the merge request(defaults to $CI_PROJECT_ID)") do |value|
+      options[:project] = value
+    end
+
+    opts.on("-m", "--mr-id MR_ID", String, "A merge request ID") do |value|
+      options[:mr_id] = value
+    end
+
+    opts.on("-i", "--instance-base-url INSTANCE_BASE_URL", String, "URL of the instance where project and merge request resides") do |value|
+      options[:instance_base_url] = value
+    end
+
+    opts.on("-o", "--output-file-path OUTPUT_PATH", String, "A path for output file") do |value|
+      options[:output_file_path] = value
+    end
+
+    opts.on("-h", "--help", "Prints this help") do
+      puts opts
+      exit
+    end
+  end.parse!
+
+  PipelineTestReportBuilder.new(options).execute
+end

scripts/rspec_helpers.sh

@@ -89,6 +89,22 @@ function crystalball_rspec_data_exists() {
   compgen -G "crystalball/rspec*.yml" >/dev/null
 }
 
+function retrieve_previous_failed_tests() {
+  local directory_for_output_reports="${1}"
+  local rspec_pg_regex="${2}"
+  local rspec_ee_pg_regex="${3}"
+  local pipeline_report_path="test_results/previous/test_reports.json"
+  local project_path="gitlab-org/gitlab"
+
+  echo 'Attempting to build pipeline test report...'
+
+  scripts/pipeline_test_report_builder.rb --instance-base-url "https://gitlab.com" --project "${project_path}" --mr-id "${CI_MERGE_REQUEST_IID}" --output-file-path "${pipeline_report_path}"
+
+  echo 'Generating failed tests lists...'
+
+  scripts/failed_tests.rb --previous-tests-report-path "${pipeline_report_path}" --output-directory "${directory_for_output_reports}" --rspec-pg-regex "${rspec_pg_regex}" --rspec-ee-pg-regex "${rspec_ee_pg_regex}"
+}
+
 function rspec_simple_job() {
   local rspec_opts="${1}"
 
@@ -172,6 +188,25 @@ function rspec_paralellized_job() {
   date
 }
 
+function rspec_rerun_previous_failed_tests() {
+  local test_file_count_threshold=${RSPEC_PREVIOUS_FAILED_TEST_FILE_COUNT_THRESHOLD:-10}
+  local matching_tests_file=${1}
+  local rspec_opts=${2}
+  local test_files="$(cat "${matching_tests_file}")"
+  local test_file_count=$(wc -w "${matching_tests_file}" | awk {'print $1'})
+
+  if [[ "${test_file_count}" -gt "${test_file_count_threshold}" ]]; then
+    echo "This job is intentionally failed because there are more than ${test_file_count_threshold} test files to rerun."
+    exit 1
+  fi
+
+  if [[ -n $test_files ]]; then
+    rspec_simple_job "${test_files}"
+  else
+    echo "No failed test files to rerun"
+  fi
+}
+
 function rspec_fail_fast() {
   local test_file_count_threshold=${RSPEC_FAIL_FAST_TEST_FILE_COUNT_THRESHOLD:-10}
   local matching_tests_file=${1}

spec/fixtures/scripts/test_report.json

+{
+  "suites": [
+    {
+      "name": "rspec unit pg12",
+      "total_time": 975.6635620000018,
+      "total_count": 3811,
+      "success_count": 3800,
+      "failed_count": 1,
+      "skipped_count": 10,
+      "error_count": 0,
+      "suite_error": null,
+      "test_cases": [
+        {
+          "status": "failed",
+          "name": "Note associations is expected not to belong to project required: ",
+          "classname": "spec.models.note_spec",
+          "file": "./spec/models/note_spec.rb",
+          "execution_time": 0.209091,
+          "system_output": "Failure/Error: it { is_expected.not_to belong_to(:project) }\n  Did not expect Note to have a belongs_to association called project\n./spec/models/note_spec.rb:9:in `block (3 levels) in <top (required)>'\n./spec/spec_helper.rb:392:in `block (3 levels) in <top (required)>'\n./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'\n./spec/spec_helper.rb:383:in `block (2 levels) in <top (required)>'\n./spec/spec_helper.rb:379:in `block (3 levels) in <top (required)>'\n./lib/gitlab/application_context.rb:31:in `with_raw_context'\n./spec/spec_helper.rb:379:in `block (2 levels) in <top (required)>'\n./spec/support/database/prevent_cross_joins.rb:95:in `block (3 levels) in <top (required)>'\n./spec/support/database/prevent_cross_joins.rb:62:in `with_cross_joins_prevented'\n./spec/support/database/prevent_cross_joins.rb:95:in `block (2 levels) in <top (required)>'",
+          "stack_trace": null,
+          "recent_failures": null
+        },
+        {
+          "status": "success",
+          "name": "Gitlab::ImportExport yields the initial tree when importing and exporting it again",
+          "classname": "spec.lib.gitlab.import_export.import_export_equivalence_spec",
+          "file": "./spec/lib/gitlab/import_export/import_export_equivalence_spec.rb",
+          "execution_time": 17.084198,
+          "system_output": null,
+          "stack_trace": null,
+          "recent_failures": null
+        }
+      ]
+    }
+  ]
+}

spec/scripts/failed_tests_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_relative '../../scripts/failed_tests'
+
+RSpec.describe FailedTests do
+  let(:report_file) { 'spec/fixtures/scripts/test_report.json' }
+  let(:output_directory) { 'tmp/previous_test_results' }
+  let(:rspec_pg_regex) { /rspec .+ pg12( .+)?/ }
+  let(:rspec_ee_pg_regex) { /rspec-ee .+ pg12( .+)?/ }
+
+  subject { described_class.new(previous_tests_report_path: report_file, output_directory: output_directory, rspec_pg_regex: rspec_pg_regex, rspec_ee_pg_regex: rspec_ee_pg_regex) }
+
+  describe '#output_failed_test_files' do
+    it 'writes the file for the suite' do
+      expect(File).to receive(:open).with(File.join(output_directory, "rspec_failed_files.txt"), 'w').once
+
+      subject.output_failed_test_files
+    end
+  end
+
+  describe '#failed_files_for_suite_collection' do
+    let(:failure_path) { 'path/to/fail_file_spec.rb' }
+    let(:other_failure_path) { 'path/to/fail_file_spec_2.rb' }
+    let(:file_contents_as_json) do
+      {
+        'suites' => [
+          {
+            'failed_count' => 1,
+            'name' => 'rspec unit pg12 10/12',
+            'test_cases' => [
+              {
+                'status' => 'failed',
+                'file' => failure_path
+              }
+            ]
+          },
+          {
+            'failed_count' => 1,
+            'name' => 'rspec-ee unit pg12',
+            'test_cases' => [
+              {
+                'status' => 'failed',
+                'file' => failure_path
+              }
+            ]
+          },
+          {
+            'failed_count' => 1,
+            'name' => 'rspec unit pg13 10/12',
+            'test_cases' => [
+              {
+                'status' => 'failed',
+                'file' => other_failure_path
+              }
+            ]
+          }
+        ]
+      }
+    end
+
+    before do
+      allow(subject).to receive(:file_contents_as_json).and_return(file_contents_as_json)
+    end
+
+    it 'returns a list of failed file paths for suite collection' do
+      result = subject.failed_files_for_suite_collection
+
+      expect(result[:rspec].to_a).to match_array(failure_path)
+      expect(result[:rspec_ee].to_a).to match_array(failure_path)
+    end
+  end
+
+  describe 'empty report' do
+    let(:file_content) do
+      '{}'
+    end
+
+    before do
+      allow(subject).to receive(:file_contents).and_return(file_content)
+    end
+
+    it 'does not fail for output files' do
+      subject.output_failed_test_files
+    end
+
+    it 'returns empty results for suite failures' do
+      result = subject.failed_files_for_suite_collection
+
+      expect(result.values.flatten).to be_empty
+    end
+  end
+
+  describe 'invalid report' do
+    let(:file_content) do
+      ''
+    end
+
+    before do
+      allow(subject).to receive(:file_contents).and_return(file_content)
+    end
+
+    it 'does not fail for output files' do
+      subject.output_failed_test_files
+    end
+
+    it 'returns empty results for suite failures' do
+      result = subject.failed_files_for_suite_collection
+
+      expect(result.values.flatten).to be_empty
+    end
+  end
+
+  describe 'missing report file' do
+    let(:report_file) { 'unknownfile.json' }
+
+    it 'does not fail for output files' do
+      subject.output_failed_test_files
+    end
+
+    it 'returns empty results for suite failures' do
+      result = subject.failed_files_for_suite_collection
+
+      expect(result.values.flatten).to be_empty
+    end
+  end
+end

spec/scripts/pipeline_test_report_builder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_relative '../../scripts/pipeline_test_report_builder'
+
+RSpec.describe PipelineTestReportBuilder do
+  let(:report_file) { 'spec/fixtures/scripts/test_report.json' }
+  let(:output_file_path) { 'tmp/previous_test_results/output_file.json' }
+
+  subject do
+    described_class.new(
+      project: 'gitlab-org/gitlab',
+      mr_id: '999',
+      instance_base_url: 'https://gitlab.com',
+      output_file_path: output_file_path
+    )
+  end
+
+  let(:mr_pipelines) do
+    [
+      {
+        'status' => 'running',
+        'created_at' => DateTime.now.to_s
+      },
+      {
+        'status' => 'failed',
+        'created_at' => (DateTime.now - 5).to_s
+      }
+    ]
+  end
+
+  let(:failed_builds_for_pipeline) do
+    [
+      {
+        'id' => 9999,
+        'stage' => 'test'
+      }
+    ]
+  end
+
+  let(:test_report_for_build) do
+    {
+      "name": "rspec-ee system pg11 geo",
+      "failed_count": 41,
+      "test_cases": [
+        {
+          "status": "failed",
+          "name": "example",
+          "classname": "ee.spec.features.geo_node_spec",
+          "file": "./ee/spec/features/geo_node_spec.rb",
+          "execution_time": 6.324748,
+          "system_output": {
+            "__content__": "\n",
+            "message": "RSpec::Core::MultipleExceptionError",
+            "type": "RSpec::Core::MultipleExceptionError"
+          }
+        }
+      ]
+    }
+  end
+
+  before do
+    allow(subject).to receive(:pipelines_for_mr).and_return(mr_pipelines)
+    allow(subject).to receive(:failed_builds_for_pipeline).and_return(failed_builds_for_pipeline)
+    allow(subject).to receive(:test_report_for_build).and_return(test_report_for_build)
+  end
+
+  describe '#test_report_for_latest_pipeline' do
+    context 'no previous pipeline' do
+      let(:mr_pipelines) { [] }
+
+      it 'returns empty hash' do
+        expect(subject.test_report_for_latest_pipeline).to eq("{}")
+      end
+    end
+
+    context 'first pipeline scenario' do
+      let(:mr_pipelines) do
+        [
+          {
+            'status' => 'running',
+            'created_at' => DateTime.now.to_s
+          }
+        ]
+      end
+
+      it 'returns empty hash' do
+        expect(subject.test_report_for_latest_pipeline).to eq("{}")
+      end
+    end
+
+    context 'no previous failed pipeline' do
+      let(:mr_pipelines) do
+        [
+          {
+            'status' => 'running',
+            'created_at' => DateTime.now.to_s
+          },
+          {
+            'status' => 'success',
+            'created_at' => (DateTime.now - 5).to_s
+          }
+        ]
+      end
+
+      it 'returns empty hash' do
+        expect(subject.test_report_for_latest_pipeline).to eq("{}")
+      end
+    end
+
+    context 'no failed test builds' do
+      let(:failed_builds_for_pipeline) do
+        [
+          {
+            'id' => 9999,
+            'stage' => 'prepare'
+          }
+        ]
+      end
+
+      it 'returns empty hash' do
+        expect(subject.test_report_for_latest_pipeline).to eq("{}")
+      end
+    end
+
+    context 'failed pipeline and failed test builds' do
+      it 'returns populated test list for suites' do
+        actual = subject.test_report_for_latest_pipeline
+        expected = {
+          'suites' => [test_report_for_build]
+        }.to_json
+
+        expect(actual).to eq(expected)
+      end
+    end
+  end
+end