diff options
author | Timothy Andrew <mail@timothyandrew.net> | 2016-09-20 22:17:37 +0300 |
---|---|---|
committer | Timothy Andrew <mail@timothyandrew.net> | 2016-09-20 22:17:37 +0300 |
commit | 918e589c2b29c18d9fe3a8e6c93a3f490c86beb1 (patch) | |
tree | 4b354c178952b4369beb16af43c6b75f9d882c49 /spec/models | |
parent | a4a0ce95001b93c2ed65a18946542f3eecdf564e (diff) |
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.
Diffstat (limited to 'spec/models')
-rw-r--r-- | spec/models/cycle_analytics/code_spec.rb | 25 | ||||
-rw-r--r-- | spec/models/cycle_analytics/issue_spec.rb | 37 | ||||
-rw-r--r-- | spec/models/cycle_analytics/plan_spec.rb | 42 | ||||
-rw-r--r-- | spec/models/cycle_analytics/production_spec.rb | 37 | ||||
-rw-r--r-- | spec/models/cycle_analytics/review_spec.rb | 19 | ||||
-rw-r--r-- | spec/models/cycle_analytics/staging_spec.rb | 45 | ||||
-rw-r--r-- | spec/models/cycle_analytics/test_spec.rb | 31 |
7 files changed, 149 insertions, 87 deletions
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 |