Commit 6481bc8c authored by Albert Salim's avatar Albert Salim Committed by Rémy Coutable

Use tff to detect foss tests in CI

- add a job to verify tff mapping file
parent ac26955d
...@@ -496,11 +496,12 @@ rspec foss-impact: ...@@ -496,11 +496,12 @@ rspec foss-impact:
- .rails:rules:ee-mr-only - .rails:rules:ee-mr-only
script: script:
- install_gitlab_gem - install_gitlab_gem
- install_tff_gem
- run_timed_command "scripts/gitaly-test-build" - run_timed_command "scripts/gitaly-test-build"
- run_timed_command "scripts/gitaly-test-spawn" - run_timed_command "scripts/gitaly-test-spawn"
- source scripts/rspec_helpers.sh - source scripts/rspec_helpers.sh
- tooling/bin/find_foss_tests tmp/matching_foss_tests.txt - tooling/bin/find_foss_tests tmp/matching_foss_tests.txt
- rspec_matched_tests tmp/matching_foss_tests.txt "--tag ~quarantine" - rspec_matched_foss_tests tmp/matching_foss_tests.txt "--tag ~quarantine"
artifacts: artifacts:
expire_in: 7d expire_in: 7d
paths: paths:
......
...@@ -149,6 +149,7 @@ ...@@ -149,6 +149,7 @@
- "*_VERSION" - "*_VERSION"
- "Gemfile{,.lock}" - "Gemfile{,.lock}"
- "Rakefile" - "Rakefile"
- "tests.yml"
- "config.ru" - "config.ru"
- "{,ee/}{app,bin,config,db,haml_lint,lib,locale,public,scripts,symbol,vendor}/**/*" - "{,ee/}{app,bin,config,db,haml_lint,lib,locale,public,scripts,symbol,vendor}/**/*"
- "doc/api/graphql/reference/*" # Files in this folder are auto-generated - "doc/api/graphql/reference/*" # Files in this folder are auto-generated
...@@ -170,6 +171,7 @@ ...@@ -170,6 +171,7 @@
- "*_VERSION" - "*_VERSION"
- "Gemfile{,.lock}" - "Gemfile{,.lock}"
- "Rakefile" - "Rakefile"
- "tests.yml"
- "config.ru" - "config.ru"
- "{,ee/}{app,bin,config,db,haml_lint,lib,locale,public,scripts,symbol,vendor}/**/*" - "{,ee/}{app,bin,config,db,haml_lint,lib,locale,public,scripts,symbol,vendor}/**/*"
- "doc/api/graphql/reference/*" # Files in this folder are auto-generated - "doc/api/graphql/reference/*" # Files in this folder are auto-generated
...@@ -193,6 +195,7 @@ ...@@ -193,6 +195,7 @@
- "*_VERSION" - "*_VERSION"
- "Gemfile{,.lock}" - "Gemfile{,.lock}"
- "Rakefile" - "Rakefile"
- "tests.yml"
- "config.ru" - "config.ru"
- "{,ee/}{app,bin,config,db,haml_lint,lib,locale,public,scripts,symbol,vendor}/**/*" - "{,ee/}{app,bin,config,db,haml_lint,lib,locale,public,scripts,symbol,vendor}/**/*"
- "doc/api/graphql/reference/*" # Files in this folder are auto-generated - "doc/api/graphql/reference/*" # Files in this folder are auto-generated
...@@ -213,6 +216,7 @@ ...@@ -213,6 +216,7 @@
- "*_VERSION" - "*_VERSION"
- "Gemfile{,.lock}" - "Gemfile{,.lock}"
- "Rakefile" - "Rakefile"
- "tests.yml"
- "config.ru" - "config.ru"
- "{,ee/}{app,bin,config,db,haml_lint,lib,locale,public,scripts,symbol,vendor}/**/*" - "{,ee/}{app,bin,config,db,haml_lint,lib,locale,public,scripts,symbol,vendor}/**/*"
- "doc/api/graphql/reference/*" # Files in this folder are auto-generated - "doc/api/graphql/reference/*" # Files in this folder are auto-generated
...@@ -762,6 +766,12 @@ ...@@ -762,6 +766,12 @@
changes: *code-backstage-patterns changes: *code-backstage-patterns
when: on_success when: on_success
.setup:rules:verify-tests-yml:
rules:
- <<: *if-default-refs
changes: *code-backstage-patterns
when: on_success
####################### #######################
# Test metadata rules # # Test metadata rules #
####################### #######################
......
...@@ -48,3 +48,14 @@ no_ee_check: ...@@ -48,3 +48,14 @@ no_ee_check:
stage: test stage: test
script: script:
- scripts/no-ee-check - scripts/no-ee-check
verify-tests-yml:
extends:
- .setup:rules:verify-tests-yml
image: ruby:2.6-alpine
stage: test
needs: []
script:
- source scripts/utils.sh
- install_tff_gem
- scripts/verify-tff-mapping
...@@ -171,6 +171,7 @@ module Gitlab ...@@ -171,6 +171,7 @@ module Gitlab
%r{\A(ee/)?scripts/} => :engineering_productivity, %r{\A(ee/)?scripts/} => :engineering_productivity,
%r{\Atooling/} => :engineering_productivity, %r{\Atooling/} => :engineering_productivity,
%r{(CODEOWNERS)} => :engineering_productivity, %r{(CODEOWNERS)} => :engineering_productivity,
%r{(tests.yml)} => :engineering_productivity,
%r{\A(ee/)?spec/features/} => :test, %r{\A(ee/)?spec/features/} => :test,
%r{\A(ee/)?spec/support/shared_examples/features/} => :test, %r{\A(ee/)?spec/support/shared_examples/features/} => :test,
......
...@@ -111,7 +111,7 @@ function rspec_paralellized_job() { ...@@ -111,7 +111,7 @@ function rspec_paralellized_job() {
date date
} }
function rspec_matched_tests() { function rspec_matched_foss_tests() {
local test_file_count_threshold=20 local test_file_count_threshold=20
local matching_tests_file=${1} local matching_tests_file=${1}
local rspec_opts=${2} local rspec_opts=${2}
...@@ -131,6 +131,6 @@ function rspec_matched_tests() { ...@@ -131,6 +131,6 @@ function rspec_matched_tests() {
if [[ -n $test_files ]]; then if [[ -n $test_files ]]; then
rspec_simple_job "${rspec_opts} ${test_files}" rspec_simple_job "${rspec_opts} ${test_files}"
else else
echo "No test files to run" echo "No FOSS test files to run"
fi fi
} }
...@@ -35,6 +35,10 @@ function install_gitlab_gem() { ...@@ -35,6 +35,10 @@ function install_gitlab_gem() {
gem install gitlab --no-document --version 4.13.0 gem install gitlab --no-document --version 4.13.0
} }
function install_tff_gem() {
gem install test_file_finder --version 0.1.0
}
function run_timed_command() { function run_timed_command() {
local cmd="${1}" local cmd="${1}"
local start=$(date +%s) local start=$(date +%s)
......
#!/usr/bin/env ruby
require 'set'
# These tests run a sanity check on the mapping file `tests.yml`
# used with the `test_file_finder` gem (`tff`) to identify matching test files.
# The verification depend on the presence of actual test files,
# so they would fail if one of the test files mentioned here is deleted.
# To minimize the chance of this test failing due to unrelated changes,
# the test files are chosen to be critical files that are unlikely to be deleted in a typical Merge Request
tests = [
{
explanation: 'EE code should map to respective spec',
source: 'ee/app/controllers/admin/licenses_controller.rb',
expected: ['ee/spec/controllers/admin/licenses_controller_spec.rb']
},
{
explanation: 'FOSS code should map to respective spec',
source: 'app/finders/admin/projects_finder.rb',
expected: ['spec/finders/admin/projects_finder_spec.rb']
},
{
explanation: 'EE extension should map to its EE extension spec and its FOSS class spec',
source: 'ee/app/finders/ee/projects_finder.rb',
expected: ['ee/spec/finders/ee/projects_finder_spec.rb', 'spec/finders/projects_finder_spec.rb']
},
{
explanation: 'Some EE extensions also map to its EE class spec, but this is not recommended: https://docs.gitlab.com/ee/development/ee_features.html#testing-ee-features-based-on-ce-features',
source: 'ee/app/models/ee/user.rb',
expected: ['ee/spec/models/user_spec.rb', 'spec/models/user_spec.rb']
},
{
explanation: 'FOSS lib should map to respective spec',
source: 'lib/gitaly/server.rb',
expected: ['spec/lib/gitaly/server_spec.rb']
},
{
explanation: 'EE lib should map to respective spec',
source: 'ee/lib/flipper_session.rb',
expected: ['ee/spec/lib/flipper_session_spec.rb']
},
{
explanation: 'Tooling should map to respective spec',
source: 'tooling/lib/tooling/test_file_finder.rb',
expected: ['spec/tooling/lib/tooling/test_file_finder_spec.rb']
},
{
explanation: 'FOSS spec code should map to itself',
source: 'spec/models/issue_spec.rb',
expected: ['spec/models/issue_spec.rb']
},
{
explanation: 'EE spec code should map to itself',
source: 'ee/spec/models/user_spec.rb',
expected: ['ee/spec/models/user_spec.rb']
},
{
explanation: 'EE extension spec should map to itself and the FOSS class spec',
source: 'ee/spec/services/ee/notification_service_spec.rb',
expected: ['ee/spec/services/ee/notification_service_spec.rb', 'spec/services/notification_service_spec.rb']
},
{
explanation: 'FOSS factory should map to factories spec',
source: 'spec/factories/users.rb',
expected: ['spec/factories_spec.rb']
},
{
explanation: 'EE factory should map to factories spec',
source: 'ee/spec/factories/users.rb',
expected: ['spec/factories_spec.rb']
},
{
explanation: 'Initializers should map to respective spec',
source: 'config/initializers/action_mailer_hooks.rb',
expected: ['spec/initializers/action_mailer_hooks_spec.rb']
},
{
explanation: 'FOSS views should map to respective spec',
source: 'app/views/admin/users/_user.html.haml',
expected: ['spec/views/admin/users/_user.html.haml_spec.rb']
},
{
explanation: 'EE views should map to respective spec',
source: 'ee/app/views/admin/licenses/show.html.haml',
expected: ['ee/spec/views/admin/licenses/show.html.haml_spec.rb']
},
{
explanation: 'DB structure should map to schema spec',
source: 'db/structure.sql',
expected: ['spec/db/schema_spec.rb']
},
{
explanation: 'Migration should map to its non-timestamped spec',
source: 'db/migrate/20191023152913_add_default_and_free_plans.rb',
expected: ['spec/migrations/add_default_and_free_plans_spec.rb']
},
{
explanation: 'Migration should map to its timestamped spec',
source: 'db/post_migrate/20190924152703_migrate_issue_trackers_data.rb',
expected: ['spec/migrations/20190924152703_migrate_issue_trackers_data_spec.rb']
}
]
class MappingTest
def initialize(explanation:, source:, expected:, mapping: 'tests.yml')
@explanation = explanation
@source = source
@mapping = mapping
@expected_set = Set.new(expected)
@actual_set = Set.new(actual)
end
def passed?
expected_set.eql?(actual_set)
end
def failed?
!passed?
end
def failure_message
"#{explanation}: #{source}: Expected #{expected_set.to_a}, got #{actual_set.to_a}."
end
private
attr_reader :explanation, :source, :expected_set, :actual_set, :mapping
def actual
`tff -f #{mapping} #{source}`.split(' ')
end
end
results = tests.map { |test| MappingTest.new(test) }
failed_tests = results.select(&:failed?)
if failed_tests.any?
puts <<~MESSAGE
tff mapping verification failed:
#{failed_tests.map(&:failure_message).join("\n")}
MESSAGE
exit 1
end
puts 'tff mapping verification passed.'
mapping:
# EE code should map to respective spec
- source: ee/app/(.+)\.rb
test: ee/spec/%s_spec.rb
# FOSS code should map to respective spec
- source: app/(.+)\.rb
test: spec/%s_spec.rb
# EE extension should also map to its FOSS class spec
- source: ee/app/(.*/)ee/(.+)\.rb
test: spec/%s%s_spec.rb
# Some EE extensions also map to its EE class spec, but this is not recommended:
# https://docs.gitlab.com/ee/development/ee_features.html#testing-ee-features-based-on-ce-features
- source: ee/app/(.*/)ee/(.+)\.rb
test: ee/spec/%s%s_spec.rb
# EE lib should map to respective spec
- source: ee/lib/(.+)\.rb
test: ee/spec/lib/%s_spec.rb
# FOSS lib & tooling should map to respective spec
- source: (tooling/)?lib/(.+)\.rb
test: spec/%slib/%s_spec.rb
# Initializers should map to respective spec
- source: config/initializers/(.+)\.rb
test: spec/initializers/%s_spec.rb
# DB structure should map to schema spec
- source: db/structure.sql
test: spec/db/schema_spec.rb
# Migration should map to either timestamped or non-timestamped spec
- source: db/(?:post_)?migrate/(?:[0-9]+)_(.+)\.rb
test: spec/migrations/%s_spec.rb
- source: db/(?:post_)?migrate/([0-9]+)_(.+)\.rb
test: spec/migrations/%s_%s_spec.rb
# EE/FOSS views should map to respective spec
- source: (ee/)?app/views/(.+)\.haml
test: '%sspec/views/%s.haml_spec.rb'
# EE/FOSS spec code should map to itself
- source: (ee/)?spec/(.+)_spec\.rb
test: '%sspec/%s_spec.rb'
# EE extension spec should map to its FOSS class spec
- source: ee/spec/(.*/)ee/(.+)\.rb
test: spec/%s%s.rb
# EE/FOSS factory should map to factories spec
- source: (ee/)?spec/factories/.+\.rb
test: spec/factories_spec.rb
#!/usr/bin/env ruby #!/usr/bin/env ruby
# frozen_string_literal: true # frozen_string_literal: true
require_relative '../../lib/gitlab/popen'
require_relative '../lib/tooling/test_file_finder'
require 'gitlab' require 'gitlab'
require 'test_file_finder'
gitlab_token = ENV.fetch('DANGER_GITLAB_API_TOKEN', '') gitlab_token = ENV.fetch('DANGER_GITLAB_API_TOKEN', '')
...@@ -21,9 +19,7 @@ mr_iid = ENV.fetch('CI_MERGE_REQUEST_IID') ...@@ -21,9 +19,7 @@ mr_iid = ENV.fetch('CI_MERGE_REQUEST_IID')
mr_changes = Gitlab.merge_request_changes(mr_project_path, mr_iid) mr_changes = Gitlab.merge_request_changes(mr_project_path, mr_iid)
changed_files = mr_changes.changes.map { |change| change['new_path'] } changed_files = mr_changes.changes.map { |change| change['new_path'] }
tests_to_run = changed_files.flat_map do |file| mapping = TestFileFinder::Mapping.load('tests.yml')
test_files = Tooling::TestFileFinder.new(file, foss_test_only: true).test_files test_files = TestFileFinder::FileFinder.new(paths: changed_files, mapping: mapping).test_files
test_files.select { |f| File.exist?(f) }
end
File.write(output_file, tests_to_run.uniq.join(' ')) File.write(output_file, test_files.uniq.join(' '))
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment