From 98c9d1207708cd7d9fc37167e65e8a26b668aaa7 Mon Sep 17 00:00:00 2001 From: Timothy Andrew <mail@timothyandrew.net> Date: Fri, 2 Sep 2016 17:33:32 +0530 Subject: [PATCH] Refactor cycle analytics specs. 1. Generalise the specs that will be common across all cycle analytics phases. 2. Rewrite specs `issue` and `plan` to use this abstracted testing strategy. 3. Specs that are specific to a given phase, or unwieldy to test in an abstracted manner, are added to each phase's spec. --- spec/models/cycle_analytics/issue_spec.rb | 85 +++----------- spec/models/cycle_analytics/plan_spec.rb | 132 ++++------------------ spec/support/cycle_analytics_helpers.rb | 96 ++++++++++++++++ 3 files changed, 133 insertions(+), 180 deletions(-) create mode 100644 spec/support/cycle_analytics_helpers.rb diff --git a/spec/models/cycle_analytics/issue_spec.rb b/spec/models/cycle_analytics/issue_spec.rb index a176a15b2b7..ef4de184d64 100644 --- a/spec/models/cycle_analytics/issue_spec.rb +++ b/spec/models/cycle_analytics/issue_spec.rb @@ -5,44 +5,14 @@ describe 'CycleAnalytics#issue', models: true do let(:from_date) { 10.days.ago } subject { CycleAnalytics.new(project, from: from_date) } - context "when a milestone is added to the issue" do - it "calculates the median of available durations (between issue creation and milestone addition)" do - time_differences = Array.new(5) do - start_time = Time.now - end_time = rand(1..10).days.from_now - - milestone = create(:milestone, project: project) - issue = Timecop.freeze(start_time) { create(:issue, project: project) } - Timecop.freeze(end_time) { issue.update(milestone: milestone) } - - end_time - start_time - end - - median_time_difference = time_differences.sort[2] - expect(subject.issue).to eq(median_time_difference) - end - end - - context "when a label is added to the issue" do - context "when the label is a list-label" do - it "calculates the median of available durations (between issue creation and label addition)" do - time_differences = Array.new(5) do - start_time = Time.now - end_time = rand(1..10).days.from_now - - list_label = create(:label, lists: [create(:list)]) - issue = Timecop.freeze(start_time) { create(:issue, project: project) } - Timecop.freeze(end_time) { issue.update(label_ids: [list_label.id]) } - - end_time - start_time - end - - median_time_difference = time_differences.sort[2] - expect(subject.issue).to eq(median_time_difference) - end - end - - it "does not make a calculation for regular labels" do + 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) @@ -53,39 +23,14 @@ describe 'CycleAnalytics#issue', models: true do end end - context "when a milestone and list-label are both added to the issue" do - it "calculates the median of available durations (between issue creation and milestone addition)" do - start_time = Time.now - milestone_add_time = rand(1..10).days.from_now - list_label_add_time = rand(1..10).days.from_now - - milestone = create(:milestone, project: project) - list_label = create(:label, lists: [create(:list)]) - issue = Timecop.freeze(start_time) { create(:issue, project: project) } - Timecop.freeze(milestone_add_time) { issue.update(milestone: milestone) } - Timecop.freeze(list_label_add_time) { issue.update(label_ids: [list_label.id]) } - - expect(subject.issue).to eq(milestone_add_time - start_time) - end - - it "does not include issues from other projects" do - milestone = create(:milestone, project: project) - list_label = create(:label, lists: [create(:list)]) - issue = create(:issue) - issue.update(milestone: milestone) - issue.update(label_ids: [list_label.id]) + context "when the issue belongs to a different project" do + it 'returns nil' do + other_project = create(:project) - expect(subject.issue).to be_nil - end - - it "excludes issues created before the 'from' date" do - before_from_date = from_date - 5.days - - milestone = create(:milestone, project: project) - list_label = create(:label, lists: [create(:list)]) - issue = Timecop.freeze(before_from_date) { create(:issue, project: project)} - issue.update(milestone: milestone) - issue.update(label_ids: [list_label.id]) + 5.times do + issue = create(:issue, project: other_project) + issue.update(milestone: create(:milestone, project: other_project)) + end expect(subject.issue).to be_nil end diff --git a/spec/models/cycle_analytics/plan_spec.rb b/spec/models/cycle_analytics/plan_spec.rb index e3eeb0e505c..a5d4128d938 100644 --- a/spec/models/cycle_analytics/plan_spec.rb +++ b/spec/models/cycle_analytics/plan_spec.rb @@ -6,126 +6,38 @@ describe 'CycleAnalytics#plan', feature: true do let(:user) { create(:user, :admin) } subject { CycleAnalytics.new(project, from: from_date) } - def create_commit_referencing_issue(issue, time: Time.now) - sha = Timecop.freeze(time) { project.repository.commit_file(user, FFaker::Product.brand, "content", "Commit for ##{issue.iid}", "master", false) } + def create_commit_referencing_issue(issue) + sha = project.repository.commit_file(user, FFaker::Product.brand, "content", "Commit for ##{issue.iid}", "master", false) commit = project.repository.commit(sha) commit.create_cross_references! end - context "when a milestone is added to the issue" do - context "when the issue is mentioned in a commit" do - it "calculates the median of available durations between the two" do - time_differences = Array.new(5) do - start_time = Time.now - end_time = rand(1..10).days.from_now + 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]) }]]) - milestone = create(:milestone, project: project) - issue = create(:issue, project: project) + 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) - Timecop.freeze(start_time) { issue.update(milestone: milestone) } - create_commit_referencing_issue(issue, time: end_time) - - end_time - start_time - end - - median_time_difference = time_differences.sort[2] - - # Use `be_within` to account for time lost between Rails invoking CLI git - # and the commit being created, which Timecop can't freeze. - expect(subject.plan).to be_within(2).of(median_time_difference) - end - end - end - - context "when a label is added to the issue" do - context "when the issue is mentioned in a commit" do - context "when the label is a list-label" do - it "calculates the median of available durations between the two" do - time_differences = Array.new(5) do - start_time = Time.now - end_time = rand(1..10).days.from_now - - issue = create(:issue, project: project) - list_label = create(:label, lists: [create(:list)]) - - Timecop.freeze(start_time) { issue.update(label_ids: [list_label.id]) } - create_commit_referencing_issue(issue, time: end_time) - - end_time - start_time - end - - median_time_difference = time_differences.sort[2] - - # Use `be_within` to account for time lost between Rails invoking CLI git - # and the commit being created, which Timecop can't freeze. - expect(subject.plan).to be_within(2).of(median_time_difference) - end - end - - it "does not make a calculation for regular labels" do - 5.times do - regular_label = create(:label) - issue = create(:issue, project: project) - issue.update(label_ids: [regular_label.id]) - - create_commit_referencing_issue(issue) - end - - expect(subject.plan).to be_nil - end + expect(subject.issue).to be_nil end end - context "when a milestone and list-label are both added to the issue" do - context "when the issue is mentioned in a commit" do - it "calculates the median of available durations between the two (using milestone addition as the 'start_time')" do - time_differences = Array.new(5) do - label_addition_time = Time.now - milestone_addition_time = rand(2..12).hours.from_now - end_time = rand(1..10).days.from_now - - issue = create(:issue, project: project) - milestone = create(:milestone, project: project) - list_label = create(:label, lists: [create(:list)]) - - Timecop.freeze(label_addition_time) { issue.update(label_ids: [list_label.id]) } - Timecop.freeze(milestone_addition_time) { issue.update(milestone: milestone) } - create_commit_referencing_issue(issue, time: end_time) - - end_time - milestone_addition_time - end - - median_time_difference = time_differences.sort[2] + it "does not include issues from other projects" do + other_project = create(:project) - # Use `be_within` to account for time lost between Rails invoking CLI git - # and the commit being created, which Timecop can't freeze. - expect(subject.plan).to be_within(2).of(median_time_difference) - end + list_label = create(:label, lists: [create(:list)]) + issue = create(:issue, project: other_project) + issue.update(milestone: create(:milestone)) + issue.update(label_ids: [list_label.id]) + create_commit_referencing_issue(issue) - it "does not include issues from other projects" do - other_project = create(:project) - - list_label = create(:label, lists: [create(:list)]) - issue = create(:issue, project: other_project) - issue.update(milestone: create(:milestone)) - issue.update(label_ids: [list_label.id]) - create_commit_referencing_issue(issue) - - expect(subject.issue).to be_nil - end - - it "excludes issues created before the 'from' date" do - before_from_date = from_date - 5.days - - milestone = create(:milestone, project: project) - list_label = create(:label, lists: [create(:list)]) - issue = Timecop.freeze(before_from_date) { create(:issue, project: project)} - issue.update(milestone: milestone) - issue.update(label_ids: [list_label.id]) - create_commit_referencing_issue(issue) - - expect(subject.issue).to be_nil - end - end + expect(subject.issue).to be_nil end end diff --git a/spec/support/cycle_analytics_helpers.rb b/spec/support/cycle_analytics_helpers.rb new file mode 100644 index 00000000000..ba18b390c1a --- /dev/null +++ b/spec/support/cycle_analytics_helpers.rb @@ -0,0 +1,96 @@ +# 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. + + def generate_cycle_analytics_spec(phase:, data_fn:, start_time_conditions:, end_time_conditions:) + 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 + 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_time - start_time + end + + median_time_difference = time_differences.sort[2] + expect(subject.send(phase)).to be_within(2).of(median_time_difference) + 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 + +RSpec.configure do |config| + config.extend CycleAnalyticsHelpers +end -- GitLab