Merge branch '62151-broken-master' into 'master'

Fix MySQL CI jobs

Closes #62156 and #62151

See merge request gitlab-org/gitlab-ce!28593
This commit is contained in:
Kamil Trzciński 2019-05-24 15:34:54 +00:00
commit 21c809ce22
18 changed files with 336 additions and 67 deletions

View file

@ -13,10 +13,8 @@ variables:
BUILD_ASSETS_IMAGE: "false"
before_script:
- bundle --version
- date
- source scripts/utils.sh
- date
- source scripts/prepare_build.sh
- date

View file

@ -6,8 +6,9 @@
.use-pg-10: &use-pg-10
services:
- postgres:10.7
- redis:alpine
- name: postgres:10.7
command: ["postgres", "-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"]
- name: redis:alpine
.use-mysql: &use-mysql
services:
@ -52,8 +53,10 @@
script:
- JOB_NAME=( $CI_JOB_NAME )
- TEST_TOOL=${JOB_NAME[0]}
- export KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${TEST_TOOL}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json
- export KNAPSACK_GENERATE_REPORT=true
- TEST_LEVEL=${JOB_NAME[1]}
- DATABASE=${JOB_NAME[2]}
- export KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${TEST_TOOL}_${TEST_LEVEL}_${DATABASE}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json
- export KNAPSACK_GENERATE_REPORT=true KNAPSACK_LOG_LEVEL=debug KNAPSACK_TEST_DIR=spec
- export SUITE_FLAKY_RSPEC_REPORT_PATH=${FLAKY_RSPEC_SUITE_REPORT_PATH}
- export FLAKY_RSPEC_REPORT_PATH=rspec_flaky/all_${TEST_TOOL}_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json
- export NEW_FLAKY_RSPEC_REPORT_PATH=rspec_flaky/new_${TEST_TOOL}_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json
@ -63,7 +66,10 @@
- '[[ -f $FLAKY_RSPEC_REPORT_PATH ]] || echo "{}" > ${FLAKY_RSPEC_REPORT_PATH}'
- '[[ -f $NEW_FLAKY_RSPEC_REPORT_PATH ]] || echo "{}" > ${NEW_FLAKY_RSPEC_REPORT_PATH}'
- scripts/gitaly-test-spawn
- knapsack rspec "--color --format documentation --format RspecJunitFormatter --out junit_rspec.xml"
- date
- 'export KNAPSACK_TEST_FILE_PATTERN=$(ruby -r./lib/quality/test_level.rb -e "puts Quality::TestLevel.new.pattern(:${TEST_LEVEL})")'
- knapsack rspec "--color --format documentation --format RspecJunitFormatter --out junit_rspec.xml --tag level:${TEST_LEVEL} --tag ~geo"
- date
artifacts:
expire_in: 31d
when: always
@ -140,19 +146,68 @@ setup-test-env:
except:
- /(^docs[\/-].*|.*-docs$)/
rspec-pg:
rspec unit pg:
<<: *rspec-metadata-pg
parallel: 50
parallel: 20
rspec-pg-10:
rspec integration pg:
<<: *rspec-metadata-pg
parallel: 6
rspec system pg:
<<: *rspec-metadata-pg
parallel: 24
rspec unit pg-10:
<<: *rspec-metadata-pg-10
<<: *only-schedules-master
parallel: 50
parallel: 20
rspec-mysql:
rspec integration pg-10:
<<: *rspec-metadata-pg-10
<<: *only-schedules-master
parallel: 6
rspec system pg-10:
<<: *rspec-metadata-pg-10
<<: *only-schedules-master
parallel: 24
rspec unit mysql:
<<: *rspec-metadata-mysql
<<: *only-schedules-master
parallel: 50
parallel: 20
rspec integration mysql:
<<: *rspec-metadata-mysql
<<: *only-schedules-master
parallel: 6
rspec system mysql:
<<: *rspec-metadata-mysql
<<: *only-schedules-master
parallel: 24
.rspec-mysql-on-demand: &rspec-mysql-on-demand
only:
variables:
- $CI_COMMIT_MESSAGE =~ /\[run mysql\]/i
- $CI_COMMIT_REF_NAME =~ /mysql/
rspec unit mysql on-demand:
<<: *rspec-metadata-mysql
<<: *rspec-mysql-on-demand
parallel: 20
rspec integration mysql on-demand:
<<: *rspec-metadata-mysql
<<: *rspec-mysql-on-demand
parallel: 6
rspec system mysql on-demand:
<<: *rspec-metadata-mysql
<<: *rspec-mysql-on-demand
parallel: 24
rspec-fast-spec-helper:
<<: *rspec-metadata-pg
@ -164,16 +219,17 @@ rspec-fast-spec-helper:
script:
- export CACHE_CLASSES=true
- scripts/gitaly-test-spawn
- bin/rspec --color --format documentation --tag quarantine spec/
- bin/rspec --color --format documentation --tag quarantine -- spec/
rspec-pg-quarantine:
rspec quarantine pg:
<<: *rspec-metadata-pg
<<: *rspec-quarantine
allow_failure: true
rspec-mysql-quarantine:
rspec quarantine mysql:
<<: *rspec-metadata-mysql
<<: *rspec-quarantine
<<: *only-schedules-master
allow_failure: true
static-analysis:

View file

@ -40,12 +40,12 @@ update-tests-metadata:
policy: push
script:
- retry gem install fog-aws mime-types activesupport rspec_profiling postgres-copy --no-document
- scripts/merge-reports ${KNAPSACK_RSPEC_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/rspec-pg_node_*.json
- scripts/merge-reports ${KNAPSACK_RSPEC_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/rspec_*_pg_node_*.json
- '[[ -z ${TESTS_METADATA_S3_BUCKET} ]] || scripts/sync-reports put $TESTS_METADATA_S3_BUCKET $KNAPSACK_RSPEC_SUITE_REPORT_PATH'
- rm -f knapsack/${CI_PROJECT_NAME}/*_node_*.json
- scripts/merge-reports ${FLAKY_RSPEC_SUITE_REPORT_PATH} rspec_flaky/all_*_*.json
- FLAKY_RSPEC_GENERATE_REPORT=1 scripts/prune-old-flaky-specs ${FLAKY_RSPEC_SUITE_REPORT_PATH}
- '[[ -z ${TESTS_METADATA_S3_BUCKET} ]] || scripts/sync-reports put $TESTS_METADATA_S3_BUCKET $KNAPSACK_RSPEC_SUITE_REPORT_PATH'
- '[[ -z ${TESTS_METADATA_S3_BUCKET} ]] || scripts/sync-reports put $TESTS_METADATA_S3_BUCKET $FLAKY_RSPEC_SUITE_REPORT_PATH'
- rm -f knapsack/${CI_PROJECT_NAME}/*_node_*.json
- rm -f rspec_flaky/all_*.json rspec_flaky/new_*.json
- scripts/insert-rspec-profiling-data
only:

View file

@ -361,7 +361,7 @@ group :development, :test do
gem 'scss_lint', '~> 0.56.0', require: false
gem 'haml_lint', '~> 0.31.0', require: false
gem 'simplecov', '~> 0.14.0', require: false
gem 'simplecov', '~> 0.16.1', require: false
gem 'bundler-audit', '~> 0.5.0', require: false
gem 'benchmark-ips', '~> 2.3.0', require: false

View file

@ -182,7 +182,7 @@ GEM
diffy (3.1.0)
discordrb-webhooks-blackst0ne (3.3.0)
rest-client (~> 2.0)
docile (1.1.5)
docile (1.3.1)
domain_name (0.5.20180417)
unf (>= 0.0.5, < 1.0.0)
doorkeeper (4.3.2)
@ -883,11 +883,11 @@ GEM
jwt (>= 1.5, < 3.0)
multi_json (~> 1.10)
simple_po_parser (1.1.2)
simplecov (0.14.1)
docile (~> 1.1.0)
simplecov (0.16.1)
docile (~> 1.1)
json (>= 1.8, < 3)
simplecov-html (~> 0.10.0)
simplecov-html (0.10.0)
simplecov-html (0.10.2)
slack-notifier (1.5.1)
spring (2.0.2)
activesupport (>= 4.2)
@ -1215,7 +1215,7 @@ DEPENDENCIES
sidekiq (~> 5.2.7)
sidekiq-cron (~> 1.0)
simple_po_parser (~> 1.1.2)
simplecov (~> 0.14.0)
simplecov (~> 0.16.1)
slack-notifier (~> 1.5.1)
spring (~> 2.0.0)
spring-commands-rspec (~> 1.0.4)

View file

@ -108,11 +108,13 @@ To make sure that indices still fit. You could find great details in:
In order to run the test you can use the following commands:
- `rake spec` to run the rspec suite
- `rake karma` to run the karma test suite
- `rake gitlab:test` to run all the tests
- `bin/rake spec` to run the rspec suite
- `bin/rake spec:unit` to run the only the unit tests
- `bin/rake spec:integration` to run the only the integration tests
- `bin/rake spec:system` to run the only the system tests
- `bin/rake karma` to run the karma test suite
Note: `rake spec` takes significant time to pass.
Note: `bin/rake spec` takes significant time to pass.
Instead of running full test suite locally you can save a lot of time by running
a single test or directory related to your changes. After you submit merge request
CI will run full test suite for you. Green CI status in the merge request means
@ -121,6 +123,9 @@ full test suite is passed.
Note: You can't run `rspec .` since this will try to run all the `_spec.rb`
files it can find, also the ones in `/tmp`
Note: You can pass RSpec command line options to the `spec:unit`,
`spec:integration`, and `spec:system` tasks, e.g. `bin/rake "spec:unit[--tag ~geo --dry-run]"`.
To run a single test file you can use:
- `bin/rspec spec/controllers/commit_controller_spec.rb` for a rspec test

View file

@ -2,19 +2,29 @@
## Test Design
Testing at GitLab is a first class citizen, not an afterthought. It's important we consider the design of our tests
as we do the design of our features.
Testing at GitLab is a first class citizen, not an afterthought. It's important we consider the design of our tests
as we do the design of our features.
When implementing a feature, we think about developing the right capabilities the right way, which helps us
narrow our scope to a manageable level. When implementing tests for a feature, we must think about developing
the right tests, but then cover _all_ the important ways the test may fail, which can quickly widen our scope to
When implementing a feature, we think about developing the right capabilities the right way, which helps us
narrow our scope to a manageable level. When implementing tests for a feature, we must think about developing
the right tests, but then cover _all_ the important ways the test may fail, which can quickly widen our scope to
a level that is difficult to manage.
Test heuristics can help solve this problem. They concisely address many of the common ways bugs
manifest themselves within our code. When designing our tests, take time to review known test heuristics to inform
our test design. We can find some helpful heuristics documented in the Handbook in the
Test heuristics can help solve this problem. They concisely address many of the common ways bugs
manifest themselves within our code. When designing our tests, take time to review known test heuristics to inform
our test design. We can find some helpful heuristics documented in the Handbook in the
[Test Design](https://about.gitlab.com/handbook/engineering/quality/guidelines/test-engineering/test-design/) section.
## Run tests against MySQL
By default, tests are only run againts PostgreSQL, but you can run them on
demand against MySQL by following one of the following conventions:
| Convention | Valid example |
|:----------------------|:-----------------------------|
| Include `mysql` in your branch name | `enhance-mysql-support` |
| Include `[run mysql]` in your commit message | `Fix MySQL support<br><br>[run mysql]` |
## Test speed
GitLab has a massive test suite that, without [parallelization], can take hours
@ -184,11 +194,11 @@ instead of 30+ seconds in case of a regular `spec_helper`.
### `let` variables
GitLab's RSpec suite has made extensive use of `let`(along with it strict, non-lazy
version `let!`) variables to reduce duplication. However, this sometimes [comes at the cost of clarity][lets-not],
version `let!`) variables to reduce duplication. However, this sometimes [comes at the cost of clarity][lets-not],
so we need to set some guidelines for their use going forward:
- `let!` variables are preferable to instance variables. `let` variables
are preferable to `let!` variables. Local variables are preferable to
are preferable to `let!` variables. Local variables are preferable to
`let` variables.
- Use `let` to reduce duplication throughout an entire spec file.
- Don't use `let` to define variables used by a single test; define them as
@ -199,8 +209,8 @@ so we need to set some guidelines for their use going forward:
- Try to avoid overriding the definition of one `let` variable with another.
- Don't define a `let` variable that's only used by the definition of another.
Use a helper method instead.
- `let!` variables should be used only in case if strict evaluation with defined
order is required, otherwise `let` will suffice. Remember that `let` is lazy and won't
- `let!` variables should be used only in case if strict evaluation with defined
order is required, otherwise `let` will suffice. Remember that `let` is lazy and won't
be evaluated until it is referenced.
[lets-not]: https://robots.thoughtbot.com/lets-not

View file

@ -4,12 +4,14 @@
_This diagram demonstrates the relative priority of each test type we use. `e2e` stands for end-to-end._
As of 2019-04-16, we have the following distribution of tests per level:
As of 2019-05-01, we have the following distribution of tests per level:
- 67 black-box tests at the system level (aka end-to-end or QA tests) in CE, 98 in EE. This represents 0.3% of all the CE tests (0.3% in EE).
- 5,457 white-box tests at the system level (aka system or feature tests) in CE, 6,585 in EE. This represents 24.6% of all the CE tests (20.3% in EE).
- 8,298 integration tests in CE, 10,633 in EE: 0.3% of all the CE tests (0.3% in EE). This represents 37.2% of all the CE tests (32.8% in EE).
- 8,403 unit tests in CE, 15,090 in EE: 0.3% of all the CE tests (0.3% in EE). This represents 37.8% of all the CE tests (46.6% in EE).
| Test level | Community Edition | Enterprise Edition | Community + Enterprise Edition |
| --------- | ---------- | -------------- | ----- |
| Black-box tests at the system level (aka end-to-end or QA tests) | 68 (0.14%) | 31 (0.2%) | 99 (0.17%) |
| White-box tests at the system level (aka system or feature tests) | 5,471 (11.9%) | 969 (7.4%) | 6440 (10.9%) |
| Integration tests | 8,333 (18.2%) | 2,244 (17.2%) | 10,577 (17.9%) |
| Unit tests | 32,031 (69.7%) | 9,778 (75.1%) | 41,809 (71%) |
## Unit tests

75
lib/quality/test_level.rb Normal file
View file

@ -0,0 +1,75 @@
# frozen_string_literal: true
module Quality
class TestLevel
UnknownTestLevelError = Class.new(StandardError)
TEST_LEVEL_FOLDERS = {
unit: %w[
bin
config
db
dependencies
factories
finders
frontend
graphql
helpers
initializers
javascripts
lib
migrations
models
policies
presenters
rack_servers
routing
rubocop
serializers
services
sidekiq
tasks
uploaders
validators
views
workers
elastic_integration
],
integration: %w[
controllers
mailers
requests
],
system: ['features']
}.freeze
attr_reader :prefix
def initialize(prefix = nil)
@prefix = prefix
@patterns = {}
@regexps = {}
end
def pattern(level)
@patterns[level] ||= "#{prefix}spec/{#{TEST_LEVEL_FOLDERS.fetch(level).join(',')}}{,/**/}*_spec.rb".freeze
end
def regexp(level)
@regexps[level] ||= Regexp.new("#{prefix}spec/(#{TEST_LEVEL_FOLDERS.fetch(level).join('|')})").freeze
end
def level_for(file_path)
case file_path
when regexp(:unit)
:unit
when regexp(:integration)
:integration
when regexp(:system)
:system
else
raise UnknownTestLevelError, "Test level for #{file_path} couldn't be set. Please rename the file properly or change the test level detection regexes in #{__FILE__}."
end
end
end
end

View file

@ -1,7 +1,32 @@
# frozen_string_literal: true
return if Rails.env.production?
Rake::Task["spec"].clear if Rake::Task.task_defined?('spec')
namespace :spec do
desc 'GitLab | Rspec | Run request specs'
desc 'GitLab | RSpec | Run unit tests'
RSpec::Core::RakeTask.new(:unit, :rspec_opts) do |t, args|
require_dependency 'quality/test_level'
t.pattern = Quality::TestLevel.new.pattern(:unit)
t.rspec_opts = args[:rspec_opts]
end
desc 'GitLab | RSpec | Run integration tests'
RSpec::Core::RakeTask.new(:integration, :rspec_opts) do |t, args|
require_dependency 'quality/test_level'
t.pattern = Quality::TestLevel.new.pattern(:integration)
t.rspec_opts = args[:rspec_opts]
end
desc 'GitLab | RSpec | Run system tests'
RSpec::Core::RakeTask.new(:system, :rspec_opts) do |t, args|
require_dependency 'quality/test_level'
t.pattern = Quality::TestLevel.new.pattern(:system)
t.rspec_opts = args[:rspec_opts]
end
desc '[Deprecated] Use the "bin/rspec --tag api" instead'
task :api do
cmds = [
%w(rake gitlab:setup),
@ -10,7 +35,7 @@ namespace :spec do
run_commands(cmds)
end
desc 'GitLab | Rspec | Run feature specs'
desc '[Deprecated] Use the "spec:system" task instead'
task :feature do
cmds = [
%w(rake gitlab:setup),
@ -19,7 +44,7 @@ namespace :spec do
run_commands(cmds)
end
desc 'GitLab | Rspec | Run model specs'
desc '[Deprecated] Use "bin/rspec spec/models" instead'
task :models do
cmds = [
%w(rake gitlab:setup),
@ -28,7 +53,7 @@ namespace :spec do
run_commands(cmds)
end
desc 'GitLab | Rspec | Run service specs'
desc '[Deprecated] Use "bin/rspec spec/services" instead'
task :services do
cmds = [
%w(rake gitlab:setup),
@ -37,7 +62,7 @@ namespace :spec do
run_commands(cmds)
end
desc 'GitLab | Rspec | Run lib specs'
desc '[Deprecated] Use "bin/rspec spec/lib" instead'
task :lib do
cmds = [
%w(rake gitlab:setup),
@ -45,15 +70,6 @@ namespace :spec do
]
run_commands(cmds)
end
desc 'GitLab | Rspec | Run other specs'
task :other do
cmds = [
%w(rake gitlab:setup),
%w(rspec spec --tag ~@api --tag ~@feature --tag ~@models --tag ~@lib --tag ~@services)
]
run_commands(cmds)
end
end
desc "GitLab | Run specs"

View file

@ -5,6 +5,7 @@ export USE_BUNDLE_INSTALL=${USE_BUNDLE_INSTALL:-true}
export BUNDLE_INSTALL_FLAGS="--without=production --jobs=$(nproc) --path=vendor --retry=3 --quiet"
if [ "$USE_BUNDLE_INSTALL" != "false" ]; then
bundle --version
bundle install --clean $BUNDLE_INSTALL_FLAGS && bundle check
fi
@ -16,12 +17,10 @@ cp config/gitlab.yml.example config/gitlab.yml
sed -i 's/bin_path: \/usr\/bin\/git/bin_path: \/usr\/local\/bin\/git/' config/gitlab.yml
# Determine the database by looking at the job name.
# For example, we'll get pg if the job is `rspec-pg 19 20`
export GITLAB_DATABASE=$(echo $CI_JOB_NAME | cut -f1 -d' ' | cut -f2 -d-)
# This would make the default database postgresql, and we could also use
# pg to mean postgresql.
if [ "$GITLAB_DATABASE" != 'mysql' ]; then
# This would make the default database postgresql.
if [[ "${CI_JOB_NAME#*mysql}" != "$CI_JOB_NAME" ]]; then
export GITLAB_DATABASE='mysql'
else
export GITLAB_DATABASE='postgresql'
fi

View file

@ -0,0 +1,105 @@
# frozen_string_literal: true
require 'fast_spec_helper'
RSpec.describe Quality::TestLevel do
describe '#pattern' do
context 'when level is unit' do
it 'returns a pattern' do
expect(subject.pattern(:unit))
.to eq("spec/{bin,config,db,dependencies,factories,finders,frontend,graphql,helpers,initializers,javascripts,lib,migrations,models,policies,presenters,rack_servers,routing,rubocop,serializers,services,sidekiq,tasks,uploaders,validators,views,workers,elastic_integration}{,/**/}*_spec.rb")
end
end
context 'when level is integration' do
it 'returns a pattern' do
expect(subject.pattern(:integration))
.to eq("spec/{controllers,mailers,requests}{,/**/}*_spec.rb")
end
end
context 'when level is system' do
it 'returns a pattern' do
expect(subject.pattern(:system))
.to eq("spec/{features}{,/**/}*_spec.rb")
end
end
context 'with a prefix' do
it 'returns a pattern' do
expect(described_class.new('ee/').pattern(:system))
.to eq("ee/spec/{features}{,/**/}*_spec.rb")
end
end
describe 'performance' do
it 'memoizes the pattern for a given level' do
expect(subject.pattern(:system).object_id).to eq(subject.pattern(:system).object_id)
end
it 'freezes the pattern for a given level' do
expect(subject.pattern(:system)).to be_frozen
end
end
end
describe '#regexp' do
context 'when level is unit' do
it 'returns a regexp' do
expect(subject.regexp(:unit))
.to eq(%r{spec/(bin|config|db|dependencies|factories|finders|frontend|graphql|helpers|initializers|javascripts|lib|migrations|models|policies|presenters|rack_servers|routing|rubocop|serializers|services|sidekiq|tasks|uploaders|validators|views|workers|elastic_integration)})
end
end
context 'when level is integration' do
it 'returns a regexp' do
expect(subject.regexp(:integration))
.to eq(%r{spec/(controllers|mailers|requests)})
end
end
context 'when level is system' do
it 'returns a regexp' do
expect(subject.regexp(:system))
.to eq(%r{spec/(features)})
end
end
context 'with a prefix' do
it 'returns a regexp' do
expect(described_class.new('ee/').regexp(:system))
.to eq(%r{ee/spec/(features)})
end
end
describe 'performance' do
it 'memoizes the regexp for a given level' do
expect(subject.regexp(:system).object_id).to eq(subject.regexp(:system).object_id)
end
it 'freezes the regexp for a given level' do
expect(subject.regexp(:system)).to be_frozen
end
end
end
describe '#level_for' do
it 'returns the correct level for a unit test' do
expect(subject.level_for('spec/models/abuse_report_spec.rb')).to eq(:unit)
end
it 'returns the correct level for an integration test' do
expect(subject.level_for('spec/mailers/abuse_report_mailer_spec.rb')).to eq(:integration)
end
it 'returns the correct level for a system test' do
expect(subject.level_for('spec/features/abuse_report_spec.rb')).to eq(:system)
end
it 'raises an error for an unknown level' do
expect { subject.level_for('spec/unknown/foo_spec.rb') }
.to raise_error(described_class::UnknownTestLevelError,
%r{Test level for spec/unknown/foo_spec.rb couldn't be set. Please rename the file properly or change the test level detection regexes in .+/lib/quality/test_level.rb.})
end
end
end

View file

@ -44,6 +44,8 @@ Dir[Rails.root.join("spec/support/shared_contexts/*.rb")].each { |f| require f }
Dir[Rails.root.join("spec/support/shared_examples/*.rb")].each { |f| require f }
Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f }
quality_level = Quality::TestLevel.new
RSpec.configure do |config|
config.use_transactional_fixtures = false
config.use_instantiated_fixtures = false
@ -55,9 +57,10 @@ RSpec.configure do |config|
config.infer_spec_type_from_file_location!
config.full_backtrace = !!ENV['CI']
config.define_derived_metadata(file_path: %r{/spec/}) do |metadata|
config.define_derived_metadata(file_path: %r{(ee)?/spec/.+_spec\.rb\z}) do |metadata|
location = metadata[:location]
metadata[:level] = quality_level.level_for(location)
metadata[:api] = true if location =~ %r{/spec/requests/api/}
# do not overwrite type if it's already set