Skip to content
代码片段 群组 项目
提交 5f02a359 编辑于 作者: Alex Kalderimis's avatar Alex Kalderimis
浏览文件

Marginalia - Supply lines in test env

Reverts an older test change now that we include line in
the test environment as well as in dev.

Updates the merge_requests_spec to read expected queries from the query
recorder log. We should be examining the `log` as the robust source for
this information. Other changes are to help with understanding and
tracing the test failure.
上级 06bacac7
No related branches found
No related tags found
无相关合并请求
...@@ -17,9 +17,9 @@ ...@@ -17,9 +17,9 @@
# As mentioned in https://github.com/basecamp/marginalia/pull/93/files, # As mentioned in https://github.com/basecamp/marginalia/pull/93/files,
# adding :line has some overhead because a regexp on the backtrace has # adding :line has some overhead because a regexp on the backtrace has
# to be run on every SQL query. Only enable this in development because # to be run on every SQL query. Only enable this in development and test because
# we've seen it slow things down. # we've seen it slow things down.
if Rails.env.development? if Gitlab.dev_or_test_env?
Marginalia::Comment.components << :line Marginalia::Comment.components << :line
Marginalia::Comment.lines_to_ignore = Regexp.union( Marginalia::Comment.lines_to_ignore = Regexp.union(
Gitlab::BacktraceCleaner::IGNORE_BACKTRACES + %w[ Gitlab::BacktraceCleaner::IGNORE_BACKTRACES + %w[
......
...@@ -502,10 +502,12 @@ def pagination_query(params) ...@@ -502,10 +502,12 @@ def pagination_query(params)
end end
context 'when only the count is requested' do context 'when only the count is requested' do
let_it_be(:merged_at) { Time.new(2020, 1, 3) }
context 'when merged at filter is present' do context 'when merged at filter is present' do
let_it_be(:merge_request) do let_it_be(:merge_request) do
create(:merge_request, :unique_branches, source_project: project).tap do |mr| create(:merge_request, :unique_branches, source_project: project).tap do |mr|
mr.metrics.update!(merged_at: Time.new(2020, 1, 3)) mr.metrics.update!(merged_at: merged_at, created_at: merged_at - 2.days)
end end
end end
...@@ -522,12 +524,18 @@ def pagination_query(params) ...@@ -522,12 +524,18 @@ def pagination_query(params)
it 'does not query the merge requests table for the count' do it 'does not query the merge requests table for the count' do
query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) }
queries = query_recorder.data.each_value.first[:occurrences] queries = query_recorder.log
expect(queries).not_to include(match(/SELECT COUNT\(\*\) FROM "merge_requests"/)) expect(queries).not_to include(match(/SELECT COUNT\(\*\) FROM "merge_requests"/))
expect(queries).to include(match(/SELECT COUNT\(\*\) FROM "merge_request_metrics"/)) expect(queries).to include(match(/SELECT COUNT\(\*\) FROM "merge_request_metrics"/))
end end
context 'when total_time_to_merge and count is queried' do context 'when total_time_to_merge and count is queried' do
let_it_be(:merge_request_2) do
create(:merge_request, :unique_branches, source_project: project).tap do |mr|
mr.metrics.update!(merged_at: merged_at, created_at: merged_at - 1.day)
end
end
let(:query) do let(:query) do
graphql_query_for(:project, { full_path: project.full_path }, <<~QUERY) graphql_query_for(:project, { full_path: project.full_path }, <<~QUERY)
mergeRequests(mergedAfter: "2020-01-01", mergedBefore: "2020-01-05", first: 0) { mergeRequests(mergedAfter: "2020-01-01", mergedBefore: "2020-01-05", first: 0) {
...@@ -537,11 +545,18 @@ def pagination_query(params) ...@@ -537,11 +545,18 @@ def pagination_query(params)
QUERY QUERY
end end
it 'does not query the merge requests table for the total_time_to_merge' do it 'uses the merge_request_metrics table for total_time_to_merge' do
query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) }
queries = query_recorder.data.each_value.first[:occurrences] expect(query_recorder.log).to include(match(/SELECT.+SUM.+FROM "merge_request_metrics" WHERE/))
expect(queries).to include(match(/SELECT.+SUM.+FROM "merge_request_metrics" WHERE/)) end
it 'returns the correct total time to merge' do
post_graphql(query, current_user: current_user)
sum = graphql_data_at(:project, :merge_requests, :total_time_to_merge)
expect(sum).to eq(3.days.to_f)
end end
end end
......
...@@ -78,12 +78,14 @@ def expect_section(query, lines) ...@@ -78,12 +78,14 @@ def expect_section(query, lines)
end end
describe 'detecting the right number of calls and their origin' do describe 'detecting the right number of calls and their origin' do
it 'detects two separate queries' do let(:control) do
control = ActiveRecord::QueryRecorder.new query_recorder_debug: true do ActiveRecord::QueryRecorder.new query_recorder_debug: true do
2.times { TestQueries.count } 2.times { TestQueries.count }
TestQueries.first TestQueries.first
end end
end
it 'detects two separate queries' do
# Check #find_query # Check #find_query
expect(control.find_query(/.*/, 0).size) expect(control.find_query(/.*/, 0).size)
.to eq(control.data.keys.size) .to eq(control.data.keys.size)
...@@ -98,8 +100,8 @@ def expect_section(query, lines) ...@@ -98,8 +100,8 @@ def expect_section(query, lines)
expect(control.log.size).to eq(3) expect(control.log.size).to eq(3)
# Ensure memoization value match the raw value above # Ensure memoization value match the raw value above
expect(control.count).to eq(control.log.size) expect(control.count).to eq(control.log.size)
# Ensure we have only two sources of queries # Ensure we have two sources of queries
expect(control.data.keys.size).to eq(1) expect(control.data.keys.size).to eq(2)
end end
end end
end end
0% 加载中 .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册