Commit e87a67e2 authored by Alina Mihaila's avatar Alina Mihaila Committed by Mikołaj Wawrzyniak

Add estimate_batch_distinct_count changes to dangerbot database

parent 2a54e01b
......@@ -25,7 +25,7 @@ A database review is required for:
generally up to the author of a merge request to decide whether or
not complex queries are being introduced and if they require a
database review.
- Changes in usage data metrics that use `count` and `distinct_count`.
- Changes in usage data metrics that use `count`, `distinct_count` and `estimate_batch_distinct_count`.
These metrics could have complex queries over large tables.
See the [Product Analytics Guide](https://about.gitlab.com/handbook/product/product-analytics-guide/)
for implementation details.
......
......@@ -128,7 +128,7 @@ module Gitlab
}.freeze
# First-match win, so be sure to put more specific regex at the top...
CATEGORIES = {
[%r{usage_data\.rb}, %r{^(\+|-).*(count|distinct_count)\(.*\)(.*)$}] => [:database, :backend],
[%r{usage_data\.rb}, %r{^(\+|-).*\s+(count|distinct_count|estimate_batch_distinct_count)\(.*\)(.*)$}] => [:database, :backend],
%r{\Adoc/.*(\.(md|png|gif|jpg))\z} => :docs,
%r{\A(CONTRIBUTING|LICENSE|MAINTENANCE|PHILOSOPHY|PROCESS|README)(\.md)?\z} => :docs,
......
......@@ -351,33 +351,23 @@ RSpec.describe Gitlab::Danger::Helper do
end
context 'having specific changes' do
it 'has database and backend categories' do
changed_files = ['usage_data.rb', 'lib/gitlab/usage_data.rb', 'ee/lib/ee/gitlab/usage_data.rb']
changed_files.each do |file|
allow(fake_git).to receive(:diff_for_file).with(file) { double(:diff, patch: "+ count(User.active)") }
expect(helper.categories_for_file(file)).to eq([:database, :backend])
end
end
it 'has backend category' do
allow(fake_git).to receive(:diff_for_file).with('usage_data.rb') { double(:diff, patch: "+ alt_usage_data(User.active)") }
expect(helper.categories_for_file('usage_data.rb')).to eq([:backend])
where(:expected_categories, :patch, :changed_files) do
[:database, :backend] | '+ count(User.active)' | ['usage_data.rb', 'lib/gitlab/usage_data.rb', 'ee/lib/ee/gitlab/usage_data.rb']
[:database, :backend] | '+ estimate_batch_distinct_count(User.active)' | ['usage_data.rb']
[:backend] | '+ alt_usage_data(User.active)' | ['usage_data.rb']
[:backend] | '+ count(User.active)' | ['user.rb']
[:backend] | '+ count(User.active)' | ['usage_data/topology.rb']
[:backend] | '+ foo_count(User.active)' | ['usage_data.rb']
end
it 'has backend category for changes outside usage_data files' do
allow(fake_git).to receive(:diff_for_file).with('user.rb') { double(:diff, patch: "+ count(User.active)") }
expect(helper.categories_for_file('user.rb')).to eq([:backend])
end
with_them do
it 'has the correct categories' do
changed_files.each do |file|
allow(fake_git).to receive(:diff_for_file).with(file) { double(:diff, patch: patch) }
it 'has backend category for files that are not usage_data.rb' do
changed_file = 'usage_data/topology.rb'
allow(fake_git).to receive(:diff_for_file).with(changed_file) { double(:diff, patch: "+ count(User.active)") }
expect(helper.categories_for_file(changed_file)).to eq([:backend])
expect(helper.categories_for_file(file)).to eq(expected_categories)
end
end
end
end
end
......
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