Commit 41cbc98b authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'improve-telemetry-danger-bot-include-database-review' into 'master'

Add database category for counters changes in usage data files

See merge request gitlab-org/gitlab!36690
parents c6336a50 3a4a45bc
...@@ -227,7 +227,10 @@ module EE ...@@ -227,7 +227,10 @@ module EE
:creator_id, :creator_id,
start: user_minimum_id, start: user_minimum_id,
finish: user_maximum_id), finish: user_maximum_id),
protected_branches: distinct_count(::Project.with_protected_branches.where(time_period), :creator_id, start: user_minimum_id, finish: user_maximum_id), protected_branches: distinct_count(::Project.with_protected_branches.where(time_period),
:creator_id,
start: user_minimum_id,
finish: user_maximum_id),
suggestions: distinct_count(::Note.with_suggestions.where(time_period), suggestions: distinct_count(::Note.with_suggestions.where(time_period),
:author_id, :author_id,
start: user_minimum_id, start: user_minimum_id,
......
...@@ -34,6 +34,18 @@ module Gitlab ...@@ -34,6 +34,18 @@ module Gitlab
.sort .sort
end end
# Returns a string containing changed lines as git diff
#
# Considering changing a line in lib/gitlab/usage_data.rb it will return:
#
# [ "--- a/lib/gitlab/usage_data.rb",
# "+++ b/lib/gitlab/usage_data.rb",
# "+ # Test change",
# "- # Old change" ]
def changed_lines(changed_file)
git.diff_for_file(changed_file).patch.split("\n").select { |line| %r{^[+-]}.match?(line) }
end
def all_ee_changes def all_ee_changes
all_changed_files.grep(%r{\Aee/}) all_changed_files.grep(%r{\Aee/})
end end
...@@ -77,10 +89,19 @@ module Gitlab ...@@ -77,10 +89,19 @@ module Gitlab
end end
end end
# Determines the categories a file is in, e.g., `[:frontend]`, `[:backend]`, or `%i[frontend engineering_productivity]`. # Determines the categories a file is in, e.g., `[:frontend]`, `[:backend]`, or `%i[frontend engineering_productivity]`
# using filename regex and specific change regex if given.
#
# @return Array<Symbol> # @return Array<Symbol>
def categories_for_file(file) def categories_for_file(file)
_, categories = CATEGORIES.find { |regexp, _| regexp.match?(file) } _, categories = CATEGORIES.find do |key, _|
filename_regex, changes_regex = Array(key)
found = filename_regex.match?(file)
found &&= changed_lines(file).any? { |changed_line| changes_regex.match?(changed_line) } if changes_regex
found
end
Array(categories || :unknown) Array(categories || :unknown)
end end
...@@ -102,6 +123,8 @@ module Gitlab ...@@ -102,6 +123,8 @@ module Gitlab
}.freeze }.freeze
# First-match win, so be sure to put more specific regex at the top... # First-match win, so be sure to put more specific regex at the top...
CATEGORIES = { CATEGORIES = {
[%r{usage_data}, %r{^(\+|-).*(count|distinct_count)\(.*\)(.*)$}] => [:database, :backend],
%r{\Adoc/.*(\.(md|png|gif|jpg))\z} => :docs, %r{\Adoc/.*(\.(md|png|gif|jpg))\z} => :docs,
%r{\A(CONTRIBUTING|LICENSE|MAINTENANCE|PHILOSOPHY|PROCESS|README)(\.md)?\z} => :docs, %r{\A(CONTRIBUTING|LICENSE|MAINTENANCE|PHILOSOPHY|PROCESS|README)(\.md)?\z} => :docs,
......
...@@ -166,7 +166,12 @@ RSpec.describe Gitlab::Danger::Helper do ...@@ -166,7 +166,12 @@ RSpec.describe Gitlab::Danger::Helper do
end end
describe '#categories_for_file' do describe '#categories_for_file' do
before do
allow(fake_git).to receive(:diff_for_file).with('usage_data.rb') { double(:diff, patch: "+ count(User.active)") }
end
where(:path, :expected_categories) do where(:path, :expected_categories) do
'usage_data.rb' | [:database, :backend]
'doc/foo.md' | [:docs] 'doc/foo.md' | [:docs]
'CONTRIBUTING.md' | [:docs] 'CONTRIBUTING.md' | [:docs]
'LICENSE' | [:docs] 'LICENSE' | [:docs]
...@@ -287,6 +292,26 @@ RSpec.describe Gitlab::Danger::Helper do ...@@ -287,6 +292,26 @@ RSpec.describe Gitlab::Danger::Helper do
it { is_expected.to eq(expected_categories) } it { is_expected.to eq(expected_categories) }
end end
context 'having specific changes' do
it 'has database and backend categories' do
allow(fake_git).to receive(:diff_for_file).with('usage_data.rb') { double(:diff, patch: "+ count(User.active)") }
expect(helper.categories_for_file('usage_data.rb')).to eq([:database, :backend])
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])
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
end
end end
describe '#label_for_category' do describe '#label_for_category' do
......
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