From 53d3294d1de6a3ded84532c1874ca48910fd69b0 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 18 Mar 2014 21:32:40 -0400 Subject: Speed up features/notes_on_merge_requests_spec This spec featured the slowest tests in the entire suite. After some debugging, the cause was found to be the large commit diff generated by comparing the stable and master branches. To fix this, the seed repository was modified to create a simple branch off of master that consists of three simple commits and minor changes. The spec was then updated to compare master to this branch instead of stable. The result is a spec group that runs in under 30 seconds, down from about 90. --- spec/factories.rb | 6 +++- spec/features/notes_on_merge_requests_spec.rb | 49 +++++++++++--------------- spec/seed_project.tar.gz | Bin 9789938 -> 9769010 bytes 3 files changed, 26 insertions(+), 29 deletions(-) (limited to 'spec') diff --git a/spec/factories.rb b/spec/factories.rb index 7fc2b7c5e97..373c2a5acff 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -146,6 +146,11 @@ FactoryGirl.define do state :reopened end + trait :simple do + source_branch "simple_merge_request" + target_branch "master" + end + factory :closed_merge_request, traits: [:closed] factory :reopened_merge_request, traits: [:reopened] factory :merge_request_with_diffs, traits: [:with_diffs] @@ -161,7 +166,6 @@ FactoryGirl.define do factory :note_on_issue, traits: [:on_issue], aliases: [:votable_note] factory :note_on_merge_request, traits: [:on_merge_request] factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff] - factory :note_on_merge_request_with_attachment, traits: [:on_merge_request, :with_attachment] trait :on_commit do project factory: :project diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index a3d8c462bf6..25a86b11fa9 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -1,14 +1,12 @@ require 'spec_helper' describe "On a merge request", js: true do - let!(:project) { create(:project) } - let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } - let!(:note) { create(:note_on_merge_request_with_attachment, project: project) } + let!(:merge_request) { create(:merge_request, :simple) } + let!(:project) { merge_request.source_project } + let!(:note) { create(:note_on_merge_request, :with_attachment, project: project) } before do - login_as :user - project.team << [@user, :master] - + login_as :admin visit project_merge_request_path(project, merge_request) end @@ -134,22 +132,20 @@ describe "On a merge request", js: true do end end -describe "On a merge request diff", js: true, focus: true do - let!(:project) { create(:project) } - let!(:merge_request) { create(:merge_request_with_diffs, source_project: project, target_project: project) } +describe "On a merge request diff", js: true do + let(:merge_request) { create(:merge_request, :with_diffs, :simple) } + let(:project) { merge_request.source_project } before do - login_as :user - project.team << [@user, :master] + login_as :admin visit diffs_project_merge_request_path(project, merge_request) end - subject { page } describe "when adding a note" do before do - find('a[data-line-code="4735dfc552ad7bf15ca468adc3cad9d05b624490_172_185"]').click + find('a[data-line-code="8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_7_7"]').click end describe "the notes holder" do @@ -160,13 +156,13 @@ describe "On a merge request diff", js: true, focus: true do describe "the note form" do it "shouldn't add a second form for same row" do - find('a[data-line-code="4735dfc552ad7bf15ca468adc3cad9d05b624490_172_185"]').click + find('a[data-line-code="8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_7_7"]').click - should have_css("tr[id='4735dfc552ad7bf15ca468adc3cad9d05b624490_172_185'] + .js-temp-notes-holder form", count: 1) + should have_css("tr[id='8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_7_7'] + .js-temp-notes-holder form", count: 1) end it "should be removed when canceled" do - within(".diff-file form[rel$='4735dfc552ad7bf15ca468adc3cad9d05b624490_172_185']") do + within(".diff-file form[rel$='8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_7_7']") do find(".js-close-discussion-note-form").trigger("click") end @@ -176,12 +172,9 @@ describe "On a merge request diff", js: true, focus: true do end describe "with muliple note forms" do - let!(:project) { create(:project) } - let!(:merge_request) { create(:merge_request_with_diffs, source_project: project, target_project: project) } - before do - find('a[data-line-code="4735dfc552ad7bf15ca468adc3cad9d05b624490_172_185"]').click - find('a[data-line-code="342e16cbbd482ac2047dc679b2749d248cc1428f_18_17"]').click + find('a[data-line-code="8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_7_7"]').click + find('a[data-line-code="8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_10_10"]').click end it { should have_css(".js-temp-notes-holder", count: 2) } @@ -189,12 +182,12 @@ describe "On a merge request diff", js: true, focus: true do describe "previewing them separately" do before do # add two separate texts and trigger previews on both - within("tr[id='4735dfc552ad7bf15ca468adc3cad9d05b624490_172_185'] + .js-temp-notes-holder") do - fill_in "note[note]", with: "One comment on line 185" + within("tr[id='8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_7_7'] + .js-temp-notes-holder") do + fill_in "note[note]", with: "One comment on line 7" find(".js-note-preview-button").trigger("click") end - within("tr[id='342e16cbbd482ac2047dc679b2749d248cc1428f_18_17'] + .js-temp-notes-holder") do - fill_in "note[note]", with: "Another comment on line 17" + within("tr[id='8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_10_10'] + .js-temp-notes-holder") do + fill_in "note[note]", with: "Another comment on line 10" find(".js-note-preview-button").trigger("click") end end @@ -202,14 +195,14 @@ describe "On a merge request diff", js: true, focus: true do describe "posting a note" do before do - within("tr[id='342e16cbbd482ac2047dc679b2749d248cc1428f_18_17'] + .js-temp-notes-holder") do - fill_in "note[note]", with: "Another comment on line 17" + within("tr[id='8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_10_10'] + .js-temp-notes-holder") do + fill_in "note[note]", with: "Another comment on line 10" click_button("Add Comment") end end it 'should be added as discussion' do - should have_content("Another comment on line 17") + should have_content("Another comment on line 10") should have_css(".notes_holder") should have_css(".notes_holder .note", count: 1) should have_link("Reply") diff --git a/spec/seed_project.tar.gz b/spec/seed_project.tar.gz index 92b9587e3f7..8d32a927da8 100644 Binary files a/spec/seed_project.tar.gz and b/spec/seed_project.tar.gz differ -- cgit v1.2.3 From a90574fab290233225375b1c9e9c232b0e540b52 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 19 Mar 2014 03:55:59 -0400 Subject: Speed up finders/merge_requests_finder_spec Uses the :simple merge request factory trait introduced by d166e70; cuts execution time of this spec in half. --- spec/finders/merge_requests_finder_spec.rb | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) (limited to 'spec') diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 76f9e753dd2..0bd2ccafcc1 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -1,13 +1,15 @@ require 'spec_helper' describe MergeRequestsFinder do - let(:user) { create :user } + let(:user) { create :user } let(:user2) { create :user } + let(:project1) { create(:project) } let(:project2) { create(:project) } - let(:merge_request1) { create(:merge_request, author: user, source_project: project1, target_project: project2) } - let(:merge_request2) { create(:merge_request, author: user, source_project: project2, target_project: project1) } - let(:merge_request3) { create(:merge_request, author: user, source_project: project2, target_project: project2) } + + let!(:merge_request1) { create(:merge_request, :simple, author: user, source_project: project1, target_project: project2) } + let!(:merge_request2) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1) } + let!(:merge_request3) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project2) } before do project1.team << [user, :master] @@ -15,13 +17,7 @@ describe MergeRequestsFinder do project2.team << [user2, :developer] end - describe :execute do - before :each do - merge_request1 - merge_request2 - merge_request3 - end - + describe "#execute" do it 'should filter by scope' do params = { scope: 'authored', state: 'opened' } merge_requests = MergeRequestsFinder.new.execute(user, params) -- cgit v1.2.3