From 918e589c2b29c18d9fe3a8e6c93a3f490c86beb1 Mon Sep 17 00:00:00 2001
From: Timothy Andrew <mail@timothyandrew.net>
Date: Wed, 21 Sep 2016 00:47:37 +0530
Subject: [PATCH] Implement a second round of review comments from @DouweM.
- Don't use `TableReferences` - using `.arel_table` is shorter!
- Move some database-related code to `Gitlab::Database`
- Remove the `MergeRequest#issues_closed` and
`Issue#closed_by_merge_requests` associations. They were either
shadowing or were too similar to existing methods. They are not being
used anywhere, so it's better to remove them to reduce confusion.
- Use Rails 3-style validations
- Index for `MergeRequest::Metrics#first_deployed_to_production_at`
- Only include `CycleAnalyticsHelpers::TestGeneration` for specs that
need it.
- Other minor refactorings.
---
app/models/cycle_analytics.rb | 66 ++++++++-----------
app/models/cycle_analytics/summary.rb | 2 +-
.../cycle_analytics/table_references.rb | 25 -------
app/models/deployment.rb | 37 ++++++-----
app/models/issue.rb | 3 +-
app/models/merge_request.rb | 8 +--
app/models/merge_requests_closing_issues.rb | 6 +-
app/services/create_deployment_service.rb | 2 +-
app/views/projects/pipelines/_head.html.haml | 9 +--
...5052008_add_table_merge_request_metrics.rb | 2 +-
lib/gitlab/database/median.rb | 35 ++++++----
lib/gitlab/database/util.rb | 12 ++++
spec/models/cycle_analytics/code_spec.rb | 25 ++++---
spec/models/cycle_analytics/issue_spec.rb | 37 +++++++----
spec/models/cycle_analytics/plan_spec.rb | 42 +++++++-----
.../models/cycle_analytics/production_spec.rb | 37 ++++++-----
spec/models/cycle_analytics/review_spec.rb | 19 ++++--
spec/models/cycle_analytics/staging_spec.rb | 45 ++++++++-----
spec/models/cycle_analytics/test_spec.rb | 31 ++++-----
.../merge_requests/create_service_spec.rb | 2 +-
.../merge_requests/refresh_service_spec.rb | 2 +-
.../merge_requests/update_service_spec.rb | 6 +-
.../test_generation.rb | 4 --
23 files changed, 252 insertions(+), 205 deletions(-)
delete mode 100644 app/models/cycle_analytics/table_references.rb
create mode 100644 lib/gitlab/database/util.rb
diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb
index 3ca160252ff..be295487fd2 100644
--- a/app/models/cycle_analytics.rb
+++ b/app/models/cycle_analytics.rb
@@ -5,55 +5,54 @@ class CycleAnalytics
def initialize(project, from:)
@project = project
@from = from
- @summary = Summary.new(project, from: from)
end
def summary
- @summary
+ @summary ||= Summary.new(@project, from: @from)
end
def issue
calculate_metric(:issue,
- TableReferences.issues[:created_at],
- [TableReferences.issue_metrics[:first_associated_with_milestone_at],
- TableReferences.issue_metrics[:first_added_to_board_at]])
+ Issue.arel_table[:created_at],
+ [Issue::Metrics.arel_table[:first_associated_with_milestone_at],
+ Issue::Metrics.arel_table[:first_added_to_board_at]])
end
def plan
calculate_metric(:plan,
- [TableReferences.issue_metrics[:first_associated_with_milestone_at],
- TableReferences.issue_metrics[:first_added_to_board_at]],
- TableReferences.issue_metrics[:first_mentioned_in_commit_at])
+ [Issue::Metrics.arel_table[:first_associated_with_milestone_at],
+ Issue::Metrics.arel_table[:first_added_to_board_at]],
+ Issue::Metrics.arel_table[:first_mentioned_in_commit_at])
end
def code
calculate_metric(:code,
- TableReferences.issue_metrics[:first_mentioned_in_commit_at],
- TableReferences.merge_requests[:created_at])
+ Issue::Metrics.arel_table[:first_mentioned_in_commit_at],
+ MergeRequest.arel_table[:created_at])
end
def test
calculate_metric(:test,
- TableReferences.merge_request_metrics[:latest_build_started_at],
- TableReferences.merge_request_metrics[:latest_build_finished_at])
+ MergeRequest::Metrics.arel_table[:latest_build_started_at],
+ MergeRequest::Metrics.arel_table[:latest_build_finished_at])
end
def review
calculate_metric(:review,
- TableReferences.merge_requests[:created_at],
- TableReferences.merge_request_metrics[:merged_at])
+ MergeRequest.arel_table[:created_at],
+ MergeRequest::Metrics.arel_table[:merged_at])
end
def staging
calculate_metric(:staging,
- TableReferences.merge_request_metrics[:merged_at],
- TableReferences.merge_request_metrics[:first_deployed_to_production_at])
+ MergeRequest::Metrics.arel_table[:merged_at],
+ MergeRequest::Metrics.arel_table[:first_deployed_to_production_at])
end
def production
calculate_metric(:production,
- TableReferences.issues[:created_at],
- TableReferences.merge_request_metrics[:first_deployed_to_production_at])
+ Issue.arel_table[:created_at],
+ MergeRequest::Metrics.arel_table[:first_deployed_to_production_at])
end
private
@@ -69,9 +68,7 @@ class CycleAnalytics
cte_table,
subtract_datetimes(base_query, end_time_attrs, start_time_attrs, name.to_s))
- median_queries = Array.wrap(median_datetime(cte_table, interval_query, name))
- results = median_queries.map { |query| run_query(query) }
- extract_median(results).presence
+ median_datetime(cte_table, interval_query, name)
end
# Join table with a row for every <issue,merge_request> pair (where the merge request
@@ -79,27 +76,22 @@ class CycleAnalytics
# are loaded with an inner join, so issues / merge requests without metrics are
# automatically excluded.
def base_query
- arel_table = TableReferences.merge_requests_closing_issues
+ arel_table = MergeRequestsClosingIssues.arel_table
# Load issues
- query = arel_table.join(TableReferences.issues).on(TableReferences.issues[:id].eq(arel_table[:issue_id])).
- join(TableReferences.issue_metrics).on(TableReferences.issues[:id].eq(TableReferences.issue_metrics[:issue_id])).
- where(TableReferences.issues[:project_id].eq(@project.id)).
- where(TableReferences.issues[:deleted_at].eq(nil)).
- where(TableReferences.issues[:created_at].gteq(@from))
+ query = arel_table.join(Issue.arel_table).on(Issue.arel_table[:id].eq(arel_table[:issue_id])).
+ join(Issue::Metrics.arel_table).on(Issue.arel_table[:id].eq(Issue::Metrics.arel_table[:issue_id])).
+ where(Issue.arel_table[:project_id].eq(@project.id)).
+ where(Issue.arel_table[:deleted_at].eq(nil)).
+ where(Issue.arel_table[:created_at].gteq(@from))
# Load merge_requests
- query = query.join(TableReferences.merge_requests, Arel::Nodes::OuterJoin).
- on(TableReferences.merge_requests[:id].eq(arel_table[:merge_request_id])).
- join(TableReferences.merge_request_metrics).
- on(TableReferences.merge_requests[:id].eq(TableReferences.merge_request_metrics[:merge_request_id]))
+ query = query.join(MergeRequest.arel_table, Arel::Nodes::OuterJoin).
+ on(MergeRequest.arel_table[:id].eq(arel_table[:merge_request_id])).
+ join(MergeRequest::Metrics.arel_table).
+ on(MergeRequest.arel_table[:id].eq(MergeRequest::Metrics.arel_table[:merge_request_id]))
# Limit to merge requests that have been deployed to production after `@from`
- query.where(TableReferences.merge_request_metrics[:first_deployed_to_production_at].gteq(@from))
- end
-
- def run_query(query)
- query = query.to_sql unless query.is_a?(String)
- ActiveRecord::Base.connection.execute(query)
+ query.where(MergeRequest::Metrics.arel_table[:first_deployed_to_production_at].gteq(@from))
end
end
diff --git a/app/models/cycle_analytics/summary.rb b/app/models/cycle_analytics/summary.rb
index 04a90a8da49..53b2cacb131 100644
--- a/app/models/cycle_analytics/summary.rb
+++ b/app/models/cycle_analytics/summary.rb
@@ -6,7 +6,7 @@ class CycleAnalytics
end
def new_issues
- @project.issues.where("created_at > ?", @from).count
+ @project.issues.created_after(@from).count
end
def commits
diff --git a/app/models/cycle_analytics/table_references.rb b/app/models/cycle_analytics/table_references.rb
deleted file mode 100644
index f276723ee0e..00000000000
--- a/app/models/cycle_analytics/table_references.rb
+++ /dev/null
@@ -1,25 +0,0 @@
-class CycleAnalytics
- module TableReferences
- class << self
- def issues
- Issue.arel_table
- end
-
- def issue_metrics
- Issue::Metrics.arel_table
- end
-
- def merge_requests
- MergeRequest.arel_table
- end
-
- def merge_request_metrics
- MergeRequest::Metrics.arel_table
- end
-
- def merge_requests_closing_issues
- MergeRequestsClosingIssues.arel_table
- end
- end
- end
-end
diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index 15f349c89bb..07d7e19e70d 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -43,23 +43,30 @@ class Deployment < ActiveRecord::Base
project.repository.is_ancestor?(commit.id, sha)
end
- def update_merge_request_metrics
- if environment.update_merge_request_metrics?
- merge_requests = project.merge_requests.
- joins(:metrics).
- where(target_branch: self.ref, merge_request_metrics: { first_deployed_to_production_at: nil }).
- where("merge_request_metrics.merged_at <= ?", self.created_at)
-
- if previous_deployment
- merge_requests = merge_requests.where("merge_request_metrics.merged_at >= ?", previous_deployment.created_at)
- end
+ def update_merge_request_metrics!
+ return unless environment.update_merge_request_metrics?
+
+ merge_requests = project.merge_requests.
+ joins(:metrics).
+ where(target_branch: self.ref, merge_request_metrics: { first_deployed_to_production_at: nil }).
+ where("merge_request_metrics.merged_at <= ?", self.created_at)
- # Need to use `map` instead of `select` because MySQL doesn't allow `SELECT`ing from the same table
- # that we're updating.
- MergeRequest::Metrics.
- where(merge_request_id: merge_requests.map(&:id), first_deployed_to_production_at: nil).
- update_all(first_deployed_to_production_at: self.created_at)
+ if previous_deployment
+ merge_requests = merge_requests.where("merge_request_metrics.merged_at >= ?", previous_deployment.created_at)
end
+
+ # Need to use `map` instead of `select` because MySQL doesn't allow `SELECT`ing from the same table
+ # that we're updating.
+ merge_request_ids =
+ if Gitlab::Database.postgresql?
+ merge_requests.select(:id)
+ elsif Gitlab::Database.mysql?
+ merge_requests.map(&:id)
+ end
+
+ MergeRequest::Metrics.
+ where(merge_request_id: merge_request_ids, first_deployed_to_production_at: nil).
+ update_all(first_deployed_to_production_at: self.created_at)
end
def previous_deployment
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 6a264ace165..25ba38a1cff 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -24,7 +24,6 @@ class Issue < ActiveRecord::Base
has_many :events, as: :target, dependent: :destroy
has_many :merge_requests_closing_issues, class_name: 'MergeRequestsClosingIssues'
- has_many :closed_by_merge_requests, through: :merge_requests_closing_issues, source: :merge_request
validates :project, presence: true
@@ -39,6 +38,8 @@ class Issue < ActiveRecord::Base
scope :order_due_date_asc, -> { reorder('issues.due_date IS NULL, issues.due_date ASC') }
scope :order_due_date_desc, -> { reorder('issues.due_date IS NULL, issues.due_date DESC') }
+ scope :created_after, -> (datetime) { where("created_at >= ?", datetime) }
+
attr_spammable :title, spam_title: true
attr_spammable :description, spam_description: true
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 0ac291ce1a0..c05718f4d5a 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -17,7 +17,6 @@ class MergeRequest < ActiveRecord::Base
has_many :events, as: :target, dependent: :destroy
has_many :merge_requests_closing_issues, class_name: 'MergeRequestsClosingIssues'
- has_many :issues_closed, through: :merge_requests_closing_issues, source: :issue
serialize :merge_params, Hash
@@ -524,11 +523,8 @@ class MergeRequest < ActiveRecord::Base
# Return the set of issues that will be closed if this merge request is accepted.
def closes_issues(current_user = self.author)
if target_branch == project.default_branch
- messages = if merge_request_diff
- commits.map(&:safe_message) << description
- else
- [description]
- end
+ messages = [description]
+ messages.concat(commits.map(&:safe_message)) if merge_request_diff
Gitlab::ClosingIssueExtractor.new(project, current_user).
closed_by_message(messages.join("\n"))
diff --git a/app/models/merge_requests_closing_issues.rb b/app/models/merge_requests_closing_issues.rb
index cd49076002d..ab597c37947 100644
--- a/app/models/merge_requests_closing_issues.rb
+++ b/app/models/merge_requests_closing_issues.rb
@@ -2,8 +2,6 @@ class MergeRequestsClosingIssues < ActiveRecord::Base
belongs_to :merge_request
belongs_to :issue
- validates_uniqueness_of :merge_request_id, scope: :issue_id
-
- validates_presence_of :merge_request
- validates_presence_of :issue
+ validates :merge_request_id, uniqueness: { scope: :issue_id }, presence: true
+ validates :issue_id, presence: true
end
diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb
index e958e91d612..799ad3e1bd0 100644
--- a/app/services/create_deployment_service.rb
+++ b/app/services/create_deployment_service.rb
@@ -13,7 +13,7 @@ class CreateDeploymentService < BaseService
deployable: deployable
)
- deployment.update_merge_request_metrics
+ deployment.update_merge_request_metrics!
deployment
end
diff --git a/app/views/projects/pipelines/_head.html.haml b/app/views/projects/pipelines/_head.html.haml
index fa1470f5fbc..a97674e84ba 100644
--- a/app/views/projects/pipelines/_head.html.haml
+++ b/app/views/projects/pipelines/_head.html.haml
@@ -20,7 +20,8 @@
%span
Environments
- = nav_link(controller: %w(cycle_analytics)) do
- = link_to project_cycle_analytics_path(@project), title: 'Cycle Analytics' do
- %span
- Cycle Analytics
+ - if current_user
+ = nav_link(controller: %w(cycle_analytics)) do
+ = link_to project_cycle_analytics_path(@project), title: 'Cycle Analytics' do
+ %span
+ Cycle Analytics
diff --git a/db/migrate/20160825052008_add_table_merge_request_metrics.rb b/db/migrate/20160825052008_add_table_merge_request_metrics.rb
index bde9238b3c6..e01cc5038b9 100644
--- a/db/migrate/20160825052008_add_table_merge_request_metrics.rb
+++ b/db/migrate/20160825052008_add_table_merge_request_metrics.rb
@@ -29,7 +29,7 @@ class AddTableMergeRequestMetrics < ActiveRecord::Migration
t.datetime 'latest_build_started_at'
t.datetime 'latest_build_finished_at'
- t.datetime 'first_deployed_to_production_at'
+ t.datetime 'first_deployed_to_production_at', index: true
t.datetime 'merged_at'
t.timestamps null: false
diff --git a/lib/gitlab/database/median.rb b/lib/gitlab/database/median.rb
index 94f060bd2c7..9adacd64934 100644
--- a/lib/gitlab/database/median.rb
+++ b/lib/gitlab/database/median.rb
@@ -3,11 +3,15 @@ module Gitlab
module Database
module Median
def median_datetime(arel_table, query_so_far, column_sym)
- if Gitlab::Database.postgresql?
- pg_median_datetime(arel_table, query_so_far, column_sym)
- elsif Gitlab::Database.mysql?
- mysql_median_datetime(arel_table, query_so_far, column_sym)
- end
+ median_queries =
+ if Gitlab::Database.postgresql?
+ pg_median_datetime_sql(arel_table, query_so_far, column_sym)
+ elsif Gitlab::Database.mysql?
+ mysql_median_datetime_sql(arel_table, query_so_far, column_sym)
+ end
+
+ results = Array.wrap(median_queries).map { |query| Util.run_query(query) }
+ extract_median(results).presence
end
def extract_median(results)
@@ -22,7 +26,7 @@ module Gitlab
end
end
- def mysql_median_datetime(arel_table, query_so_far, column_sym)
+ def mysql_median_datetime_sql(arel_table, query_so_far, column_sym)
query = arel_table.
from(arel_table.project(Arel.sql('*')).order(arel_table[column_sym]).as(arel_table.table_name)).
project(average([arel_table[column_sym]], 'median')).
@@ -44,7 +48,7 @@ module Gitlab
]
end
- def pg_median_datetime(arel_table, query_so_far, column_sym)
+ def pg_median_datetime_sql(arel_table, query_so_far, column_sym)
# Create a CTE with the column we're operating on, row number (after sorting by the column
# we're operating on), and count of the table we're operating on (duplicated across) all rows
# of the CTE. For example, if we're looking to find the median of the `projects.star_count`
@@ -59,10 +63,11 @@ module Gitlab
cte = Arel::Nodes::As.new(
cte_table,
arel_table.
- project(arel_table[column_sym].as(column_sym.to_s),
- Arel::Nodes::Over.new(Arel::Nodes::NamedFunction.new("row_number", []),
- Arel::Nodes::Window.new.order(arel_table[column_sym])).as('row_id'),
- arel_table.project("COUNT(1)").as('ct')).
+ project(
+ arel_table[column_sym].as(column_sym.to_s),
+ Arel::Nodes::Over.new(Arel::Nodes::NamedFunction.new("row_number", []),
+ Arel::Nodes::Window.new.order(arel_table[column_sym])).as('row_id'),
+ arel_table.project("COUNT(1)").as('ct')).
# Disallow negative values
where(arel_table[column_sym].gteq(zero_interval)))
@@ -70,9 +75,11 @@ module Gitlab
# by 'where cte.row_id between cte.ct / 2.0 AND cte.ct / 2.0 + 1'). Find the average of the
# selected rows, and this is the median value.
cte_table.project(average([extract_epoch(cte_table[column_sym])], "median")).
- where(Arel::Nodes::Between.new(cte_table[:row_id],
- Arel::Nodes::And.new([(cte_table[:ct] / Arel.sql('2.0')),
- (cte_table[:ct] / Arel.sql('2.0') + 1)]))).
+ where(Arel::Nodes::Between.new(
+ cte_table[:row_id],
+ Arel::Nodes::And.new(
+ [(cte_table[:ct] / Arel.sql('2.0')),
+ (cte_table[:ct] / Arel.sql('2.0') + 1)]))).
with(query_so_far, cte)
end
diff --git a/lib/gitlab/database/util.rb b/lib/gitlab/database/util.rb
new file mode 100644
index 00000000000..12b68deae89
--- /dev/null
+++ b/lib/gitlab/database/util.rb
@@ -0,0 +1,12 @@
+module Gitlab
+ module Database
+ module Util
+ class << self
+ def run_query(query)
+ query = query.to_sql unless query.is_a?(String)
+ ActiveRecord::Base.connection.execute(query)
+ end
+ end
+ end
+ end
+end
diff --git a/spec/models/cycle_analytics/code_spec.rb b/spec/models/cycle_analytics/code_spec.rb
index e81c78df8f0..b9381e33914 100644
--- a/spec/models/cycle_analytics/code_spec.rb
+++ b/spec/models/cycle_analytics/code_spec.rb
@@ -1,19 +1,28 @@
require 'spec_helper'
describe 'CycleAnalytics#code', feature: true do
+ extend CycleAnalyticsHelpers::TestGeneration
+
let(:project) { create(:project) }
let(:from_date) { 10.days.ago }
let(:user) { create(:user, :admin) }
subject { CycleAnalytics.new(project, from: from_date) }
- generate_cycle_analytics_spec(phase: :code,
- data_fn: -> (context) { { issue: context.create(:issue, project: context.project) } },
- start_time_conditions: [["issue mentioned in a commit", -> (context, data) { context.create_commit_referencing_issue(data[:issue]) }]],
- end_time_conditions: [["merge request that closes issue is created", -> (context, data) { context.create_merge_request_closing_issue(data[:issue]) }]],
- post_fn: -> (context, data) do
- context.merge_merge_requests_closing_issue(data[:issue])
- context.deploy_master
- end)
+ generate_cycle_analytics_spec(
+ phase: :code,
+ data_fn: -> (context) { { issue: context.create(:issue, project: context.project) } },
+ start_time_conditions: [["issue mentioned in a commit",
+ -> (context, data) do
+ context.create_commit_referencing_issue(data[:issue])
+ end]],
+ end_time_conditions: [["merge request that closes issue is created",
+ -> (context, data) do
+ context.create_merge_request_closing_issue(data[:issue])
+ end]],
+ post_fn: -> (context, data) do
+ context.merge_merge_requests_closing_issue(data[:issue])
+ context.deploy_master
+ end)
context "when a regular merge request (that doesn't close the issue) is created" do
it "returns nil" do
diff --git a/spec/models/cycle_analytics/issue_spec.rb b/spec/models/cycle_analytics/issue_spec.rb
index 8d7d03193f0..e9cc71254ab 100644
--- a/spec/models/cycle_analytics/issue_spec.rb
+++ b/spec/models/cycle_analytics/issue_spec.rb
@@ -1,23 +1,36 @@
require 'spec_helper'
describe 'CycleAnalytics#issue', models: true do
+ extend CycleAnalyticsHelpers::TestGeneration
+
let(:project) { create(:project) }
let(:from_date) { 10.days.ago }
let(:user) { create(:user, :admin) }
subject { CycleAnalytics.new(project, from: from_date) }
- generate_cycle_analytics_spec(phase: :issue,
- data_fn: -> (context) { { issue: context.build(:issue, project: context.project) } },
- start_time_conditions: [["issue created", -> (context, data) { data[:issue].save }]],
- end_time_conditions: [["issue associated with a milestone", -> (context, data) { data[:issue].update(milestone: context.create(:milestone, project: context.project)) if data[:issue].persisted? }],
- ["list label added to issue", -> (context, data) { data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id]) if data[:issue].persisted? }]],
- post_fn: -> (context, data) do
- if data[:issue].persisted?
- context.create_merge_request_closing_issue(data[:issue].reload)
- context.merge_merge_requests_closing_issue(data[:issue])
- context.deploy_master
- end
- end)
+ generate_cycle_analytics_spec(
+ phase: :issue,
+ data_fn: -> (context) { { issue: context.build(:issue, project: context.project) } },
+ start_time_conditions: [["issue created", -> (context, data) { data[:issue].save }]],
+ end_time_conditions: [["issue associated with a milestone",
+ -> (context, data) do
+ if data[:issue].persisted?
+ data[:issue].update(milestone: context.create(:milestone, project: context.project))
+ end
+ end],
+ ["list label added to issue",
+ -> (context, data) do
+ if data[:issue].persisted?
+ data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id])
+ end
+ end]],
+ post_fn: -> (context, data) do
+ if data[:issue].persisted?
+ context.create_merge_request_closing_issue(data[:issue].reload)
+ context.merge_merge_requests_closing_issue(data[:issue])
+ context.deploy_master
+ end
+ end)
context "when a regular label (instead of a list label) is added to the issue" do
it "returns nil" do
diff --git a/spec/models/cycle_analytics/plan_spec.rb b/spec/models/cycle_analytics/plan_spec.rb
index 4a66615e53b..5b8c96dc992 100644
--- a/spec/models/cycle_analytics/plan_spec.rb
+++ b/spec/models/cycle_analytics/plan_spec.rb
@@ -1,26 +1,38 @@
require 'spec_helper'
describe 'CycleAnalytics#plan', feature: true do
+ extend CycleAnalyticsHelpers::TestGeneration
+
let(:project) { create(:project) }
let(:from_date) { 10.days.ago }
let(:user) { create(:user, :admin) }
subject { CycleAnalytics.new(project, from: from_date) }
- generate_cycle_analytics_spec(phase: :plan,
- data_fn: -> (context) do
- {
- issue: context.create(:issue, project: context.project),
- branch_name: context.random_git_name
- }
- end,
- start_time_conditions: [["issue associated with a milestone", -> (context, data) { data[:issue].update(milestone: context.create(:milestone, project: context.project)) }],
- ["list label added to issue", -> (context, data) { data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id]) }]],
- end_time_conditions: [["issue mentioned in a commit", -> (context, data) { context.create_commit_referencing_issue(data[:issue], branch_name: data[:branch_name]) }]],
- post_fn: -> (context, data) do
- context.create_merge_request_closing_issue(data[:issue], source_branch: data[:branch_name])
- context.merge_merge_requests_closing_issue(data[:issue])
- context.deploy_master
- end)
+ generate_cycle_analytics_spec(
+ phase: :plan,
+ data_fn: -> (context) do
+ {
+ issue: context.create(:issue, project: context.project),
+ branch_name: context.random_git_name
+ }
+ end,
+ start_time_conditions: [["issue associated with a milestone",
+ -> (context, data) do
+ data[:issue].update(milestone: context.create(:milestone, project: context.project))
+ end],
+ ["list label added to issue",
+ -> (context, data) do
+ data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id])
+ end]],
+ end_time_conditions: [["issue mentioned in a commit",
+ -> (context, data) do
+ context.create_commit_referencing_issue(data[:issue], branch_name: data[:branch_name])
+ end]],
+ post_fn: -> (context, data) do
+ context.create_merge_request_closing_issue(data[:issue], source_branch: data[:branch_name])
+ context.merge_merge_requests_closing_issue(data[:issue])
+ context.deploy_master
+ end)
context "when a regular label (instead of a list label) is added to the issue" do
it "returns nil" do
diff --git a/spec/models/cycle_analytics/production_spec.rb b/spec/models/cycle_analytics/production_spec.rb
index 703f8d5f782..1f5e5cab92d 100644
--- a/spec/models/cycle_analytics/production_spec.rb
+++ b/spec/models/cycle_analytics/production_spec.rb
@@ -1,28 +1,31 @@
require 'spec_helper'
describe 'CycleAnalytics#production', feature: true do
+ extend CycleAnalyticsHelpers::TestGeneration
+
let(:project) { create(:project) }
let(:from_date) { 10.days.ago }
let(:user) { create(:user, :admin) }
subject { CycleAnalytics.new(project, from: from_date) }
- generate_cycle_analytics_spec(phase: :production,
- data_fn: -> (context) { { issue: context.build(:issue, project: context.project) } },
- start_time_conditions: [["issue is created", -> (context, data) { data[:issue].save }]],
- before_end_fn: lambda do |context, data|
- context.create_merge_request_closing_issue(data[:issue])
- context.merge_merge_requests_closing_issue(data[:issue])
- end,
- end_time_conditions:
- [["merge request that closes issue is deployed to production", -> (context, data) { context.deploy_master }],
- ["production deploy happens after merge request is merged (along with other changes)",
- lambda do |context, data|
- # Make other changes on master
- sha = context.project.repository.commit_file(context.user, context.random_git_name, "content", "commit message", 'master', false)
- context.project.repository.commit(sha)
-
- context.deploy_master
- end]])
+ generate_cycle_analytics_spec(
+ phase: :production,
+ data_fn: -> (context) { { issue: context.build(:issue, project: context.project) } },
+ start_time_conditions: [["issue is created", -> (context, data) { data[:issue].save }]],
+ before_end_fn: lambda do |context, data|
+ context.create_merge_request_closing_issue(data[:issue])
+ context.merge_merge_requests_closing_issue(data[:issue])
+ end,
+ end_time_conditions:
+ [["merge request that closes issue is deployed to production", -> (context, data) { context.deploy_master }],
+ ["production deploy happens after merge request is merged (along with other changes)",
+ lambda do |context, data|
+ # Make other changes on master
+ sha = context.project.repository.commit_file(context.user, context.random_git_name, "content", "commit message", 'master', false)
+ context.project.repository.commit(sha)
+
+ context.deploy_master
+ end]])
context "when a regular merge request (that doesn't close the issue) is merged and deployed" do
it "returns nil" do
diff --git a/spec/models/cycle_analytics/review_spec.rb b/spec/models/cycle_analytics/review_spec.rb
index 100ce11299a..b6e26d8f261 100644
--- a/spec/models/cycle_analytics/review_spec.rb
+++ b/spec/models/cycle_analytics/review_spec.rb
@@ -1,16 +1,25 @@
require 'spec_helper'
describe 'CycleAnalytics#review', feature: true do
+ extend CycleAnalyticsHelpers::TestGeneration
+
let(:project) { create(:project) }
let(:from_date) { 10.days.ago }
let(:user) { create(:user, :admin) }
subject { CycleAnalytics.new(project, from: from_date) }
- generate_cycle_analytics_spec(phase: :review,
- data_fn: -> (context) { { issue: context.create(:issue, project: context.project) } },
- start_time_conditions: [["merge request that closes issue is created", -> (context, data) { context.create_merge_request_closing_issue(data[:issue]) }]],
- end_time_conditions: [["merge request that closes issue is merged", -> (context, data) { context.merge_merge_requests_closing_issue(data[:issue]) }]],
- post_fn: -> (context, data) { context.deploy_master })
+ generate_cycle_analytics_spec(
+ phase: :review,
+ data_fn: -> (context) { { issue: context.create(:issue, project: context.project) } },
+ start_time_conditions: [["merge request that closes issue is created",
+ -> (context, data) do
+ context.create_merge_request_closing_issue(data[:issue])
+ end]],
+ end_time_conditions: [["merge request that closes issue is merged",
+ -> (context, data) do
+ context.merge_merge_requests_closing_issue(data[:issue])
+ end]],
+ post_fn: -> (context, data) { context.deploy_master })
context "when a regular merge request (that doesn't close the issue) is created and merged" do
it "returns nil" do
diff --git a/spec/models/cycle_analytics/staging_spec.rb b/spec/models/cycle_analytics/staging_spec.rb
index 105f566f41c..af1c4477ddb 100644
--- a/spec/models/cycle_analytics/staging_spec.rb
+++ b/spec/models/cycle_analytics/staging_spec.rb
@@ -1,26 +1,41 @@
require 'spec_helper'
describe 'CycleAnalytics#staging', feature: true do
+ extend CycleAnalyticsHelpers::TestGeneration
+
let(:project) { create(:project) }
let(:from_date) { 10.days.ago }
let(:user) { create(:user, :admin) }
subject { CycleAnalytics.new(project, from: from_date) }
- generate_cycle_analytics_spec(phase: :staging,
- data_fn: lambda do |context|
- issue = context.create(:issue, project: context.project)
- { issue: issue, merge_request: context.create_merge_request_closing_issue(issue) }
- end,
- start_time_conditions: [["merge request that closes issue is merged", -> (context, data) { context.merge_merge_requests_closing_issue(data[:issue])} ]],
- end_time_conditions: [["merge request that closes issue is deployed to production", -> (context, data) { context.deploy_master }],
- ["production deploy happens after merge request is merged (along with other changes)",
- lambda do |context, data|
- # Make other changes on master
- sha = context.project.repository.commit_file(context.user, context.random_git_name, "content", "commit message", 'master', false)
- context.project.repository.commit(sha)
-
- context.deploy_master
- end]])
+ generate_cycle_analytics_spec(
+ phase: :staging,
+ data_fn: lambda do |context|
+ issue = context.create(:issue, project: context.project)
+ { issue: issue, merge_request: context.create_merge_request_closing_issue(issue) }
+ end,
+ start_time_conditions: [["merge request that closes issue is merged",
+ -> (context, data) do
+ context.merge_merge_requests_closing_issue(data[:issue])
+ end ]],
+ end_time_conditions: [["merge request that closes issue is deployed to production",
+ -> (context, data) do
+ context.deploy_master
+ end],
+ ["production deploy happens after merge request is merged (along with other changes)",
+ lambda do |context, data|
+ # Make other changes on master
+ sha = context.project.repository.commit_file(
+ context.user,
+ context.random_git_name,
+ "content",
+ "commit message",
+ 'master',
+ false)
+ context.project.repository.commit(sha)
+
+ context.deploy_master
+ end]])
context "when a regular merge request (that doesn't close the issue) is merged and deployed" do
it "returns nil" do
diff --git a/spec/models/cycle_analytics/test_spec.rb b/spec/models/cycle_analytics/test_spec.rb
index 79edc29c173..89ace0b2742 100644
--- a/spec/models/cycle_analytics/test_spec.rb
+++ b/spec/models/cycle_analytics/test_spec.rb
@@ -1,26 +1,27 @@
require 'spec_helper'
describe 'CycleAnalytics#test', feature: true do
+ extend CycleAnalyticsHelpers::TestGeneration
+
let(:project) { create(:project) }
let(:from_date) { 10.days.ago }
let(:user) { create(:user, :admin) }
subject { CycleAnalytics.new(project, from: from_date) }
- generate_cycle_analytics_spec(phase: :test,
- data_fn: lambda do |context|
- issue = context.create(:issue, project: context.project)
- merge_request = context.create_merge_request_closing_issue(issue)
- {
- pipeline: context.create(:ci_pipeline, ref: merge_request.source_branch, sha: merge_request.diff_head_sha, project: context.project),
- issue: issue
- }
- end,
- start_time_conditions: [["pipeline is started", -> (context, data) { data[:pipeline].run! }]],
- end_time_conditions: [["pipeline is finished", -> (context, data) { data[:pipeline].succeed! }]],
- post_fn: -> (context, data) do
- context.merge_merge_requests_closing_issue(data[:issue])
- context.deploy_master
- end)
+ generate_cycle_analytics_spec(
+ phase: :test,
+ data_fn: lambda do |context|
+ issue = context.create(:issue, project: context.project)
+ merge_request = context.create_merge_request_closing_issue(issue)
+ pipeline = context.create(:ci_pipeline, ref: merge_request.source_branch, sha: merge_request.diff_head_sha, project: context.project)
+ { pipeline: pipeline, issue: issue }
+ end,
+ start_time_conditions: [["pipeline is started", -> (context, data) { data[:pipeline].run! }]],
+ end_time_conditions: [["pipeline is finished", -> (context, data) { data[:pipeline].succeed! }]],
+ post_fn: -> (context, data) do
+ context.merge_merge_requests_closing_issue(data[:issue])
+ context.deploy_master
+ end)
context "when the pipeline is for a regular merge request (that doesn't close an issue)" do
it "returns nil" do
diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb
index a0614abcf62..ff6bad6aaba 100644
--- a/spec/services/merge_requests/create_service_spec.rb
+++ b/spec/services/merge_requests/create_service_spec.rb
@@ -108,7 +108,7 @@ describe MergeRequests::CreateService, services: true do
allow(service).to receive(:execute_hooks)
merge_request = service.execute
- expect(merge_request.issues_closed).to match_array([first_issue, second_issue])
+ expect(merge_request.reload.closes_issues).to match_array([first_issue, second_issue])
end
end
end
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index 7ecad8d6bad..22cb122526c 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -200,7 +200,7 @@ describe MergeRequests::RefreshService, services: true do
allow(refresh_service).to receive(:execute_hooks)
refresh_service.execute(@oldrev, @newrev, 'refs/heads/feature')
- expect(merge_request.reload.issues_closed).to eq([issue])
+ expect(merge_request.reload.closes_issues).to eq([issue])
end
end
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 84b0acffaac..68dd0382a3d 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -274,7 +274,7 @@ describe MergeRequests::UpdateService, services: true do
allow(service).to receive(:execute_hooks)
service.execute(merge_request)
- expect(merge_request.reload.issues_closed).to match_array([first_issue, second_issue])
+ expect(merge_request.reload.closes_issues).to match_array([first_issue, second_issue])
end
it 'removes `MergeRequestsClosingIssues` records when issues are not closed anymore' do
@@ -288,13 +288,13 @@ describe MergeRequests::UpdateService, services: true do
merge_request = MergeRequests::CreateService.new(project, user, opts).execute
- expect(merge_request.issues_closed).to match_array([first_issue, second_issue])
+ expect(merge_request.reload.closes_issues).to match_array([first_issue, second_issue])
service = described_class.new(project, user, description: "not closing any issues")
allow(service).to receive(:execute_hooks)
service.execute(merge_request.reload)
- expect(merge_request.reload.issues_closed).to be_empty
+ expect(merge_request.reload.closes_issues).to be_empty
end
end
end
diff --git a/spec/support/cycle_analytics_helpers/test_generation.rb b/spec/support/cycle_analytics_helpers/test_generation.rb
index 4d5ce86691f..8e19a6c92e2 100644
--- a/spec/support/cycle_analytics_helpers/test_generation.rb
+++ b/spec/support/cycle_analytics_helpers/test_generation.rb
@@ -159,7 +159,3 @@ module CycleAnalyticsHelpers
end
end
end
-
-RSpec.configure do |config|
- config.extend CycleAnalyticsHelpers::TestGeneration
-end
--
GitLab