diff options
author | Stan Hu <stanhu@gmail.com> | 2016-09-21 08:05:02 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2016-09-21 08:05:02 +0300 |
commit | 5416ab8a0df000bfa9f853840d44d992a975db83 (patch) | |
tree | 0fc99b8b83235de0db18b967aef9ec1feacfe1f3 /spec | |
parent | 0c7f38bd5b59458a94a9637e06287c8bbbaec82d (diff) | |
parent | 244ec0a84c969454bfa05f66dedb22f2b1172323 (diff) |
Merge branch '21170-cycle-analytics' into 'master'
Cycle Analytics: first iteration
## What does this MR do?
- Implement the first iteration of the "Cycle Analytics" feature.
## What are the relevant issue numbers?
- Closes #21170
## Screenshots
![cycle_analytics_screencast.gif](/uploads/d23c3c912caa6935fd47b53ca3a56b97/cycle_analytics.gif)
## Backend Tasks
- [x] Implementation
- [x] Phases
- [x] Issue (Tracker)
- [x] Plan (Board)
- [x] Code (IDE)
- [x] Test (CI)
- [x] Review (MR)
- [x] Staging (CD)
- [x] Production (Total)
- [x] Make heuristics more modular
- [x] Scope to project
- [x] Date range (30 days, 90 days)
- [x] Access restriction
- [x] Test
- [x] Find a better way to test these phases
- [x] Phases
- [x] Issue (Tracker)
- [x] Plan (Board)
- [x] Code (IDE)
- [x] Test (CI)
- [x] Review (MR)
- [x] Staging (CD)
- [x] Production (Total)
- [x] Test for "end case happens before start case"
- [x] Consolidate helper
- [x] Miniboss review
- [x] Performance testing with mock data
- [x] Improve performance
- [x] Pre-calculate "merge requests closing issues
- [x] Pre-calculate everything else
- [x] Test performance against 10k issues
- [x] Test all pre-calculation code
- [x] Ci::Pipeline -> build start/finish
- [x] Ci::Pipeline#merge_requests
- [x] Issue -> record default metrics after save
- [x] MergeRequest -> record default metrics after save
- [x] Deployment -> Update "first_deployed_to_production_at" for MR metrics
- [x] Git Push -> Update "first commit mention" for issue metrics
- [x] Merge request create/update/refresh -> Update "merge requests closing issues"
- [x] Remove `MergeRequestsClosingIssues` when necessary
- [x] Changes to unblock Fatih
- [x] Add summary data
- [x] `stats` should be array
- [x] Let `stats` be `null` if all `stats` are null
- [x] Indexes for "merge requests closing issues"
- [x] Test summary data
- [x] Scope everything to project
- [x] Find out why tests were passing
- [x] Filter should include issues/MRs which have made it to production within the range
- [x] Don't create duplicate `MergeRequestsClosingIssues`
- [x] Fix tests
- [x] MySQL median
- [x] Assign to Douwe for review
- [x] Fix conflicts
- [x] Implement suggestions from Yorick's review
- [x] Test on PG
- [x] Test on MySQL
- [x] Refactor
- [x] Cleanup
- [x] What happens if we have no data at all?
- [x] Extract common queries to methods / scopes
- [x] Remove unused queries
- [x] Downtime for foreign key migrations
- [x] Find a way around "if issue.metrics.present?" all over the place
- [x] Find a way around "if merge_request.metrics.present?" all over the place
- [x] Test migrations on a fresh database
- [x] MySQL
- [x] Pg
- [x] Access issues
- While the project is public and the visibility is set to "Everyone with access", you cannot visit the cycle analytics page when signed out.
- [x] CHANGELOG
- [x] Implement suggestions from Douwe's review
- [x] First set of comments
- [x] Second set of comments
- [x] Third set of comments
- [x] Fourth set of comments
- [x] Make sure build is green
- [ ] Make issue for "polish"
- [ ] EE MR
See merge request !5986
Diffstat (limited to 'spec')
21 files changed, 1044 insertions, 1 deletions
diff --git a/spec/factories/deployments.rb b/spec/factories/deployments.rb index 82591604fcb..6f24bf58d14 100644 --- a/spec/factories/deployments.rb +++ b/spec/factories/deployments.rb @@ -3,11 +3,12 @@ FactoryGirl.define do sha '97de212e80737a608d939f648d959671fb0a0142' ref 'master' tag false + project nil environment factory: :environment after(:build) do |deployment, evaluator| - deployment.project = deployment.environment.project + deployment.project ||= deployment.environment.project end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index f1857f846dc..550a890797e 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -187,6 +187,37 @@ describe Ci::Pipeline, models: true do end end + describe "merge request metrics" do + let(:project) { FactoryGirl.create :project } + let(:pipeline) { FactoryGirl.create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) } + let!(:merge_request) { create(:merge_request, source_project: project, source_branch: pipeline.ref) } + + context 'when transitioning to running' do + it 'records the build start time' do + time = Time.now + Timecop.freeze(time) { build.run } + + expect(merge_request.reload.metrics.latest_build_started_at).to be_within(1.second).of(time) + end + + it 'clears the build end time' do + build.run + + expect(merge_request.reload.metrics.latest_build_finished_at).to be_nil + end + end + + context 'when transitioning to success' do + it 'records the build end time' do + build.run + time = Time.now + Timecop.freeze(time) { build.success } + + expect(merge_request.reload.metrics.latest_build_finished_at).to be_within(1.second).of(time) + end + end + end + def create_build(name, queued_at = current, started_from = 0) create(:ci_build, name: name, @@ -468,4 +499,28 @@ describe Ci::Pipeline, models: true do stage_idx: stage_idx) end end + + describe "#merge_requests" do + let(:project) { FactoryGirl.create :project } + let(:pipeline) { FactoryGirl.create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) } + + it "returns merge requests whose `diff_head_sha` matches the pipeline's SHA" do + merge_request = create(:merge_request, source_project: project, source_branch: pipeline.ref) + + expect(pipeline.merge_requests).to eq([merge_request]) + end + + it "doesn't return merge requests whose source branch doesn't match the pipeline's ref" do + create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master') + + expect(pipeline.merge_requests).to be_empty + end + + it "doesn't return merge requests whose `diff_head_sha` doesn't match the pipeline's SHA" do + create(:merge_request, source_project: project, source_branch: pipeline.ref) + allow_any_instance_of(MergeRequest).to receive(:diff_head_sha) { '97de212e80737a608d939f648d959671fb0a0142b' } + + expect(pipeline.merge_requests).to be_empty + end + end end diff --git a/spec/models/cycle_analytics/code_spec.rb b/spec/models/cycle_analytics/code_spec.rb new file mode 100644 index 00000000000..b9381e33914 --- /dev/null +++ b/spec/models/cycle_analytics/code_spec.rb @@ -0,0 +1,42 @@ +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) 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 + 5.times do + issue = create(:issue, project: project) + + create_commit_referencing_issue(issue) + create_merge_request_closing_issue(issue, message: "Closes nothing") + + merge_merge_requests_closing_issue(issue) + deploy_master + 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..e9cc71254ab --- /dev/null +++ b/spec/models/cycle_analytics/issue_spec.rb @@ -0,0 +1,50 @@ +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) 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 + 5.times do + regular_label = create(:label) + issue = create(:issue, project: project) + issue.update(label_ids: [regular_label.id]) + + create_merge_request_closing_issue(issue) + merge_merge_requests_closing_issue(issue) + deploy_master + 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..5b8c96dc992 --- /dev/null +++ b/spec/models/cycle_analytics/plan_spec.rb @@ -0,0 +1,52 @@ +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) 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 + branch_name = random_git_name + label = create(:label) + issue = create(:issue, project: project) + issue.update(label_ids: [label.id]) + create_commit_referencing_issue(issue, branch_name: branch_name) + + create_merge_request_closing_issue(issue, source_branch: branch_name) + merge_merge_requests_closing_issue(issue) + deploy_master + + 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..1f5e5cab92d --- /dev/null +++ b/spec/models/cycle_analytics/production_spec.rb @@ -0,0 +1,54 @@ +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]]) + + 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..b6e26d8f261 --- /dev/null +++ b/spec/models/cycle_analytics/review_spec.rb @@ -0,0 +1,35 @@ +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) 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 + 5.times do + MergeRequests::MergeService.new(project, user).execute(create(:merge_request)) + + deploy_master + 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..af1c4477ddb --- /dev/null +++ b/spec/models/cycle_analytics/staging_spec.rb @@ -0,0 +1,64 @@ +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) 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 + 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/summary_spec.rb b/spec/models/cycle_analytics/summary_spec.rb new file mode 100644 index 00000000000..743bc2da33f --- /dev/null +++ b/spec/models/cycle_analytics/summary_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +describe CycleAnalytics::Summary, models: true do + let(:project) { create(:project) } + let(:from) { Time.now } + let(:user) { create(:user, :admin) } + subject { described_class.new(project, from: from) } + + describe "#new_issues" do + it "finds the number of issues created after the 'from date'" do + Timecop.freeze(5.days.ago) { create(:issue, project: project) } + Timecop.freeze(5.days.from_now) { create(:issue, project: project) } + + expect(subject.new_issues).to eq(1) + end + + it "doesn't find issues from other projects" do + Timecop.freeze(5.days.from_now) { create(:issue, project: create(:project)) } + + expect(subject.new_issues).to eq(0) + end + end + + describe "#commits" do + it "finds the number of commits created after the 'from date'" do + Timecop.freeze(5.days.ago) { create_commit("Test message", project, user, 'master') } + Timecop.freeze(5.days.from_now) { create_commit("Test message", project, user, 'master') } + + expect(subject.commits).to eq(1) + end + + it "doesn't find commits from other projects" do + Timecop.freeze(5.days.from_now) { create_commit("Test message", create(:project), user, 'master') } + + expect(subject.commits).to eq(0) + end + end + + describe "#deploys" do + it "finds the number of deploys made created after the 'from date'" do + Timecop.freeze(5.days.ago) { create(:deployment, project: project) } + Timecop.freeze(5.days.from_now) { create(:deployment, project: project) } + + expect(subject.deploys).to eq(1) + end + + it "doesn't find commits from other projects" do + Timecop.freeze(5.days.from_now) { create(:deployment, project: create(:project)) } + + expect(subject.deploys).to eq(0) + 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..89ace0b2742 --- /dev/null +++ b/spec/models/cycle_analytics/test_spec.rb @@ -0,0 +1,94 @@ +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) + { 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 + 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.succeed! + + merge_merge_requests_closing_issue(issue) + deploy_master + 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! + + deploy_master + 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! + + merge_merge_requests_closing_issue(issue) + deploy_master + 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! + + merge_merge_requests_closing_issue(issue) + deploy_master + end + + expect(subject.test).to be_nil + end + end +end diff --git a/spec/models/issue/metrics_spec.rb b/spec/models/issue/metrics_spec.rb new file mode 100644 index 00000000000..e170b087ebc --- /dev/null +++ b/spec/models/issue/metrics_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe Issue::Metrics, models: true do + let(:project) { create(:project) } + + subject { create(:issue, project: project) } + + describe "when recording the default set of issue metrics on issue save" do + context "milestones" do + it "records the first time an issue is associated with a milestone" do + time = Time.now + Timecop.freeze(time) { subject.update(milestone: create(:milestone)) } + metrics = subject.metrics + + expect(metrics).to be_present + expect(metrics.first_associated_with_milestone_at).to be_within(1.second).of(time) + end + + it "does not record the second time an issue is associated with a milestone" do + time = Time.now + Timecop.freeze(time) { subject.update(milestone: create(:milestone)) } + Timecop.freeze(time + 2.hours) { subject.update(milestone: nil) } + Timecop.freeze(time + 6.hours) { subject.update(milestone: create(:milestone)) } + metrics = subject.metrics + + expect(metrics).to be_present + expect(metrics.first_associated_with_milestone_at).to be_within(1.second).of(time) + end + end + + context "list labels" do + it "records the first time an issue is associated with a list label" do + list_label = create(:label, lists: [create(:list)]) + time = Time.now + Timecop.freeze(time) { subject.update(label_ids: [list_label.id]) } + metrics = subject.metrics + + expect(metrics).to be_present + expect(metrics.first_added_to_board_at).to be_within(1.second).of(time) + end + + it "does not record the second time an issue is associated with a list label" do + time = Time.now + first_list_label = create(:label, lists: [create(:list)]) + Timecop.freeze(time) { subject.update(label_ids: [first_list_label.id]) } + second_list_label = create(:label, lists: [create(:list)]) + Timecop.freeze(time + 5.hours) { subject.update(label_ids: [second_list_label.id]) } + metrics = subject.metrics + + expect(metrics).to be_present + expect(metrics.first_added_to_board_at).to be_within(1.second).of(time) + end + end + end +end diff --git a/spec/models/merge_request/metrics_spec.rb b/spec/models/merge_request/metrics_spec.rb new file mode 100644 index 00000000000..a79dd215d41 --- /dev/null +++ b/spec/models/merge_request/metrics_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe MergeRequest::Metrics, models: true do + let(:project) { create(:project) } + + subject { create(:merge_request, source_project: project) } + + describe "when recording the default set of metrics on merge request save" do + it "records the merge time" do + time = Time.now + Timecop.freeze(time) { subject.mark_as_merged } + metrics = subject.metrics + + expect(metrics).to be_present + expect(metrics.merged_at).to be_within(1.second).of(time) + end + end +end diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index 41b897f36cd..343b4385bf2 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -169,4 +169,83 @@ describe CreateDeploymentService, services: true do end end end + + describe "merge request metrics" do + let(:params) do + { + environment: 'production', + ref: 'master', + tag: false, + sha: '97de212e80737a608d939f648d959671fb0a0142b', + } + end + + let(:merge_request) { create(:merge_request, target_branch: 'master', source_branch: 'feature', source_project: project) } + + context "while updating the 'first_deployed_to_production_at' time" do + before { merge_request.mark_as_merged } + + context "for merge requests merged before the current deploy" do + it "sets the time if the deploy's environment is 'production'" do + time = Time.now + Timecop.freeze(time) { service.execute } + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_within(1.second).of(time) + end + + it "doesn't set the time if the deploy's environment is not 'production'" do + staging_params = params.merge(environment: 'staging') + service = described_class.new(project, user, staging_params) + service.execute + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil + end + + it 'does not raise errors if the merge request does not have a metrics record' do + merge_request.metrics.destroy + + expect(merge_request.reload.metrics).to be_nil + expect { service.execute }.not_to raise_error + end + end + + context "for merge requests merged before the previous deploy" do + context "if the 'first_deployed_to_production_at' time is already set" do + it "does not overwrite the older 'first_deployed_to_production_at' time" do + # Previous deploy + time = Time.now + Timecop.freeze(time) { service.execute } + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_within(1.second).of(time) + + # Current deploy + service = described_class.new(project, user, params) + Timecop.freeze(time + 12.hours) { service.execute } + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_within(1.second).of(time) + end + end + + context "if the 'first_deployed_to_production_at' time is not already set" do + it "does not overwrite the older 'first_deployed_to_production_at' time" do + # Previous deploy + time = 5.minutes.from_now + Timecop.freeze(time) { service.execute } + + expect(merge_request.reload.metrics.merged_at).to be < merge_request.reload.metrics.first_deployed_to_production_at + + merge_request.reload.metrics.update(first_deployed_to_production_at: nil) + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil + + # Current deploy + service = described_class.new(project, user, params) + Timecop.freeze(time + 12.hours) { service.execute } + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil + end + end + end + end + end end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 22724434a7f..22991c5bc86 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -339,6 +339,43 @@ describe GitPushService, services: true do end end + describe "issue metrics" do + let(:issue) { create :issue, project: project } + let(:commit_author) { create :user } + let(:commit) { project.commit } + let(:commit_time) { Time.now } + + before do + project.team << [commit_author, :developer] + project.team << [user, :developer] + + allow(commit).to receive_messages( + safe_message: "this commit \n mentions #{issue.to_reference}", + references: [issue], + author_name: commit_author.name, + author_email: commit_author.email, + committed_date: commit_time + ) + + allow(project.repository).to receive(:commits_between).and_return([commit]) + end + + context "while saving the 'first_mentioned_in_commit_at' metric for an issue" do + it 'sets the metric for referenced issues' do + execute_service(project, user, @oldrev, @newrev, @ref) + + expect(issue.reload.metrics.first_mentioned_in_commit_at).to be_within(1.second).of(commit_time) + end + + it 'does not set the metric for non-referenced issues' do + non_referenced_issue = create(:issue, project: project) + execute_service(project, user, @oldrev, @newrev, @ref) + + expect(non_referenced_issue.reload.metrics.first_mentioned_in_commit_at).to be_nil + end + end + end + describe "closing issues from pushed commits containing a closing reference" do let(:issue) { create :issue, project: project } let(:other_issue) { create :issue, project: project } diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index c1e4f8bd96b..b8142889075 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -83,5 +83,34 @@ describe MergeRequests::CreateService, services: true do } end end + + context 'while saving references to issues that the created merge request closes' do + let(:first_issue) { create(:issue, project: project) } + let(:second_issue) { create(:issue, project: project) } + + let(:opts) do + { + title: 'Awesome merge_request', + source_branch: 'feature', + target_branch: 'master', + force_remove_source_branch: '1' + } + end + + before do + project.team << [user, :master] + project.team << [assignee, :developer] + end + + it 'creates a `MergeRequestsClosingIssues` record for each issue' do + issue_closing_opts = opts.merge(description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}") + service = described_class.new(project, user, issue_closing_opts) + allow(service).to receive(:execute_hooks) + merge_request = service.execute + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to match_array([first_issue.id, second_issue.id]) + end + end end end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index fff86480c6d..a162df5fc34 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -174,6 +174,58 @@ describe MergeRequests::RefreshService, services: true do end end + context 'merge request metrics' do + let(:issue) { create :issue, project: @project } + let(:commit_author) { create :user } + let(:commit) { project.commit } + + before do + project.team << [commit_author, :developer] + project.team << [user, :developer] + + allow(commit).to receive_messages( + safe_message: "Closes #{issue.to_reference}", + references: [issue], + author_name: commit_author.name, + author_email: commit_author.email, + committed_date: Time.now + ) + + allow_any_instance_of(MergeRequest).to receive(:commits).and_return([commit]) + end + + context 'when the merge request is sourced from the same project' do + it 'creates a `MergeRequestsClosingIssues` record for each issue closed by a commit' do + merge_request = create(:merge_request, target_branch: 'master', source_branch: 'feature', source_project: @project) + refresh_service = service.new(@project, @user) + allow(refresh_service).to receive(:execute_hooks) + refresh_service.execute(@oldrev, @newrev, 'refs/heads/feature') + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to eq([issue.id]) + end + end + + context 'when the merge request is sourced from a different project' do + it 'creates a `MergeRequestsClosingIssues` record for each issue closed by a commit' do + forked_project = create(:project) + create(:forked_project_link, forked_to_project: forked_project, forked_from_project: @project) + + merge_request = create(:merge_request, + target_branch: 'master', + source_branch: 'feature', + target_project: @project, + source_project: forked_project) + refresh_service = service.new(@project, @user) + allow(refresh_service).to receive(:execute_hooks) + refresh_service.execute(@oldrev, @newrev, 'refs/heads/feature') + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to eq([issue.id]) + end + end + end + def reload_mrs @merge_request.reload @fork_merge_request.reload diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 6dfeb581975..33db34c0f62 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -263,5 +263,42 @@ describe MergeRequests::UpdateService, services: true do end end end + + context 'while saving references to issues that the updated merge request closes' do + let(:first_issue) { create(:issue, project: project) } + let(:second_issue) { create(:issue, project: project) } + + it 'creates a `MergeRequestsClosingIssues` record for each issue' do + issue_closing_opts = { description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}" } + service = described_class.new(project, user, issue_closing_opts) + allow(service).to receive(:execute_hooks) + service.execute(merge_request) + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to match_array([first_issue.id, second_issue.id]) + end + + it 'removes `MergeRequestsClosingIssues` records when issues are not closed anymore' do + opts = { + title: 'Awesome merge_request', + description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}", + source_branch: 'feature', + target_branch: 'master', + force_remove_source_branch: '1' + } + + merge_request = MergeRequests::CreateService.new(project, user, opts).execute + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to match_array([first_issue.id, second_issue.id]) + + service = described_class.new(project, user, description: "not closing any issues") + allow(service).to receive(:execute_hooks) + service.execute(merge_request.reload) + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to be_empty + end + end end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index f81a58899fd..0d152534c38 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -379,6 +379,7 @@ describe NotificationService, services: true do it "emails subscribers of the issue's labels" do subscriber = create(:user) label = create(:label, issues: [issue]) + issue.reload label.toggle_subscription(subscriber) notification.new_issue(issue, @u_disabled) @@ -399,6 +400,7 @@ describe NotificationService, services: true do project.team << [guest, :guest] label = create(:label, issues: [confidential_issue]) + confidential_issue.reload label.toggle_subscription(non_member) label.toggle_subscription(author) label.toggle_subscription(assignee) diff --git a/spec/support/cycle_analytics_helpers.rb b/spec/support/cycle_analytics_helpers.rb new file mode 100644 index 00000000000..e8e760a6187 --- /dev/null +++ b/spec/support/cycle_analytics_helpers.rb @@ -0,0 +1,64 @@ +module CycleAnalyticsHelpers + def create_commit_referencing_issue(issue, branch_name: random_git_name) + project.repository.add_branch(user, branch_name, 'master') + create_commit("Commit for ##{issue.iid}", issue.project, user, branch_name) + end + + def create_commit(message, project, user, branch_name) + filename = random_git_name + oldrev = project.repository.commit(branch_name).sha + + options = { + committer: project.repository.user_to_committer(user), + author: project.repository.user_to_committer(user), + commit: { message: message, branch: branch_name, update_ref: true }, + file: { content: "content", path: filename, update: false } + } + + commit_sha = Gitlab::Git::Blob.commit(project.repository, options) + project.repository.commit(commit_sha) + + GitPushService.new(project, + user, + oldrev: oldrev, + newrev: commit_sha, + ref: 'refs/heads/master').execute + end + + def create_merge_request_closing_issue(issue, message: nil, source_branch: nil) + if !source_branch || project.repository.commit(source_branch).blank? + source_branch = random_git_name + project.repository.add_branch(user, source_branch, 'master') + end + + 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..8e19a6c92e2 --- /dev/null +++ b/spec/support/cycle_analytics_helpers/test_generation.rb @@ -0,0 +1,161 @@ +# 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. + # post_fn: Code that needs to be run after running the end time conditions. + + def generate_cycle_analytics_spec(phase:, data_fn:, start_time_conditions:, end_time_conditions:, before_end_fn: nil, post_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 + + Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn + + 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 + + Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn + 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 + + Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn + + 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 + + Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn + 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 + + post_fn[self, data] if post_fn + 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 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 |