From ef24576fc2239935d2d7b553e7d55674abf4eb4c Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sun, 2 Sep 2012 02:13:13 -0400 Subject: [PATCH] Redesign gfm helper specs Should now be much clearer about what each spec is actually testing. For example, instead of testing stuff like link classes and titles in every single call, we only test those things once, in their own specs. --- spec/helpers/gitlab_markdown_helper_spec.rb | 344 ++++++++++++-------- 1 file changed, 200 insertions(+), 144 deletions(-) diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index 28bd46ecb99..00164e0cdd7 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -1,232 +1,288 @@ require "spec_helper" describe GitlabMarkdownHelper do + let!(:project) { create(:project) } + + let(:user) { create(:user, name: 'gfm') } + let(:commit) { CommitDecorator.decorate(project.commit) } + let(:issue) { create(:issue, project: project) } + let(:merge_request) { create(:merge_request, project: project) } + let(:snippet) { create(:snippet, project: project) } + let(:member) { project.users_projects.where(user_id: user).first } + before do - @project = Factory(:project) - @commit = @project.repo.commits.first.parents.first - @commit = CommitDecorator.decorate(Commit.new(@commit)) - @other_project = Factory :project, path: "OtherPath", code: "OtherCode" - @fake_user = Factory :user, name: "fred" + # Helper expects a @project instance variable + @project = project end describe "#gfm" do - it "should return text if @project is not set" do - @project = nil + it "should return unaltered text if project is nil" do + actual = "Testing references: ##{issue.id}" - gfm("foo").should == "foo" + gfm(actual).should_not == actual + + @project = nil + gfm(actual).should == actual + end + + it "should not alter non-references" do + actual = expected = "_Please_ *stop* 'helping' and all the other b*$#%' you do." + gfm(actual).should == expected + end + + it "should not touch HTML entities" do + actual = expected = "We'll accept good pull requests." + gfm(actual).should == expected + end + + it "should forward HTML options to links" do + gfm("Fixed in #{commit.id}", class: "foo").should have_selector("a.gfm.foo") end describe "referencing a commit" do + let(:expected) { project_commit_path(project, commit) } + it "should link using a full id" do - gfm("Reverts changes from #{@commit.id}").should == "Reverts changes from #{link_to @commit.id, project_commit_path(@project, id: @commit.id), title: "Commit: #{@commit.author_name} - #{@commit.title}", class: "gfm gfm-commit "}" + actual = "Reverts #{commit.id}" + gfm(actual).should match(expected) end it "should link using a short id" do - gfm("Backported from #{@commit.id[0, 6]}").should == "Backported from #{link_to @commit.id[0, 6], project_commit_path(@project, id: @commit.id), title: "Commit: #{@commit.author_name} - #{@commit.title}", class: "gfm gfm-commit "}" + actual = "Backported from #{commit.short_id(6)}" + gfm(actual).should match(expected) end - it "should link with adjecent text" do - gfm("Reverted (see #{@commit.id})").should == "Reverted (see #{link_to @commit.id, project_commit_path(@project, id: @commit.id), title: "Commit: #{@commit.author_name} - #{@commit.title}", class: "gfm gfm-commit "})" + it "should link with adjacent text" do + actual = "Reverted (see #{commit.id})" + gfm(actual).should match(expected) + end + + it "should keep whitespace intact" do + actual = "Changes #{commit.id} dramatically" + expected = /Changes #{commit.id}<\/a> dramatically/ + gfm(actual).should match(expected) end it "should not link with an invalid id" do - gfm("What happened in 12345678?").should == "What happened in 12345678?" + actual = expected = "What happened in #{commit.id.reverse}" + gfm(actual).should == expected + end + + it "should include a title attribute" do + actual = "Reverts #{commit.id}" + gfm(actual).should match(/title="#{commit.link_title}"/) + end + + it "should include standard gfm classes" do + actual = "Reverts #{commit.id}" + gfm(actual).should match(/class="\s?gfm gfm-commit\s?"/) end end describe "referencing a team member" do - it "should link using a simple name" do - user = Factory :user, name: "barry" - @project.users << user - member = @project.users_projects.where(user_id: user).first + let(:actual) { "@#{user.name} you are right." } + let(:expected) { project_team_member_path(project, member) } - gfm("@#{user.name} you are right").should == "#{link_to "@#{user.name}", project_team_member_path(@project, member), class: "gfm gfm-team_member "} you are right" + before do + project.users << user + end + + it "should link using a simple name" do + gfm(actual).should match(expected) end it "should link using a name with dots" do - user = Factory :user, name: "alphA.Beta" - @project.users << user - member = @project.users_projects.where(user_id: user).first - - gfm("@#{user.name} you are right").should == "#{link_to "@#{user.name}", project_team_member_path(@project, member), class: "gfm gfm-team_member "} you are right" + user.update_attributes(name: "alphA.Beta") + gfm(actual).should match(expected) end it "should link using name with underscores" do - user = Factory :user, name: "ping_pong_king" - @project.users << user - member = @project.users_projects.where(user_id: user).first - - gfm("@#{user.name} you are right").should == "#{link_to "@#{user.name}", project_team_member_path(@project, member), class: "gfm gfm-team_member "} you are right" + user.update_attributes(name: "ping_pong_king") + gfm(actual).should match(expected) end - it "should link with adjecent text" do - user = Factory.create(:user, name: "ace") - @project.users << user - member = @project.users_projects.where(user_id: user).first - - gfm("Mail the Admin (@#{user.name})").should == "Mail the Admin (#{link_to "@#{user.name}", project_team_member_path(@project, member), class: "gfm gfm-team_member "})" + it "should link with adjacent text" do + actual = "Mail the admin (@gfm)" + gfm(actual).should match(expected) end - it "should add styles" do - user = Factory :user, name: "barry" - @project.users << user - gfm("@#{user.name} you are right").should have_selector(".gfm.gfm-team_member") + it "should keep whitespace intact" do + actual = "Yes, @#{user.name} is right." + expected = /Yes, @#{user.name}<\/a> is right/ + gfm(actual).should match(expected) end - it "should not link using a bogus name" do - gfm("What hapened to @foo?").should == "What hapened to @foo?" + it "should not link with an invalid id" do + actual = expected = "@#{user.name.reverse} you are right." + gfm(actual).should == expected + end + + it "should include standard gfm classes" do + gfm(actual).should match(/class="\s?gfm gfm-team_member\s?"/) + end + end + + # Shared examples for referencing an object + # + # Expects the following attributes to be available in the example group: + # + # - object - The object itself + # - reference - The object reference string (e.g., #1234, $1234, !1234) + # + # Currently limited to Snippets, Issues and MergeRequests + shared_examples 'referenced object' do + let(:actual) { "Reference to #{reference}" } + let(:expected) { polymorphic_path([project, object]) } + + it "should link using a valid id" do + gfm(actual).should match(expected) + end + + it "should link with adjacent text" do + # Wrap the reference in parenthesis + gfm(actual.gsub(reference, "(#{reference})")).should match(expected) + + # Append some text to the end of the reference + gfm(actual.gsub(reference, "#{reference}, right?")).should match(expected) + end + + it "should keep whitespace intact" do + actual = "Referenced #{reference} already." + expected = /Referenced [^\s]+<\/a> already/ + gfm(actual).should match(expected) + end + + it "should not link with an invalid id" do + # Modify the reference string so it's still parsed, but is invalid + reference.gsub!(/^(.)(\d+)$/, '\1' + ('\2' * 2)) + gfm(actual).should == actual + end + + it "should include a title attribute" do + title = "#{object.class.to_s.titlecase}: #{object.title}" + gfm(actual).should match(/title="#{title}"/) + end + + it "should include standard gfm classes" do + css = object.class.to_s.underscore + gfm(actual).should match(/class="\s?gfm gfm-#{css}\s?"/) end end describe "referencing an issue" do - before do - @issue = Factory :issue, assignee: @fake_user, author: @fake_user, project: @project - @invalid_issue = Factory :issue, assignee: @fake_user, author: @fake_user, project: @other_project - end + let(:object) { issue } + let(:reference) { "##{issue.id}" } - it "should link using a correct id" do - gfm("Fixes ##{@issue.id}").should == "Fixes #{link_to "##{@issue.id}", project_issue_path(@project, @issue), title: "Issue: #{@issue.title}", class: "gfm gfm-issue "}" - end - - it "should link with adjecent text" do - gfm("This has already been discussed (see ##{@issue.id})").should == "This has already been discussed (see #{link_to "##{@issue.id}", project_issue_path(@project, @issue), title: "Issue: #{@issue.title}", class: "gfm gfm-issue "})" - end - - it "should add styles" do - gfm("Fixes ##{@issue.id}").should have_selector(".gfm.gfm-issue") - end - - it "should not link using an invalid id" do - gfm("##{@invalid_issue.id} has been marked duplicate of this").should == "##{@invalid_issue.id} has been marked duplicate of this" - end + include_examples 'referenced object' end describe "referencing a merge request" do - before do - @merge_request = Factory :merge_request, assignee: @fake_user, author: @fake_user, project: @project - @invalid_merge_request = Factory :merge_request, assignee: @fake_user, author: @fake_user, project: @other_project - end + let(:object) { merge_request } + let(:reference) { "!#{merge_request.id}" } - it "should link using a correct id" do - gfm("Fixed in !#{@merge_request.id}").should == "Fixed in #{link_to "!#{@merge_request.id}", project_merge_request_path(@project, @merge_request), title: "Merge Request: #{@merge_request.title}", class: "gfm gfm-merge_request "}" - end - - it "should link with adjecent text" do - gfm("This has been fixed already (see !#{@merge_request.id})").should == "This has been fixed already (see #{link_to "!#{@merge_request.id}", project_merge_request_path(@project, @merge_request), title: "Merge Request: #{@merge_request.title}", class: "gfm gfm-merge_request "})" - end - - it "should add styles" do - gfm("Fixed in !#{@merge_request.id}").should have_selector(".gfm.gfm-merge_request") - end - - it "should not link using an invalid id" do - gfm("!#{@invalid_merge_request.id} violates our coding guidelines") - end + include_examples 'referenced object' end describe "referencing a snippet" do - before do - @snippet = Factory.create(:snippet, - title: "Render asset to string", - author: @fake_user, - project: @project) - end + let(:object) { snippet } + let(:reference) { "$#{snippet.id}" } - it "should link using a correct id" do - gfm("Check out $#{@snippet.id}").should == "Check out #{link_to "$#{@snippet.id}", project_snippet_path(@project, @snippet), title: "Snippet: #{@snippet.title}", class: "gfm gfm-snippet "}" - end - - it "should link with adjecent text" do - gfm("I have created a snippet for that ($#{@snippet.id})").should == "I have created a snippet for that (#{link_to "$#{@snippet.id}", project_snippet_path(@project, @snippet), title: "Snippet: #{@snippet.title}", class: "gfm gfm-snippet "})" - end - - it "should add styles" do - gfm("Check out $#{@snippet.id}").should have_selector(".gfm.gfm-snippet") - end - - it "should not link using an invalid id" do - gfm("Don't use $1234").should == "Don't use $1234" - end + include_examples 'referenced object' end - it "should link to multiple things" do - user = Factory :user, name: "barry" - @project.users << user - member = @project.users_projects.where(user_id: user).first + describe "referencing multiple objects" do + let(:actual) { "!#{merge_request.id} -> #{commit.id} -> ##{issue.id}" } - gfm("Let @#{user.name} fix the *mess* in #{@commit.id}").should == "Let #{link_to "@#{user.name}", project_team_member_path(@project, member), class: "gfm gfm-team_member "} fix the *mess* in #{link_to @commit.id, project_commit_path(@project, id: @commit.id), title: "Commit: #{@commit.author_name} - #{@commit.title}", class: "gfm gfm-commit "}" - end + it "should link to the merge request" do + expected = project_merge_request_path(project, merge_request) + gfm(actual).should match(expected) + end - it "should not trip over other stuff" do - gfm("_Please_ *stop* 'helping' and all the other b*$#%' you do.").should == "_Please_ *stop* 'helping' and all the other b*$#%' you do." - end + it "should link to the commit" do + expected = project_commit_path(project, commit) + gfm(actual).should match(expected) + end - it "should not touch HTML entities" do - gfm("We'll accept good pull requests.").should == "We'll accept good pull requests." - end - - it "should forward HTML options to links" do - gfm("fixed in #{@commit.id}", class: "foo").should have_selector("a.foo") + it "should link to the issue" do + expected = project_issue_path(project, issue) + gfm(actual).should match(expected) + end end end describe "#link_to_gfm" do - let(:issue1) { Factory :issue, assignee: @fake_user, author: @fake_user, project: @project } - let(:issue2) { Factory :issue, assignee: @fake_user, author: @fake_user, project: @project } + let(:commit_path) { project_commit_path(project, commit) } + let(:issues) { create_list(:issue, 2, project: project) } it "should handle references nested in links with all the text" do - link_to_gfm("This should finally fix ##{issue1.id} and ##{issue2.id} for real", project_commit_path(@project, id: @commit.id)).should == "#{link_to "This should finally fix ", project_commit_path(@project, id: @commit.id)}#{link_to "##{issue1.id}", project_issue_path(@project, issue1), title: "Issue: #{issue1.title}", class: "gfm gfm-issue "}#{link_to " and ", project_commit_path(@project, id: @commit.id)}#{link_to "##{issue2.id}", project_issue_path(@project, issue2), title: "Issue: #{issue2.title}", class: "gfm gfm-issue "}#{link_to " for real", project_commit_path(@project, id: @commit.id)}" + actual = link_to_gfm("This should finally fix ##{issues[0].id} and ##{issues[1].id} for real", commit_path) + + # Break the result into groups of links with their content, without + # closing tags + groups = actual.split("") + + # Leading commit link + groups[0].should match(/href="#{commit_path}"/) + groups[0].should match(/This should finally fix $/) + + # First issue link + groups[1].should match(/href="#{project_issue_path(project, issues[0])}"/) + groups[1].should match(/##{issues[0].id}$/) + + # Internal commit link + groups[2].should match(/href="#{commit_path}"/) + groups[2].should match(/ and /) + + # Second issue link + groups[3].should match(/href="#{project_issue_path(project, issues[1])}"/) + groups[3].should match(/##{issues[1].id}$/) + + # Trailing commit link + groups[4].should match(/href="#{commit_path}"/) + groups[4].should match(/ for real$/) end it "should forward HTML options" do - link_to_gfm("This should finally fix ##{issue1.id} for real", project_commit_path(@project, id: @commit.id), class: "foo").should have_selector(".foo") + actual = link_to_gfm("Fixed in #{commit.id}", commit_path, class: 'foo') + actual.should have_selector 'a.gfm.gfm-commit.foo' end end describe "#markdown" do - before do - @issue = Factory :issue, assignee: @fake_user, author: @fake_user, project: @project - @merge_request = Factory :merge_request, assignee: @fake_user, author: @fake_user, project: @project - @note = Factory.create(:note, - note: "Screenshot of the new feature", - project: @project, - noteable_id: @commit.id, - noteable_type: "Commit", - attachment: "screenshot123.jpg") - @snippet = Factory.create(:snippet, - title: "Render asset to string", - author: @fake_user, - project: @project) - - @other_user = Factory :user, name: "bill" - @project.users << @other_user - @member = @project.users_projects.where(user_id: @other_user).first - end - it "should handle references in paragraphs" do - markdown("\n\nLorem ipsum dolor sit amet, consectetur adipiscing elit. #{@commit.id} Nam pulvinar sapien eget odio adipiscing at faucibus orci vestibulum.\n").should == "

Lorem ipsum dolor sit amet, consectetur adipiscing elit. #{link_to @commit.id, project_commit_path(@project, id: @commit.id), title: "Commit: #{@commit.author_name} - #{@commit.title}", class: "gfm gfm-commit "} Nam pulvinar sapien eget odio adipiscing at faucibus orci vestibulum.

\n" + markdown("\n\nLorem ipsum dolor sit amet, consectetur adipiscing elit. #{commit.id} Nam pulvinar sapien eget odio adipiscing at faucibus orci vestibulum.\n").should == "

Lorem ipsum dolor sit amet, consectetur adipiscing elit. #{link_to commit.id, project_commit_path(project, commit), title: commit.link_title, class: "gfm gfm-commit "} Nam pulvinar sapien eget odio adipiscing at faucibus orci vestibulum.

\n" end it "should handle references in headers" do - markdown("\n# Working around ##{@issue.id} for now\n## Apply !#{@merge_request.id}").should == "

Working around #{link_to "##{@issue.id}", project_issue_path(@project, @issue), title: "Issue: #{@issue.title}", class: "gfm gfm-issue "} for now

\n\n

Apply #{link_to "!#{@merge_request.id}", project_merge_request_path(@project, @merge_request), title: "Merge Request: #{@merge_request.title}", class: "gfm gfm-merge_request "}

\n" + actual = "\n# Working around ##{issue.id}\n## Apply !#{merge_request.id}" + + markdown(actual).should match(%r{Working around ##{issue.id}}) + markdown(actual).should match(%r{Apply !#{merge_request.id}}) end it "should handle references in lists" do - markdown("\n* dark: ##{@issue.id}\n* light by @#{@other_user.name}\n").should == "
    \n
  • dark: #{link_to "##{@issue.id}", project_issue_path(@project, @issue), title: "Issue: #{@issue.title}", class: "gfm gfm-issue "}
  • \n
  • light by #{link_to "@#{@other_user.name}", project_team_member_path(@project, @member), class: "gfm gfm-team_member "}
  • \n
\n" + project.users << user + + actual = "\n* dark: ##{issue.id}\n* light by @#{member.user_name}" + + markdown(actual).should match(%r{
  • dark: ##{issue.id}
  • }) + markdown(actual).should match(%r{
  • light by @#{member.user_name}
  • }) end it "should handle references in " do - markdown("Apply _!#{@merge_request.id}_ ASAP").should == "

    Apply #{link_to "!#{@merge_request.id}", project_merge_request_path(@project, @merge_request), title: "Merge Request: #{@merge_request.title}", class: "gfm gfm-merge_request "} ASAP

    \n" + actual = "Apply _!#{merge_request.id}_ ASAP" + + markdown(actual).should match(%r{Apply !#{merge_request.id}}) end it "should leave code blocks untouched" do - markdown("\n some code from $#{@snippet.id}\n here too\n").should == "
    some code from $#{@snippet.id}\nhere too\n
    \n
    \n" + markdown("\n some code from $#{snippet.id}\n here too\n").should == "
    some code from $#{snippet.id}\nhere too\n
    \n
    \n" - markdown("\n```\nsome code from $#{@snippet.id}\nhere too\n```\n").should == "
    some code from $#{@snippet.id}\nhere too\n
    \n
    \n" + markdown("\n```\nsome code from $#{snippet.id}\nhere too\n```\n").should == "
    some code from $#{snippet.id}\nhere too\n
    \n
    \n" end it "should leave inline code untouched" do - markdown("\nDon't use `$#{@snippet.id}` here.\n").should == "

    Don't use $#{@snippet.id} here.

    \n" + markdown("\nDon't use `$#{snippet.id}` here.\n").should == "

    Don't use $#{snippet.id} here.

    \n" end end end