diff options
24 files changed, 913 insertions, 3 deletions
@@ -316,6 +316,7 @@ group :test do gem 'webmock', '~> 1.21.0' gem 'test_after_commit', '~> 0.4.2' gem 'sham_rack', '~> 1.3.6' + gem 'timecop', '~> 0.8.0' end group :production do diff --git a/Gemfile.lock b/Gemfile.lock index ab573b4d31e..fbdde038ae9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -969,6 +969,7 @@ DEPENDENCIES teaspoon-jasmine (~> 2.2.0) test_after_commit (~> 0.4.2) thin (~> 1.7.0) + timecop (~> 0.8.0) turbolinks (~> 2.5.0) u2f (~> 0.2.1) uglifier (~> 2.7.2) diff --git a/app/controllers/projects/cycle_analytics_controller.rb b/app/controllers/projects/cycle_analytics_controller.rb new file mode 100644 index 00000000000..68cc79fb166 --- /dev/null +++ b/app/controllers/projects/cycle_analytics_controller.rb @@ -0,0 +1,23 @@ +class Projects::CycleAnalyticsController < Projects::ApplicationController + before_action :authorize_read_cycle_analytics! + + def show + @cycle_analytics = CycleAnalytics.new(@project, from: parse_start_date) + end + + private + + def parse_start_date + case cycle_analytics_params[:start_date] + when '30' then 30.days.ago + when '90' then 90.days.ago + else 90.days.ago + end + end + + def cycle_analytics_params + return {} unless params[:cycle_analytics].present? + + { start_date: params[:cycle_analytics][:start_date] } + end +end diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb new file mode 100644 index 00000000000..a52b5aff559 --- /dev/null +++ b/app/models/cycle_analytics.rb @@ -0,0 +1,71 @@ +class CycleAnalytics + attr_reader :from + + def initialize(project, from:) + @project = project + @from = from + end + + def issue + calculate_metric(Queries::issues(@project, created_after: @from), + -> (data_point) { data_point[:issue].created_at }, + [Queries::issue_first_associated_with_milestone_at, Queries::issue_first_added_to_list_label_at]) + end + + def plan + calculate_metric(Queries::issues(@project, created_after: @from), + [Queries::issue_first_associated_with_milestone_at, Queries::issue_first_added_to_list_label_at], + Queries::issue_first_mentioned_in_commit_at) + end + + def code + calculate_metric(Queries::merge_requests_closing_issues(@project, created_after: @from), + Queries::issue_first_mentioned_in_commit_at, + -> (data_point) { data_point[:merge_request].created_at }) + end + + def test + calculate_metric(Queries::merge_requests_closing_issues(@project, created_after: @from), + Queries::merge_request_build_started_at, + Queries::merge_request_build_finished_at) + end + + def review + calculate_metric(Queries::merge_requests_closing_issues(@project, created_after: @from), + -> (data_point) { data_point[:merge_request].created_at }, + Queries::merge_request_merged_at) + end + + def staging + calculate_metric(Queries::merge_requests_closing_issues(@project, created_after: @from), + Queries::merge_request_merged_at, + Queries::merge_request_deployed_to_production_at) + end + + def production + calculate_metric(Queries::merge_requests_closing_issues(@project, created_after: @from), + -> (data_point) { data_point[:issue].created_at }, + Queries::merge_request_deployed_to_production_at) + end + + private + + def calculate_metric(data, start_time_fns, end_time_fns) + times = data.map do |data_point| + start_time = Array.wrap(start_time_fns).map { |fn| fn[data_point] }.compact.first + end_time = Array.wrap(end_time_fns).map { |fn| fn[data_point] }.compact.first + + if start_time.present? && end_time.present? && end_time >= start_time + end_time - start_time + end + end + + median(times.compact.sort) + end + + def median(coll) + return if coll.empty? + size = coll.length + (coll[size / 2] + coll[(size - 1) / 2]) / 2.0 + end +end diff --git a/app/models/cycle_analytics/queries.rb b/app/models/cycle_analytics/queries.rb new file mode 100644 index 00000000000..32c49ec52ba --- /dev/null +++ b/app/models/cycle_analytics/queries.rb @@ -0,0 +1,121 @@ +class CycleAnalytics + module Queries + class << self + def issues(project, created_after:) + project.issues.where("created_at >= ?", created_after).map { |issue| { issue: issue } } + end + + def merge_requests_closing_issues(project, options = {}) + issues(project, options).map do |data_point| + merge_requests = data_point[:issue].closed_by_merge_requests(nil, check_if_open: false) + merge_requests.map { |merge_request| { issue: data_point[:issue], merge_request: merge_request } } + end.flatten + end + + def issue_first_associated_with_milestone_at + lambda do |data_point| + issue = data_point[:issue] + issue.metrics.first_associated_with_milestone_at if issue.metrics.present? + end + end + + def issue_first_added_to_list_label_at + lambda do |data_point| + issue = data_point[:issue] + issue.metrics.first_added_to_board_at if issue.metrics.present? + end + end + + def issue_first_mentioned_in_commit_at + lambda do |data_point| + issue = data_point[:issue] + commits_mentioning_issue = issue.notes.system.map { |note| note.all_references.commits }.flatten + commits_mentioning_issue.map(&:committed_date).min if commits_mentioning_issue.present? + end + end + + def merge_request_first_closed_at + lambda do |data_point| + merge_request = data_point[:merge_request] + merge_request.metrics.first_closed_at if merge_request.metrics.present? + end + end + + def merge_request_merged_at + lambda do |data_point| + merge_request = data_point[:merge_request] + merge_request.metrics.merged_at if merge_request.metrics.present? + end + end + + def merge_request_build_started_at + lambda do |data_point| + merge_request = data_point[:merge_request] + tip = merge_request.commits.first + return unless tip + + pipeline = Ci::Pipeline.success.find_by_sha(tip.sha) + pipeline.started_at if pipeline + end + end + + def merge_request_build_finished_at + lambda do |data_point| + merge_request = data_point[:merge_request] + tip = merge_request.commits.first + return unless tip + + pipeline = Ci::Pipeline.success.find_by_sha(tip.sha) + pipeline.finished_at if pipeline + end + end + + def merge_request_deployed_to_any_environment_at + lambda do |data_point| + merge_request = data_point[:merge_request] + if merge_request.metrics.present? + deployments = Deployment.where(ref: merge_request.target_branch).where("created_at > ?", merge_request.metrics.merged_at) + deployment = deployments.order(:created_at).first + deployment.created_at if deployment + end + end + end + + def merge_request_deployed_to_production_at + lambda do |data_point| + merge_request = data_point[:merge_request] + if merge_request.metrics.present? + # The first production deploy to the target branch that occurs after the merge request has been merged in. + # TODO: Does this need to account for reverts? + deployments = Deployment.joins(:environment).where(ref: merge_request.target_branch, "environments.name" => "production"). + where("deployments.created_at > ?", merge_request.metrics.merged_at) + deployment = deployments.order(:created_at).first + deployment.created_at if deployment + end + end + end + + def issue_closing_merge_request_opened_at + lambda do |data_point| + issue = data_point[:issue] + merge_requests = issue.closed_by_merge_requests(nil, check_if_open: false) + merge_requests.map(&:created_at).min if merge_requests.present? + end + end + + def merge_request_wip_flag_first_removed_at + lambda do |data_point| + merge_request = data_point[:merge_request] + merge_request.metrics.wip_flag_first_removed_at if merge_request.metrics.present? + end + end + + def merge_request_first_assigned_to_user_other_than_author_at + lambda do |data_point| + merge_request = data_point[:merge_request] + merge_request.metrics.first_assigned_to_user_other_than_author if merge_request.metrics.present? + end + end + end + end +end diff --git a/app/models/issue.rb b/app/models/issue.rb index 788611305fe..cec5e456c94 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -23,6 +23,8 @@ class Issue < ActiveRecord::Base has_many :events, as: :target, dependent: :destroy + has_one :metrics, dependent: :destroy + validates :project, presence: true scope :cared, ->(user) { where(assignee_id: user) } @@ -36,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') } + after_save :record_metrics + attr_spammable :title, spam_title: true attr_spammable :description, spam_description: true @@ -194,8 +198,8 @@ class Issue < ActiveRecord::Base # From all notes on this issue, we'll select the system notes about linked # merge requests. Of those, the MRs closing `self` are returned. - def closed_by_merge_requests(current_user = nil) - return [] unless open? + def closed_by_merge_requests(current_user = nil, check_if_open: true) + return [] if !open? && check_if_open ext = all_references(current_user) @@ -203,7 +207,11 @@ class Issue < ActiveRecord::Base note.all_references(current_user, extractor: ext) end - ext.merge_requests.select { |mr| mr.open? && mr.closes_issue?(self) } + if check_if_open + ext.merge_requests.select { |mr| mr.open? && mr.closes_issue?(self) } + else + ext.merge_requests.select { |mr| mr.closes_issue?(self) } + end end def moved? @@ -270,4 +278,9 @@ class Issue < ActiveRecord::Base def check_for_spam? project.public? end + + def record_metrics + metrics = Metrics.find_or_create_by(issue_id: self.id) + metrics.record! + end end diff --git a/app/models/issue/metrics.rb b/app/models/issue/metrics.rb new file mode 100644 index 00000000000..4436696cc1a --- /dev/null +++ b/app/models/issue/metrics.rb @@ -0,0 +1,21 @@ +class Issue::Metrics < ActiveRecord::Base + belongs_to :issue + + def record! + if issue.milestone_id.present? && self.first_associated_with_milestone_at.blank? + self.first_associated_with_milestone_at = Time.now + end + + if issue_assigned_to_list_label? && self.first_added_to_board_at.blank? + self.first_added_to_board_at = Time.now + end + + self.save if self.changed? + end + + private + + def issue_assigned_to_list_label? + issue.labels.any? { |label| label.lists.present? } + end +end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b0b1313f94a..8127c2cdd8d 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -13,6 +13,7 @@ class MergeRequest < ActiveRecord::Base has_many :merge_request_diffs, dependent: :destroy has_one :merge_request_diff, -> { order('merge_request_diffs.id DESC') } + has_one :metrics, dependent: :destroy has_many :events, as: :target, dependent: :destroy @@ -31,6 +32,8 @@ class MergeRequest < ActiveRecord::Base # when creating new merge request attr_accessor :can_be_created, :compare_commits, :compare + after_save :record_metrics + state_machine :state, initial: :opened do event :close do transition [:reopened, :opened] => :closed @@ -812,4 +815,9 @@ class MergeRequest < ActiveRecord::Base @conflicts_can_be_resolved_in_ui = false end end + + def record_metrics + metrics = Metrics.find_or_create_by(merge_request_id: self.id) + metrics.record! + end end diff --git a/app/models/merge_request/metrics.rb b/app/models/merge_request/metrics.rb new file mode 100644 index 00000000000..a0bc41c9cc1 --- /dev/null +++ b/app/models/merge_request/metrics.rb @@ -0,0 +1,23 @@ +class MergeRequest::Metrics < ActiveRecord::Base + belongs_to :merge_request + + def record! + if !merge_request.work_in_progress? && self.wip_flag_first_removed_at.blank? + self.wip_flag_first_removed_at = Time.now + end + + if merge_request.author_id != merge_request.assignee_id && self.first_assigned_to_user_other_than_author.blank? + self.first_assigned_to_user_other_than_author = Time.now + end + + if merge_request.merged? && self.merged_at.blank? + self.merged_at = Time.now + end + + if merge_request.closed? && self.first_closed_at.blank? + self.first_closed_at = Time.now + end + + self.save if self.changed? + end +end diff --git a/app/views/projects/cycle_analytics/show.html.haml b/app/views/projects/cycle_analytics/show.html.haml new file mode 100644 index 00000000000..50ac9904445 --- /dev/null +++ b/app/views/projects/cycle_analytics/show.html.haml @@ -0,0 +1,51 @@ +%h2 Cycle Analytics from #{@cycle_analytics.from} to Today + +%ul.list-group + %li.list-group-item + Issue: + - if issue = @cycle_analytics.issue + = distance_of_time_in_words issue + - else + = "<Not enough data>" + + %li.list-group-item + Plan: + - if plan = @cycle_analytics.plan + = distance_of_time_in_words plan + - else + = "<Not enough data>" + + %li.list-group-item + Code: + - if code = @cycle_analytics.code.presence + = distance_of_time_in_words code + - else + = "<Not enough data>" + + %li.list-group-item + Test: + - if test = @cycle_analytics.test.presence + = distance_of_time_in_words test + - else + = "<Not enough data>" + + %li.list-group-item + Review: + - if review = @cycle_analytics.review.presence + = distance_of_time_in_words review + - else + = "<Not enough data>" + + %li.list-group-item + Staging: + - if staging = @cycle_analytics.staging.presence + = distance_of_time_in_words staging + - else + = "<Not enough data>" + + %li.list-group-item + Production: + - if production = @cycle_analytics.production.presence + = distance_of_time_in_words production + - else + = "<Not enough data>" diff --git a/config/routes.rb b/config/routes.rb index 262a174437a..ac056148ab4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -779,6 +779,8 @@ Rails.application.routes.draw do resources :environments + resource :cycle_analytics + resources :builds, only: [:index, :show], constraints: { id: /\d+/ } do collection do post :cancel_all diff --git a/db/migrate/20160824124900_add_table_issue_metrics.rb b/db/migrate/20160824124900_add_table_issue_metrics.rb new file mode 100644 index 00000000000..256c1b7c15c --- /dev/null +++ b/db/migrate/20160824124900_add_table_issue_metrics.rb @@ -0,0 +1,36 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddTableIssueMetrics < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + create_table :issue_metrics do |t| + t.references :issue, index: { name: "index_issue_metrics" }, foreign_key: true, null: false + + t.datetime 'first_associated_with_milestone_at' + t.datetime 'first_added_to_board_at' + + t.timestamps null: false + end + end +end diff --git a/db/migrate/20160825052008_add_table_merge_request_metrics.rb b/db/migrate/20160825052008_add_table_merge_request_metrics.rb new file mode 100644 index 00000000000..809804cbfea --- /dev/null +++ b/db/migrate/20160825052008_add_table_merge_request_metrics.rb @@ -0,0 +1,38 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddTableMergeRequestMetrics < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + create_table :merge_request_metrics do |t| + t.references :merge_request, index: { name: "index_merge_request_metrics" }, foreign_key: true, null: false + + t.datetime 'wip_flag_first_removed_at' + t.datetime 'first_assigned_to_user_other_than_author' + t.datetime 'merged_at' + t.datetime 'first_closed_at' + + t.timestamps null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index c9023a02c77..2c380de3d50 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -436,6 +436,16 @@ ActiveRecord::Schema.define(version: 20160901141443) do add_index "identities", ["user_id"], name: "index_identities_on_user_id", using: :btree + create_table "issue_metrics", force: :cascade do |t| + t.integer "issue_id", null: false + t.datetime "first_associated_with_milestone_at" + t.datetime "first_added_to_board_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "issue_metrics", ["issue_id"], name: "index_issue_metrics", using: :btree + create_table "issues", force: :cascade do |t| t.string "title" t.integer "assignee_id" @@ -578,6 +588,18 @@ ActiveRecord::Schema.define(version: 20160901141443) do add_index "merge_request_diffs", ["merge_request_id"], name: "index_merge_request_diffs_on_merge_request_id", using: :btree + create_table "merge_request_metrics", force: :cascade do |t| + t.integer "merge_request_id", null: false + t.datetime "wip_flag_first_removed_at" + t.datetime "first_assigned_to_user_other_than_author" + t.datetime "merged_at" + t.datetime "first_closed_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "merge_request_metrics", ["merge_request_id"], name: "index_merge_request_metrics", using: :btree + create_table "merge_requests", force: :cascade do |t| t.string "target_branch", null: false t.string "source_branch", null: false @@ -1144,8 +1166,10 @@ ActiveRecord::Schema.define(version: 20160901141443) do add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree add_foreign_key "boards", "projects" + add_foreign_key "issue_metrics", "issues" add_foreign_key "lists", "boards" add_foreign_key "lists", "labels" + add_foreign_key "merge_request_metrics", "merge_requests" add_foreign_key "personal_access_tokens", "users" add_foreign_key "protected_branch_merge_access_levels", "protected_branches" add_foreign_key "protected_branch_push_access_levels", "protected_branches" diff --git a/spec/models/cycle_analytics/code_spec.rb b/spec/models/cycle_analytics/code_spec.rb new file mode 100644 index 00000000000..18dd4d0f1ab --- /dev/null +++ b/spec/models/cycle_analytics/code_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe 'CycleAnalytics#code', feature: true do + 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]) }]]) + + context "when a regular merge request (that doesn't close the issue) is created" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + + create_commit_referencing_issue(issue) + create_merge_request_closing_issue(issue, message: "Closes nothing") + end + + expect(subject.code).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/issue_spec.rb b/spec/models/cycle_analytics/issue_spec.rb new file mode 100644 index 00000000000..06715498d26 --- /dev/null +++ b/spec/models/cycle_analytics/issue_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe 'CycleAnalytics#issue', models: true do + let(:project) { create(:project) } + let(:from_date) { 10.days.ago } + 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? }]]) + + context "when a regular label (instead of a list label) is added to the issue" do + it "returns nil" do + 5.times do + regular_label = create(:label) + issue = create(:issue, project: project) + issue.update(label_ids: [regular_label.id]) + end + + expect(subject.issue).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/plan_spec.rb b/spec/models/cycle_analytics/plan_spec.rb new file mode 100644 index 00000000000..fa092bf6825 --- /dev/null +++ b/spec/models/cycle_analytics/plan_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe 'CycleAnalytics#plan', feature: true do + 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) { { issue: context.create(:issue, project: context.project) } }, + 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]) }]]) + + context "when a regular label (instead of a list label) is added to the issue" do + it "returns nil" do + label = create(:label) + issue = create(:issue, project: project) + issue.update(label_ids: [label.id]) + create_commit_referencing_issue(issue) + + expect(subject.issue).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/production_spec.rb b/spec/models/cycle_analytics/production_spec.rb new file mode 100644 index 00000000000..703f8d5f782 --- /dev/null +++ b/spec/models/cycle_analytics/production_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe 'CycleAnalytics#production', feature: true do + 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]]) + + context "when a regular merge request (that doesn't close the issue) is merged and deployed" do + it "returns nil" do + 5.times do + merge_request = create(:merge_request) + MergeRequests::MergeService.new(project, user).execute(merge_request) + deploy_master + end + + expect(subject.production).to be_nil + end + end + + context "when the deployment happens to a non-production environment" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + merge_request = create_merge_request_closing_issue(issue) + MergeRequests::MergeService.new(project, user).execute(merge_request) + deploy_master(environment: 'staging') + end + + expect(subject.production).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/review_spec.rb b/spec/models/cycle_analytics/review_spec.rb new file mode 100644 index 00000000000..867a90d6258 --- /dev/null +++ b/spec/models/cycle_analytics/review_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe 'CycleAnalytics#review', feature: true do + 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]) }]]) + + context "when a regular merge request (that doesn't close the issue) is created and merged" do + it "returns nil" do + 5.times do + MergeRequests::MergeService.new(project, user).execute(create(:merge_request)) + end + + expect(subject.review).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/staging_spec.rb b/spec/models/cycle_analytics/staging_spec.rb new file mode 100644 index 00000000000..105f566f41c --- /dev/null +++ b/spec/models/cycle_analytics/staging_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe 'CycleAnalytics#staging', feature: true do + 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]]) + + context "when a regular merge request (that doesn't close the issue) is merged and deployed" do + it "returns nil" do + 5.times do + merge_request = create(:merge_request) + MergeRequests::MergeService.new(project, user).execute(merge_request) + deploy_master + end + + expect(subject.staging).to be_nil + end + end + + context "when the deployment happens to a non-production environment" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + merge_request = create_merge_request_closing_issue(issue) + MergeRequests::MergeService.new(project, user).execute(merge_request) + deploy_master(environment: 'staging') + end + + expect(subject.staging).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/test_spec.rb b/spec/models/cycle_analytics/test_spec.rb new file mode 100644 index 00000000000..aa7faa74d38 --- /dev/null +++ b/spec/models/cycle_analytics/test_spec.rb @@ -0,0 +1,74 @@ +require 'spec_helper' + +describe 'CycleAnalytics#test', feature: true do + 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: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha) } + end, + start_time_conditions: [["pipeline is started", -> (context, data) { data[:pipeline].run! }]], + end_time_conditions: [["pipeline is finished", -> (context, data) { data[:pipeline].succeed! }]]) + + context "when the pipeline is for a regular merge request (that doesn't close an issue)" do + it "returns nil" do + 5.times do + merge_request = create(:merge_request) + pipeline = create(:ci_pipeline, ref: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha) + + pipeline.run! + pipeline.succeed! + end + + expect(subject.test).to be_nil + end + end + + context "when the pipeline is not for a merge request" do + it "returns nil" do + 5.times do + pipeline = create(:ci_pipeline, ref: "refs/heads/master", sha: project.repository.commit('master').sha) + + pipeline.run! + pipeline.succeed! + end + + expect(subject.test).to be_nil + end + end + + context "when the pipeline is dropped (failed)" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + merge_request = create_merge_request_closing_issue(issue) + pipeline = create(:ci_pipeline, ref: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha) + + pipeline.run! + pipeline.drop! + end + + expect(subject.test).to be_nil + end + end + + context "when the pipeline is cancelled" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + merge_request = create_merge_request_closing_issue(issue) + pipeline = create(:ci_pipeline, ref: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha) + + pipeline.run! + pipeline.cancel! + end + + expect(subject.test).to be_nil + end + end +end diff --git a/spec/support/cycle_analytics_helpers.rb b/spec/support/cycle_analytics_helpers.rb new file mode 100644 index 00000000000..c5fe1170423 --- /dev/null +++ b/spec/support/cycle_analytics_helpers.rb @@ -0,0 +1,41 @@ +module CycleAnalyticsHelpers + def create_commit_referencing_issue(issue) + sha = project.repository.commit_file(user, random_git_name, "content", "Commit for ##{issue.iid}", "master", false) + commit = project.repository.commit(sha) + commit.create_cross_references!(user) + end + + def create_merge_request_closing_issue(issue, message: nil) + source_branch = random_git_name + project.repository.add_branch(user, source_branch, 'master') + sha = project.repository.commit_file(user, random_git_name, "content", "commit message", source_branch, false) + project.repository.commit(sha) + + opts = { + title: 'Awesome merge_request', + description: message || "Fixes #{issue.to_reference}", + source_branch: source_branch, + target_branch: 'master' + } + + MergeRequests::CreateService.new(project, user, opts).execute + end + + def merge_merge_requests_closing_issue(issue) + merge_requests = issue.closed_by_merge_requests + merge_requests.each { |merge_request| MergeRequests::MergeService.new(project, user).execute(merge_request) } + end + + def deploy_master(environment: 'production') + CreateDeploymentService.new(project, user, { + environment: environment, + ref: 'master', + tag: false, + sha: project.repository.commit('master').sha + }).execute + end +end + +RSpec.configure do |config| + config.include CycleAnalyticsHelpers +end diff --git a/spec/support/cycle_analytics_helpers/test_generation.rb b/spec/support/cycle_analytics_helpers/test_generation.rb new file mode 100644 index 00000000000..52609524564 --- /dev/null +++ b/spec/support/cycle_analytics_helpers/test_generation.rb @@ -0,0 +1,154 @@ +# rubocop:disable Metrics/AbcSize + +# Note: The ABC size is large here because we have a method generating test cases with +# multiple nested contexts. This shouldn't count as a violation. + +module CycleAnalyticsHelpers + module TestGeneration + # Generate the most common set of specs that all cycle analytics phases need to have. + # + # Arguments: + # + # phase: Which phase are we testing? Will call `CycleAnalytics.new.send(phase)` for the final assertion + # data_fn: A function that returns a hash, constituting initial data for the test case + # start_time_conditions: An array of `conditions`. Each condition is an tuple of `condition_name` and `condition_fn`. `condition_fn` is called with + # `context` (no lexical scope, so need to do `context.create` for factories, for example) and `data` (from the `data_fn`). + # Each `condition_fn` is expected to implement a case which consitutes the start of the given cycle analytics phase. + # end_time_conditions: An array of `conditions`. Each condition is an tuple of `condition_name` and `condition_fn`. `condition_fn` is called with + # `context` (no lexical scope, so need to do `context.create` for factories, for example) and `data` (from the `data_fn`). + # Each `condition_fn` is expected to implement a case which consitutes the end of the given cycle analytics phase. + # before_end_fn: This function is run before calling the end time conditions. Used for setup that needs to be run between the start and end conditions. + + def generate_cycle_analytics_spec(phase:, data_fn:, start_time_conditions:, end_time_conditions:, before_end_fn: nil) + combinations_of_start_time_conditions = (1..start_time_conditions.size).flat_map { |size| start_time_conditions.combination(size).to_a } + combinations_of_end_time_conditions = (1..end_time_conditions.size).flat_map { |size| end_time_conditions.combination(size).to_a } + + scenarios = combinations_of_start_time_conditions.product(combinations_of_end_time_conditions) + scenarios.each do |start_time_conditions, end_time_conditions| + context "start condition: #{start_time_conditions.map(&:first).to_sentence}" do + context "end condition: #{end_time_conditions.map(&:first).to_sentence}" do + it "finds the median of available durations between the two conditions" do + time_differences = Array.new(5) do |index| + data = data_fn[self] + start_time = (index * 10).days.from_now + end_time = start_time + rand(1..5).days + + start_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(start_time) { condition_fn[self, data] } + end + + # Run `before_end_fn` at the midpoint between `start_time` and `end_time` + Timecop.freeze(start_time + (end_time - start_time) / 2) { before_end_fn[self, data] } if before_end_fn + + end_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(end_time) { condition_fn[self, data] } + end + + end_time - start_time + end + + median_time_difference = time_differences.sort[2] + expect(subject.send(phase)).to be_within(5).of(median_time_difference) + end + + context "when the data belongs to another project" do + let(:other_project) { create(:project) } + + it "returns nil" do + # Use a stub to "trick" the data/condition functions + # into using another project. This saves us from having to + # define separate data/condition functions for this particular + # test case. + allow(self).to receive(:project) { other_project } + + 5.times do + data = data_fn[self] + start_time = Time.now + end_time = rand(1..10).days.from_now + + start_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(start_time) { condition_fn[self, data] } + end + + end_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(end_time) { condition_fn[self, data] } + end + end + + # Turn off the stub before checking assertions + allow(self).to receive(:project).and_call_original + + expect(subject.send(phase)).to be_nil + end + end + + context "when the end condition happens before the start condition" do + it 'returns nil' do + data = data_fn[self] + start_time = Time.now + end_time = start_time + rand(1..5).days + + # Run `before_end_fn` at the midpoint between `start_time` and `end_time` + Timecop.freeze(start_time + (end_time - start_time) / 2) { before_end_fn[self, data] } if before_end_fn + + end_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(start_time) { condition_fn[self, data] } + end + + start_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(end_time) { condition_fn[self, data] } + end + + expect(subject.send(phase)).to be_nil + end + end + end + end + + context "start condition NOT PRESENT: #{start_time_conditions.map(&:first).to_sentence}" do + context "end condition: #{end_time_conditions.map(&:first).to_sentence}" do + it "returns nil" do + 5.times do + data = data_fn[self] + end_time = rand(1..10).days.from_now + + end_time_conditions.each_with_index do |(condition_name, condition_fn), index| + Timecop.freeze(end_time + index.days) { condition_fn[self, data] } + end + end + + expect(subject.send(phase)).to be_nil + end + end + end + + context "start condition: #{start_time_conditions.map(&:first).to_sentence}" do + context "end condition NOT PRESENT: #{end_time_conditions.map(&:first).to_sentence}" do + it "returns nil" do + 5.times do + data = data_fn[self] + start_time = Time.now + + start_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(start_time) { condition_fn[self, data] } + end + end + + expect(subject.send(phase)).to be_nil + end + end + end + end + + context "when none of the start / end conditions are matched" do + it "returns nil" do + expect(subject.send(phase)).to be_nil + end + end + end + end +end + +RSpec.configure do |config| + config.extend CycleAnalyticsHelpers::TestGeneration +end diff --git a/spec/support/git_helpers.rb b/spec/support/git_helpers.rb new file mode 100644 index 00000000000..93422390ef7 --- /dev/null +++ b/spec/support/git_helpers.rb @@ -0,0 +1,9 @@ +module GitHelpers + def random_git_name + "#{FFaker::Product.brand}-#{FFaker::Product.brand}-#{rand(1000)}" + end +end + +RSpec.configure do |config| + config.include GitHelpers +end |