From c452fa8124ffe18e2e74e14491dcb3e419e60057 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 13 May 2016 14:53:42 -0500 Subject: [PATCH] Update specs --- features/steps/shared/diff_note.rb | 12 +-- spec/factories/notes.rb | 4 +- spec/features/notes_on_merge_requests_spec.rb | 2 +- spec/models/note_spec.rb | 76 +------------------ 4 files changed, 10 insertions(+), 84 deletions(-) diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb index e846c52d474..e8b1e4b4879 100644 --- a/features/steps/shared/diff_note.rb +++ b/features/steps/shared/diff_note.rb @@ -23,7 +23,7 @@ module SharedDiffNote page.within(diff_file_selector) do click_diff_line(sample_commit.line_code) - page.within("form[id$='#{sample_commit.line_code}']") do + page.within("form[id$='#{sample_commit.line_code}-true']") do fill_in "note[note]", with: "Typo, please fix" find(".js-comment-button").trigger("click") sleep 0.05 @@ -33,7 +33,7 @@ module SharedDiffNote step 'I leave a diff comment in a parallel view on the left side like "Old comment"' do click_parallel_diff_line(sample_commit.line_code, 'old') - page.within("#{diff_file_selector} form[id$='#{sample_commit.line_code}']") do + page.within("#{diff_file_selector} form[id$='#{sample_commit.line_code}-true']") do fill_in "note[note]", with: "Old comment" find(".js-comment-button").trigger("click") end @@ -41,7 +41,7 @@ module SharedDiffNote step 'I leave a diff comment in a parallel view on the right side like "New comment"' do click_parallel_diff_line(sample_commit.line_code, 'new') - page.within("#{diff_file_selector} form[id$='#{sample_commit.line_code}']") do + page.within("#{diff_file_selector} form[id$='#{sample_commit.line_code}-true']") do fill_in "note[note]", with: "New comment" find(".js-comment-button").trigger("click") end @@ -51,7 +51,7 @@ module SharedDiffNote page.within(diff_file_selector) do click_diff_line(sample_commit.line_code) - page.within("form[id$='#{sample_commit.line_code}']") do + page.within("form[id$='#{sample_commit.line_code}-true']") do fill_in "note[note]", with: "Should fix it :smile:" find('.js-md-preview-button').click end @@ -62,7 +62,7 @@ module SharedDiffNote page.within(diff_file_selector) do click_diff_line(sample_commit.del_line_code) - page.within("form[id$='#{sample_commit.del_line_code}']") do + page.within("form[id$='#{sample_commit.del_line_code}-true']") do fill_in "note[note]", with: "DRY this up" find('.js-md-preview-button').click end @@ -91,7 +91,7 @@ module SharedDiffNote page.within(diff_file_selector) do click_diff_line(sample_commit.line_code) - page.within("form[id$='#{sample_commit.line_code}']") do + page.within("form[id$='#{sample_commit.line_code}-true']") do fill_in 'note[note]', with: ':smile:' click_button('Comment') end diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index 840b13196a6..26719f2652c 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -9,10 +9,10 @@ FactoryGirl.define do author factory :note_on_commit, traits: [:on_commit] - factory :note_on_commit_diff, traits: [:on_commit, :on_diff] + factory :note_on_commit_diff, traits: [:on_commit, :on_diff], class: LegacyDiffNote 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_diff, traits: [:on_merge_request, :on_diff], class: LegacyDiffNote factory :note_on_project_snippet, traits: [:on_project_snippet] factory :system_note, traits: [:system] factory :downvote_note, traits: [:award, :downvote] diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index 389812ff7e1..9e9fec01943 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -192,7 +192,7 @@ describe 'Comments', feature: true do end it 'should be removed when canceled' do - page.within(".diff-file form[id$='#{line_code}']") do + page.within(".diff-file form[id$='#{line_code}-true']") do find('.js-close-discussion-note-form').trigger('click') end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 264888cb376..5d916f0e6a6 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -34,20 +34,6 @@ describe Note, models: true do end end - describe "Commit diff line notes" do - let!(:note) { create(:note_on_commit_diff, note: "+1 from me") } - let!(:commit) { note.noteable } - - it "should save a valid note" do - expect(note.commit_id).to eq(commit.id) - expect(note.noteable.id).to eq(commit.id) - end - - it "should be recognized by #legacy_diff_note?" do - expect(note).to be_legacy_diff_note - end - end - describe 'authorization' do before do @p1 = create(:project) @@ -144,66 +130,6 @@ describe Note, models: true do end end - describe '#active?' do - it 'is always true when the note has no associated diff' do - note = build(:note) - - expect(note).to receive(:diff).and_return(nil) - - expect(note).to be_active - end - - it 'is never true when the note has no noteable associated' do - note = build(:note) - - expect(note).to receive(:diff).and_return(double) - expect(note).to receive(:noteable).and_return(nil) - - expect(note).not_to be_active - end - - it 'returns the memoized value if defined' do - note = build(:note) - - expect(note).to receive(:diff).and_return(double) - expect(note).to receive(:noteable).and_return(double) - - note.instance_variable_set(:@active, 'foo') - expect(note).not_to receive(:find_noteable_diff) - - expect(note.active?).to eq 'foo' - end - - context 'for a merge request noteable' do - it 'is false when noteable has no matching diff' do - merge = build_stubbed(:merge_request, :simple) - note = build(:note, noteable: merge) - - allow(note).to receive(:diff).and_return(double) - expect(note).to receive(:find_noteable_diff).and_return(nil) - - expect(note).not_to be_active - end - - it 'is true when noteable has a matching diff' do - merge = create(:merge_request, :simple) - - # Generate a real line_code value so we know it will match. We use a - # random line from a random diff just for funsies. - diff = merge.diffs.to_a.sample - line = Gitlab::Diff::Parser.new.parse(diff.diff.each_line).to_a.sample - code = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos) - - # We're persisting in order to trigger the set_diff callback - note = create(:note, noteable: merge, line_code: code) - - # Make sure we don't get a false positive from a guard clause - expect(note).to receive(:find_noteable_diff).and_call_original - expect(note).to be_active - end - end - end - describe "editable?" do it "returns true" do note = build(:note) @@ -254,7 +180,7 @@ describe Note, models: true do end it "is not an award emoji when comment is on a diff" do - note = create(:note, note: ":blowfish:", noteable: merge_request, line_code: "11d5d2e667e9da4f7f610f81d86c974b146b13bd_0_2") + note = create(:note_on_merge_request_diff, note: ":blowfish:", noteable: merge_request, line_code: "11d5d2e667e9da4f7f610f81d86c974b146b13bd_0_2") note = note.reload expect(note.note).to eq(":blowfish:")