From 0a767f7b618161278b66f7e1d842a51c8cab37b3 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Wed, 7 Sep 2016 14:29:12 +0530 Subject: [PATCH] Consolidate all cycle analytics helper methods. 1. Move the test generation to `CycleAnalyticsHelpers::TestGeneration` 2. Move all helper methods (previously placed in each individual spec file) to `CycleAnalyticsHelpers` --- spec/models/cycle_analytics/code_spec.rb | 20 -- spec/models/cycle_analytics/plan_spec.rb | 6 - .../models/cycle_analytics/production_spec.rb | 30 --- spec/models/cycle_analytics/review_spec.rb | 21 --- spec/models/cycle_analytics/staging_spec.rb | 30 --- spec/models/cycle_analytics/test_spec.rb | 16 -- spec/support/cycle_analytics_helpers.rb | 171 +++--------------- .../test_generation.rb | 154 ++++++++++++++++ 8 files changed, 184 insertions(+), 264 deletions(-) create mode 100644 spec/support/cycle_analytics_helpers/test_generation.rb diff --git a/spec/models/cycle_analytics/code_spec.rb b/spec/models/cycle_analytics/code_spec.rb index a81b5cf444d..18dd4d0f1ab 100644 --- a/spec/models/cycle_analytics/code_spec.rb +++ b/spec/models/cycle_analytics/code_spec.rb @@ -6,26 +6,6 @@ describe 'CycleAnalytics#code', feature: true do let(:user) { create(:user, :admin) } subject { CycleAnalytics.new(project, from: from_date) } - 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') - - 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 - 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]) }]], diff --git a/spec/models/cycle_analytics/plan_spec.rb b/spec/models/cycle_analytics/plan_spec.rb index 42984169b28..fa092bf6825 100644 --- a/spec/models/cycle_analytics/plan_spec.rb +++ b/spec/models/cycle_analytics/plan_spec.rb @@ -6,12 +6,6 @@ describe 'CycleAnalytics#plan', feature: true do let(:user) { create(:user, :admin) } subject { CycleAnalytics.new(project, from: from_date) } - 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! - end - 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)) }], diff --git a/spec/models/cycle_analytics/production_spec.rb b/spec/models/cycle_analytics/production_spec.rb index 76e42c1c2d4..703f8d5f782 100644 --- a/spec/models/cycle_analytics/production_spec.rb +++ b/spec/models/cycle_analytics/production_spec.rb @@ -6,36 +6,6 @@ describe 'CycleAnalytics#production', feature: true do let(:user) { create(:user, :admin) } subject { CycleAnalytics.new(project, from: from_date) } - 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 - 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 }]], diff --git a/spec/models/cycle_analytics/review_spec.rb b/spec/models/cycle_analytics/review_spec.rb index 8530d70a38e..867a90d6258 100644 --- a/spec/models/cycle_analytics/review_spec.rb +++ b/spec/models/cycle_analytics/review_spec.rb @@ -6,27 +6,6 @@ describe 'CycleAnalytics#review', feature: true do let(:user) { create(:user, :admin) } subject { CycleAnalytics.new(project, from: from_date) } - def create_merge_request_closing_issue(issue) - 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: "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 - 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]) }]], diff --git a/spec/models/cycle_analytics/staging_spec.rb b/spec/models/cycle_analytics/staging_spec.rb index 23e01532500..105f566f41c 100644 --- a/spec/models/cycle_analytics/staging_spec.rb +++ b/spec/models/cycle_analytics/staging_spec.rb @@ -6,36 +6,6 @@ describe 'CycleAnalytics#staging', feature: true do let(:user) { create(:user, :admin) } subject { CycleAnalytics.new(project, from: from_date) } - 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 - generate_cycle_analytics_spec(phase: :staging, data_fn: lambda do |context| issue = context.create(:issue, project: context.project) diff --git a/spec/models/cycle_analytics/test_spec.rb b/spec/models/cycle_analytics/test_spec.rb index af4d55f5801..aa7faa74d38 100644 --- a/spec/models/cycle_analytics/test_spec.rb +++ b/spec/models/cycle_analytics/test_spec.rb @@ -6,22 +6,6 @@ describe 'CycleAnalytics#test', feature: true do let(:user) { create(:user, :admin) } subject { CycleAnalytics.new(project, from: from_date) } - 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 - generate_cycle_analytics_spec(phase: :test, data_fn: lambda do |context| issue = context.create(:issue, project: context.project) diff --git a/spec/support/cycle_analytics_helpers.rb b/spec/support/cycle_analytics_helpers.rb index 60524624782..c5fe1170423 100644 --- a/spec/support/cycle_analytics_helpers.rb +++ b/spec/support/cycle_analytics_helpers.rb @@ -1,152 +1,41 @@ -# 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 - # 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 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 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 } + 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) - 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 + opts = { + title: 'Awesome merge_request', + description: message || "Fixes #{issue.to_reference}", + source_branch: source_branch, + target_branch: 'master' + } - start_time_conditions.each do |condition_name, condition_fn| - Timecop.freeze(start_time) { condition_fn[self, data] } - end + MergeRequests::CreateService.new(project, user, opts).execute + 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 + 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 - 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 + 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.extend CycleAnalyticsHelpers + 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..4e386691d78 --- /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