From 388228c8fe41bab463f36c34c0b936e27bf44f48 Mon Sep 17 00:00:00 2001
From: Andrejs Cunskis <acunskis@gitlab.com>
Date: Tue, 1 Oct 2024 01:26:21 +0000
Subject: [PATCH] Prevent coverband from starting with default config
Improve error messaging in tests
Remove unnecessary CI variable definition
Fetch only runtime coverage data
---
.gitlab/ci/test-on-gdk/main.gitlab-ci.yml | 2 +-
config/coverband.rb | 15 +++
config/initializers/coverband.rb | 16 +---
lib/api/internal/coverage.rb | 10 +-
.../support/formatters/coverband_formatter.rb | 58 ++++++------
.../formatters/coverband_formatter_spec.rb | 93 ++++++++-----------
spec/requests/api/internal/coverage_spec.rb | 6 +-
7 files changed, 102 insertions(+), 98 deletions(-)
create mode 100644 config/coverband.rb
diff --git a/.gitlab/ci/test-on-gdk/main.gitlab-ci.yml b/.gitlab/ci/test-on-gdk/main.gitlab-ci.yml
index 7b3b115ad1a81..b182643850812 100644
--- a/.gitlab/ci/test-on-gdk/main.gitlab-ci.yml
+++ b/.gitlab/ci/test-on-gdk/main.gitlab-ci.yml
@@ -83,7 +83,7 @@ include:
GITLAB_QA_ADMIN_ACCESS_TOKEN: $QA_ADMIN_ACCESS_TOKEN
FF_NETWORK_PER_BUILD: "true"
RSPEC_LAST_RUN_RESULTS_FILE: "$CI_PROJECT_DIR/qa/tmp/examples.txt"
- COVERBAND_ENABLED: "$COVERBAND_ENABLED"
+ COVERBAND_ENABLED: "$COVERBAND_ENABLED" # explicitly define variable so it is passed in to gdk service container
before_script:
- echo "SUITE_RAN=true" > "$QA_SUITE_STATUS_ENV_FILE"
- echo -e "\e[0Ksection_start:`date +%s`:install_gems[collapsed=true]\r\e[0KInstall gems"
diff --git a/config/coverband.rb b/config/coverband.rb
new file mode 100644
index 0000000000000..8cf99014f569d
--- /dev/null
+++ b/config/coverband.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+# This file is loaded from 'initializers/coverband.rb' if coverband is enabled
+
+Coverband.configure do |config|
+ config.store = Coverband::Adapters::RedisStore.new(Gitlab::Redis::SharedState.redis)
+ config.background_reporting_sleep_seconds = 1
+ config.reporting_wiggle = 0 # Since this is not run in production disable wiggle and report every second.
+ config.ignore += %w[spec/.* lib/tasks/.*
+ config/application.rb config/boot.rb config/initializers/.* db/post_migrate/.*
+ config/puma.rb bin/.* config/environments/.* db/migrate/.*]
+
+ config.verbose = false # this spams logfile a lot, set to true for debugging locally
+ config.logger = Gitlab::AppLogger.primary_logger
+end
diff --git a/config/initializers/coverband.rb b/config/initializers/coverband.rb
index aed72a3ff61e0..98ef0c08e36b8 100644
--- a/config/initializers/coverband.rb
+++ b/config/initializers/coverband.rb
@@ -3,17 +3,7 @@
# Configuration used by coverband gem when "COVERBAND_ENABLED" is set to "true"
return unless Gitlab::Utils.to_boolean(ENV['COVERBAND_ENABLED'], default: false)
-require 'coverband'
-
-Coverband.configure do |config|
- config.store = Coverband::Adapters::RedisStore.new(Gitlab::Redis::SharedState.redis)
- config.background_reporting_sleep_seconds = 1
- config.reporting_wiggle = nil # Since this is not run in production disable wiggle and report every second.
- config.ignore += %w[spec/.* lib/tasks/.*
- config/application.rb config/boot.rb config/initializers/.* db/post_migrate/.*
- config/puma.rb bin/.* config/environments/.* db/migrate/.*]
+# Set coverband config file name so it is not started with default configuration
+ENV["COVERBAND_CONFIG"] = Rails.root.join("config/coverband.rb").to_s
- config.verbose = true
- config.csp_policy = true
- config.logger = Gitlab::AppLogger
-end
+require 'coverband'
diff --git a/lib/api/internal/coverage.rb b/lib/api/internal/coverage.rb
index cba50280e5d6e..e2fbbd9897d7c 100644
--- a/lib/api/internal/coverage.rb
+++ b/lib/api/internal/coverage.rb
@@ -22,12 +22,18 @@ class Coverage < ::API::Base
end
get do
- coverage = ::Coverband.configuration.store.coverage
- coverage&.keys
+ # Fetch only runtime coverage data which is tracked during E2E spec execution
+ # skip hash check due to hash mismatch on some environments which results in empty coverage data
+ ::Coverband.configuration.store.coverage(::Coverband::RUNTIME_TYPE, skip_hash_check: true).keys
end
delete do
::Coverband.configuration.store.clear!
+
+ status 200
+ {
+ message: "Cleared source code paths coverage mapping"
+ }
end
end
end
diff --git a/qa/qa/support/formatters/coverband_formatter.rb b/qa/qa/support/formatters/coverband_formatter.rb
index d4c669744d1f7..2e3659e59d6e5 100644
--- a/qa/qa/support/formatters/coverband_formatter.rb
+++ b/qa/qa/support/formatters/coverband_formatter.rb
@@ -9,13 +9,13 @@ module Formatters
class CoverbandFormatter < ::RSpec::Core::Formatters::BaseFormatter
include Support::API
- COVERAGE_API_PATH = '/api/v4/internal/coverage'
-
def initialize(output)
super
+
@test_mapping = Hash.new { |hsh, key| hsh[key] = [] }
@logger = Runtime::Logger.logger
@headers_access_token = { "PRIVATE-TOKEN" => Runtime::Env.admin_personal_access_token }
+ @cov_api_endpoint = "#{Runtime::Scenario.gitlab_address}/api/v4/internal/coverage"
end
::RSpec::Core::Formatters.register(
@@ -30,45 +30,49 @@ def initialize(output)
# @param [RSpec::Core::Notifications::ExamplesNotification] notification
# @return [void]
def stop(_notification)
- logger.info("Saving test coverage mapping json file")
-
save_test_mapping
end
# Example start event
- def example_started(_example_notification)
- QA::Support::Retrier.retry_until(max_attempts: 5, sleep_interval: 1, message: "Retry clear coverage") do
- resp = delete("#{Runtime::Scenario.gitlab_address}#{COVERAGE_API_PATH}",
- headers: headers_access_token)
+ def example_started(example_notification)
+ return if example_notification.example.metadata[:skip]
- logger.error("Failed to clear coverage, code: #{resp.code}, body: #{resp.body}") if resp.code != 200
+ response = nil
+ QA::Support::Retrier.retry_until(max_attempts: 5, sleep_interval: 1) do
+ response = delete(cov_api_endpoint, headers: headers_access_token)
+ next true if response.code == 200
- resp.code == 200
+ logger.debug("Failed to clear coverage, code: #{response.code}, body: #{response.body}")
+ false
end
- logger.debug("Cleared coverage data before example starts")
- rescue StandardError => e
- logger.error("Failed to clear coverage. Exception trace: #{e}")
+ logger.info("Cleared coverage data")
+ rescue StandardError
+ logger.error("Failed to clear coverage, code: #{response.code}, body: #{response.body}")
end
# Example finish event
def example_finished(example_notification)
- cov_resp = nil
- QA::Support::Retrier.retry_until(max_attempts: 10, sleep_interval: 2, message: "Retry fetch coverage") do
- cov_resp = get("#{Runtime::Scenario.gitlab_address}/api/v4/internal/coverage",
- headers: headers_access_token)
+ return if example_notification.example.metadata[:skip] || example_failed?(example_notification)
+
+ response = nil
+ QA::Support::Retrier.retry_until(max_attempts: 10, sleep_interval: 2) do
+ response = get(cov_api_endpoint, headers: headers_access_token)
+ coverage = JSON.parse(response.body)
+ next true if response.code == 200 && coverage.any?
- if cov_resp.code != 200
- logger.error("Fetching coverage data failed, code: #{cov_resp.code}, body: #{cov_resp.body}")
+ if response.code != 200
+ logger.debug("Fetching coverage data failed, code: #{response.code}, body: #{response.body}")
end
- cov_resp.code == 200 && !JSON.parse(cov_resp.body).empty?
+ logger.debug("Fetching coverage data failed, no coverage data available") if coverage.empty?
+ false
end
example_path = example_notification.example.metadata[:location]
- test_mapping[example_path] = JSON.parse(cov_resp.body) unless example_failed?(example_notification)
- logger.debug("Coverage paths were stored in mapping hash")
- rescue StandardError => e
- logger.error("Failed to fetch coverage mapping. Trace: #{e}")
+ test_mapping[example_path] = JSON.parse(response.body)
+ logger.info("Fetched coverage data")
+ rescue StandardError
+ logger.error("Failed to fetch coverage data, code: #{response.code}, body: #{response.body}")
end
def example_failed?(example_notification)
@@ -83,14 +87,14 @@ def save_test_mapping
# To write two different files in case of failed specs being retried
File.write(file, test_mapping.to_json)
- logger.debug("Saved test code paths mapping to #{file}")
+ logger.info("Saved test coverage mapping data to #{file}")
rescue StandardError => e
- logger.error("Failed to save test code paths mapping, error: #{e}")
+ logger.error("Failed to save test coverage mapping data, error: #{e}")
end
private
- attr_reader :test_mapping, :logger, :headers_access_token
+ attr_reader :test_mapping, :logger, :headers_access_token, :cov_api_endpoint
end
end
end
diff --git a/qa/spec/support/formatters/coverband_formatter_spec.rb b/qa/spec/support/formatters/coverband_formatter_spec.rb
index a10a44a65f557..cbc1fad8a7ce1 100644
--- a/qa/spec/support/formatters/coverband_formatter_spec.rb
+++ b/qa/spec/support/formatters/coverband_formatter_spec.rb
@@ -9,10 +9,8 @@
instance_double(RSpec::Core::Notifications::ExampleNotification, example: rspec_example)
end
- # rubocop:disable RSpec/VerifiedDoubles -- Custom object
-
let(:rspec_example) do
- double(
+ instance_double(
RSpec::Core::Example,
file_path: 'create_issue_spec.rb',
execution_result: instance_double(RSpec::Core::Example::ExecutionResult, status: status),
@@ -24,22 +22,14 @@
)
end
- # rubocop:enable RSpec/VerifiedDoubles
-
let(:gitlab_address) { 'http://gitlab.test.com' }
let(:api_path) { "#{gitlab_address}/api/v4/internal/coverage" }
-
let(:status) { :failed }
let(:token_header) { { "PRIVATE-TOKEN" => 'token' } }
- let(:api_response) do
- instance_double(
- ::RestClient::Response
- )
- end
-
- let(:non_empty_response) do
- '{"test mapping":1}'
- end
+ let(:logger) { instance_double(ActiveSupport::Logger, debug: true, error: true, info: true) }
+ let(:api_response) { instance_double(::RestClient::Response, code: code, body: body) }
+ let(:code) { 200 }
+ let(:body) { '[]' }
let(:mapping) do
{ "./qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb:5": { "test mapping": 1 } }
@@ -47,108 +37,103 @@
before do
stub_env('GITLAB_QA_ADMIN_ACCESS_TOKEN', 'token')
+
+ allow(QA::Runtime::Logger).to receive(:logger).and_return(logger)
+ allow(QA::Runtime::Scenario).to receive(:gitlab_address).and_return(gitlab_address)
+ allow(::RestClient::Request).to receive(:execute).and_return(api_response)
end
context 'with example_started' do
- before do
- allow(QA::Runtime::Scenario).to receive(:gitlab_address).and_return(gitlab_address)
- allow(api_response).to receive(:body).and_return({})
- allow(::RestClient::Request).to receive(:execute).and_return(api_response)
- end
-
context 'when success response' do
- before do
- allow(api_response).to receive(:code).and_return(200)
- end
-
it 'logs coverage cleared', :aggregate_failures do
- expect(QA::Runtime::Logger.logger).to receive(:debug).with("Cleared coverage data before example starts").once
formatter.example_started(rspec_example_notification)
+
+ expect(logger).to have_received(:info).with("Cleared coverage data").once
end
end
context 'with failure response' do
+ let(:code) { 401 }
+
before do
- allow(api_response).to receive(:code).and_return(401)
- allow(QA::Support::Retrier).to receive(:retry_until).and_wrap_original do |method|
- method.call(max_attempts: 1)
+ allow(QA::Support::Retrier).to receive(:retry_until).and_wrap_original do |method, *_args, &block|
+ method.call(max_attempts: 1, &block)
end
end
it 'logs error message' do
- expect(QA::Runtime::Logger.logger).to receive(:error).with(/Failed to clear coverage.*/).once
formatter.example_started(rspec_example_notification)
+
+ expect(logger).to have_received(:error).with("Failed to clear coverage, code: #{code}, body: #{body}").once
end
end
end
context 'when example finished' do
- before do
- allow(QA::Runtime::Scenario).to receive(:gitlab_address).and_return(gitlab_address)
- allow(::RestClient::Request).to receive(:execute).and_return(api_response)
- end
-
context 'with success response and non empty coverage' do
let(:status) { :passed }
-
- before do
- allow(api_response).to receive(:code).and_return(200)
- allow(api_response).to receive(:body).and_return(non_empty_response)
- end
+ let(:body) { '{"test mapping":1}' }
it 'logs success message and does not log any errors' do
- expect(QA::Runtime::Logger.logger).not_to receive(:error)
- expect(QA::Runtime::Logger.logger).to receive(:debug).with("Coverage paths were stored in mapping hash").once
formatter.example_finished(rspec_example_notification)
+
+ expect(logger).not_to have_received(:error)
+ expect(logger).to have_received(:info).with("Fetched coverage data").once
end
end
context 'with failure response' do
let(:status) { :passed }
+ let(:code) { 401 }
before do
- allow(api_response).to receive(:code).and_return(401)
- allow(api_response).to receive(:body).and_return({})
- allow(QA::Support::Retrier).to receive(:retry_until).and_wrap_original do |method|
- method.call(max_attempts: 1)
+ allow(QA::Support::Retrier).to receive(:retry_until).and_wrap_original do |method, *_args, &block|
+ method.call(max_attempts: 1, &block)
end
end
it 'logs error message' do
- expect(QA::Runtime::Logger.logger).to receive(:error).with(/Failed to fetch coverage mapping.*/).once
formatter.example_finished(rspec_example_notification)
+
+ expect(logger).to have_received(:error)
+ .with("Failed to fetch coverage data, code: #{code}, body: #{body}")
+ .once
end
end
end
context 'when save_test_mapping is called' do
+ let(:file_name_pattern) { "test-code-paths-mapping-job-name" }
+
before do
stub_env('CI_JOB_NAME_SLUG', 'job-name')
end
- let(:file_name_pattern) { "test-code-paths-mapping-job-name" }
-
context 'with mapping data present' do
before do
allow(formatter).to receive(:test_mapping).and_return(mapping)
+ allow(::File).to receive(:write)
end
it 'writes to file' do
- expect(::File).to receive(:write).with(/#{file_name_pattern}/, mapping.to_json).once
- expect(QA::Runtime::Logger.logger).to receive(:debug).with(/Saved test code paths mapping to.*/).once
formatter.save_test_mapping
+
+ expect(::File).to have_received(:write).with(/#{file_name_pattern}/, mapping.to_json).once
+ expect(logger).to have_received(:info).with(/Saved test coverage mapping data to \S+\.json/).once
end
end
context 'when writing to file throws an error' do
before do
- allow(::File).to receive(:write).and_raise(StandardError)
+ allow(::File).to receive(:write).and_raise("some error")
end
it 'raises an error' do
- expect(QA::Runtime::Logger.logger).to receive(:error)
- .with(/Failed to save test code paths mapping, error:.*/).once
formatter.save_test_mapping
+
+ expect(logger).to have_received(:error)
+ .with("Failed to save test coverage mapping data, error: some error")
+ .once
end
end
end
diff --git a/spec/requests/api/internal/coverage_spec.rb b/spec/requests/api/internal/coverage_spec.rb
index 10731231761df..82862567a2a37 100644
--- a/spec/requests/api/internal/coverage_spec.rb
+++ b/spec/requests/api/internal/coverage_spec.rb
@@ -36,8 +36,12 @@
before do
stub_const('Coverband', Class.new)
- allow(Coverband).to receive_message_chain(:configuration, :store, :coverage).and_return(coverage_hash)
+ stub_const('Coverband::RUNTIME_TYPE', :runtime)
+
allow(Coverband).to receive_message_chain(:configuration, :store, :clear!).and_return({})
+ allow(Coverband).to receive_message_chain(:configuration, :store, :coverage)
+ .with(:runtime, skip_hash_check: true)
+ .and_return(coverage_hash)
end
it 'GET returns 200', :aggregate_failures do
--
GitLab