From b0480dd34e4ac7b3c0d0aa2262fc21ecedae7653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= Date: Thu, 11 Jul 2024 12:15:48 +0200 Subject: [PATCH] DEV: Avoid instance variables in specs Small followup of https://github.com/discourse/discourse/pull/27705 --- .../engine/github_actions_onebox_spec.rb | 32 ++++++------ .../onebox/engine/github_blob_onebox_spec.rb | 28 ++++++----- .../engine/github_commit_onebox_spec.rb | 50 ++++++++++--------- .../onebox/engine/github_issue_onebox_spec.rb | 13 ++--- .../engine/github_pull_request_onebox_spec.rb | 44 ++++++++-------- .../onebox/engine/wikipedia_onebox_spec.rb | 16 +++--- spec/support/onebox_helpers.rb | 14 +++--- 7 files changed, 105 insertions(+), 92 deletions(-) diff --git a/spec/lib/onebox/engine/github_actions_onebox_spec.rb b/spec/lib/onebox/engine/github_actions_onebox_spec.rb index 37130b3a10b..f1e655b735c 100644 --- a/spec/lib/onebox/engine/github_actions_onebox_spec.rb +++ b/spec/lib/onebox/engine/github_actions_onebox_spec.rb @@ -2,23 +2,26 @@ RSpec.describe Onebox::Engine::GithubActionsOnebox do describe "PR check run" do - before do - @link = "https://github.com/discourse/discourse/pull/13128/checks?check_run_id=2660861130" - @pr_run_uri = "https://api.github.com/repos/discourse/discourse/pulls/13128" - @run_uri = "https://api.github.com/repos/discourse/discourse/check-runs/2660861130" + let(:pr_run_uri) { "https://api.github.com/repos/discourse/discourse/pulls/13128" } + let(:run_uri) { "https://api.github.com/repos/discourse/discourse/check-runs/2660861130" } - stub_request(:get, @pr_run_uri).to_return( + before do + stub_request(:get, pr_run_uri).to_return( status: 200, body: onebox_response("githubactions_pr"), ) - stub_request(:get, @run_uri).to_return( + stub_request(:get, run_uri).to_return( status: 200, body: onebox_response("githubactions_pr_run"), ) end - include_context "with engines" + include_context "with engines" do + let(:link) do + "https://github.com/discourse/discourse/pull/13128/checks?check_run_id=2660861130" + end + end it_behaves_like "an engine" describe "#to_html" do @@ -36,7 +39,7 @@ RSpec.describe Onebox::Engine::GithubActionsOnebox do it "sends it as part of the request" do html - expect(WebMock).to have_requested(:get, @run_uri).with( + expect(WebMock).to have_requested(:get, run_uri).with( headers: { "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}", }, @@ -46,17 +49,18 @@ RSpec.describe Onebox::Engine::GithubActionsOnebox do end describe "GitHub Actions run" do - before do - @link = "https://github.com/discourse/discourse/actions/runs/873214216" - @pr_run_uri = "https://api.github.com/repos/discourse/discourse/actions/runs/873214216" + let(:pr_run_uri) { "https://api.github.com/repos/discourse/discourse/actions/runs/873214216" } - stub_request(:get, @pr_run_uri).to_return( + before do + stub_request(:get, pr_run_uri).to_return( status: 200, body: onebox_response("githubactions_actions_run"), ) end - include_context "with engines" + include_context "with engines" do + let(:link) { "https://github.com/discourse/discourse/actions/runs/873214216" } + end it_behaves_like "an engine" describe "#to_html" do @@ -78,7 +82,7 @@ RSpec.describe Onebox::Engine::GithubActionsOnebox do it "sends it as part of the request" do html - expect(WebMock).to have_requested(:get, @pr_run_uri).with( + expect(WebMock).to have_requested(:get, pr_run_uri).with( headers: { "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}", }, diff --git a/spec/lib/onebox/engine/github_blob_onebox_spec.rb b/spec/lib/onebox/engine/github_blob_onebox_spec.rb index 6d77c2e00d3..473798a28f2 100644 --- a/spec/lib/onebox/engine/github_blob_onebox_spec.rb +++ b/spec/lib/onebox/engine/github_blob_onebox_spec.rb @@ -1,19 +1,23 @@ # frozen_string_literal: true RSpec.describe Onebox::Engine::GithubBlobOnebox do + let(:raw_uri) do + "https://raw.githubusercontent.com/discourse/onebox/master/lib/onebox/engine/github_blob_onebox.rb" + end + before do - @link = - "https://github.com/discourse/onebox/blob/master/lib/onebox/engine/github_blob_onebox.rb" - @uri = URI.parse(@link) - @raw_uri = - "https://raw.githubusercontent.com/discourse/onebox/master/lib/onebox/engine/github_blob_onebox.rb" - stub_request(:get, @raw_uri).to_return( + stub_request(:get, raw_uri).to_return( status: 200, body: onebox_response(described_class.onebox_name), ) end - include_context "with engines" + include_context "with engines" do + let(:link) do + "https://github.com/discourse/onebox/blob/master/lib/onebox/engine/github_blob_onebox.rb" + end + let(:uri) { URI.parse(link) } + end it_behaves_like "an engine" describe "#to_html" do @@ -27,15 +31,15 @@ RSpec.describe Onebox::Engine::GithubBlobOnebox do it "does not include blob contents if it is binary" do # stub_request if the response must be binary (ASCII-8BIT) - uri = mock("object") - uri.stubs(:open).returns(File.open("#{Rails.root}/spec/fixtures/pdf/small.pdf", "rb")) - URI.stubs(:parse).with(@link).returns(@uri) + uri_mock = mock("object") + uri_mock.stubs(:open).returns(File.open("#{Rails.root}/spec/fixtures/pdf/small.pdf", "rb")) + URI.stubs(:parse).with(link).returns(uri) URI .stubs(:parse) .with( "https://raw.githubusercontent.com/discourse/onebox/master/lib/onebox/engine/github_blob_onebox.rb", ) - .returns(uri) + .returns(uri_mock) expect(html).not_to include("/Pages") expect(html).to include("This file is binary.") @@ -46,7 +50,7 @@ RSpec.describe Onebox::Engine::GithubBlobOnebox do it "sends it as part of the request" do html - expect(WebMock).to have_requested(:get, @raw_uri).with( + expect(WebMock).to have_requested(:get, raw_uri).with( headers: { "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}", }, diff --git a/spec/lib/onebox/engine/github_commit_onebox_spec.rb b/spec/lib/onebox/engine/github_commit_onebox_spec.rb index 6551cb4ac7f..b1082d80719 100644 --- a/spec/lib/onebox/engine/github_commit_onebox_spec.rb +++ b/spec/lib/onebox/engine/github_commit_onebox_spec.rb @@ -3,15 +3,17 @@ RSpec.describe Onebox::Engine::GithubCommitOnebox do describe "regular commit url" do before do - @link = - "https://github.com/discourse/discourse/commit/803d023e2307309f8b776ab3b8b7e38ba91c0919" - @uri = - "https://api.github.com/repos/discourse/discourse/commits/803d023e2307309f8b776ab3b8b7e38ba91c0919" - - stub_request(:get, @uri).to_return(status: 200, body: onebox_response("githubcommit")) + stub_request( + :get, + "https://api.github.com/repos/discourse/discourse/commits/803d023e2307309f8b776ab3b8b7e38ba91c0919", + ).to_return(status: 200, body: onebox_response("githubcommit")) end - include_context "with engines" + include_context "with engines" do + let(:link) do + "https://github.com/discourse/discourse/commit/803d023e2307309f8b776ab3b8b7e38ba91c0919" + end + end it_behaves_like "an engine" describe "#to_html" do @@ -57,26 +59,27 @@ RSpec.describe Onebox::Engine::GithubCommitOnebox do it "sends it as part of the request" do html - expect(WebMock).to have_requested(:get, @uri).with( - headers: { - "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}", - }, - ) + expect(WebMock).to have_requested( + :get, + "https://api.github.com/repos/discourse/discourse/commits/803d023e2307309f8b776ab3b8b7e38ba91c0919", + ).with(headers: { "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}" }) end end end describe "PR with commit URL" do before do - @link = - "https://github.com/discourse/discourse/pull/4662/commit/803d023e2307309f8b776ab3b8b7e38ba91c0919" - @uri = - "https://api.github.com/repos/discourse/discourse/commits/803d023e2307309f8b776ab3b8b7e38ba91c0919" - - stub_request(:get, @uri).to_return(status: 200, body: onebox_response("githubcommit")) + stub_request( + :get, + "https://api.github.com/repos/discourse/discourse/commits/803d023e2307309f8b776ab3b8b7e38ba91c0919", + ).to_return(status: 200, body: onebox_response("githubcommit")) end - include_context "with engines" + include_context "with engines" do + let(:link) do + "https://github.com/discourse/discourse/pull/4662/commit/803d023e2307309f8b776ab3b8b7e38ba91c0919" + end + end # TODO: fix test to make sure it's not failing when matching object # it_behaves_like "an engine" @@ -123,11 +126,10 @@ RSpec.describe Onebox::Engine::GithubCommitOnebox do it "sends it as part of the request" do html - expect(WebMock).to have_requested(:get, @uri).with( - headers: { - "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}", - }, - ) + expect(WebMock).to have_requested( + :get, + "https://api.github.com/repos/discourse/discourse/commits/803d023e2307309f8b776ab3b8b7e38ba91c0919", + ).with(headers: { "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}" }) end end end diff --git a/spec/lib/onebox/engine/github_issue_onebox_spec.rb b/spec/lib/onebox/engine/github_issue_onebox_spec.rb index 6a37be69726..4801889ff47 100644 --- a/spec/lib/onebox/engine/github_issue_onebox_spec.rb +++ b/spec/lib/onebox/engine/github_issue_onebox_spec.rb @@ -1,17 +1,18 @@ # frozen_string_literal: true RSpec.describe Onebox::Engine::GithubIssueOnebox do - before do - @link = "https://github.com/discourse/discourse/issues/1" - @issue_uri = "https://api.github.com/repos/discourse/discourse/issues/1" + let(:issue_uri) { "https://api.github.com/repos/discourse/discourse/issues/1" } - stub_request(:get, @issue_uri).to_return( + before do + stub_request(:get, issue_uri).to_return( status: 200, body: onebox_response("github_issue_onebox"), ) end - include_context "with engines" + include_context "with engines" do + let(:link) { "https://github.com/discourse/discourse/issues/1" } + end it_behaves_like "an engine" describe "#to_html" do @@ -27,7 +28,7 @@ RSpec.describe Onebox::Engine::GithubIssueOnebox do it "sends it as part of the request" do html - expect(WebMock).to have_requested(:get, @issue_uri).with( + expect(WebMock).to have_requested(:get, issue_uri).with( headers: { "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}", }, diff --git a/spec/lib/onebox/engine/github_pull_request_onebox_spec.rb b/spec/lib/onebox/engine/github_pull_request_onebox_spec.rb index dbb00b4f133..c787d75db43 100644 --- a/spec/lib/onebox/engine/github_pull_request_onebox_spec.rb +++ b/spec/lib/onebox/engine/github_pull_request_onebox_spec.rb @@ -1,17 +1,19 @@ # frozen_string_literal: true RSpec.describe Onebox::Engine::GithubPullRequestOnebox do - before do - @link = "https://github.com/discourse/discourse/pull/1253/" - @uri = "https://api.github.com/repos/discourse/discourse/pulls/1253" + let(:gh_link) { "https://github.com/discourse/discourse/pull/1253/" } + let(:api_uri) { "https://api.github.com/repos/discourse/discourse/pulls/1253" } - stub_request(:get, @uri).to_return( + before do + stub_request(:get, api_uri).to_return( status: 200, body: onebox_response(described_class.onebox_name), ) end - include_context "with engines" + include_context "with engines" do + let(:link) { gh_link } + end it_behaves_like "an engine" describe "#to_html" do @@ -54,16 +56,15 @@ RSpec.describe Onebox::Engine::GithubPullRequestOnebox do end context "with commit links" do - before do - @link = - "https://github.com/discourse/discourse/pull/1253/commits/d7d3be1130c665cc7fab9f05dbf32335229137a6" - @uri = - "https://api.github.com/repos/discourse/discourse/commits/d7d3be1130c665cc7fab9f05dbf32335229137a6" + let(:gh_link) do + "https://github.com/discourse/discourse/pull/1253/commits/d7d3be1130c665cc7fab9f05dbf32335229137a6" + end - stub_request(:get, @uri).to_return( - status: 200, - body: onebox_response(described_class.onebox_name + "_commit"), - ) + before do + stub_request( + :get, + "https://api.github.com/repos/discourse/discourse/commits/d7d3be1130c665cc7fab9f05dbf32335229137a6", + ).to_return(status: 200, body: onebox_response(described_class.onebox_name + "_commit")) end it "includes commit name" do @@ -76,14 +77,13 @@ RSpec.describe Onebox::Engine::GithubPullRequestOnebox do end context "with comment links" do - before do - @link = "https://github.com/discourse/discourse/pull/1253/#issuecomment-21597425" - @uri = "https://api.github.com/repos/discourse/discourse/issues/comments/21597425" + let(:gh_link) { "https://github.com/discourse/discourse/pull/1253/#issuecomment-21597425" } - stub_request(:get, @uri).to_return( - status: 200, - body: onebox_response(described_class.onebox_name + "_comment"), - ) + before do + stub_request( + :get, + "https://api.github.com/repos/discourse/discourse/issues/comments/21597425", + ).to_return(status: 200, body: onebox_response(described_class.onebox_name + "_comment")) end it "includes comment" do @@ -96,7 +96,7 @@ RSpec.describe Onebox::Engine::GithubPullRequestOnebox do it "sends it as part of the request" do html - expect(WebMock).to have_requested(:get, @uri).with( + expect(WebMock).to have_requested(:get, api_uri).with( headers: { "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}", }, diff --git a/spec/lib/onebox/engine/wikipedia_onebox_spec.rb b/spec/lib/onebox/engine/wikipedia_onebox_spec.rb index 176d04880e0..819ff75c27d 100644 --- a/spec/lib/onebox/engine/wikipedia_onebox_spec.rb +++ b/spec/lib/onebox/engine/wikipedia_onebox_spec.rb @@ -1,16 +1,18 @@ # frozen_string_literal: true RSpec.describe Onebox::Engine::WikipediaOnebox do - before do - @link = "http://en.wikipedia.org/wiki/Billy_Jack" + let(:wp_link) { "http://en.wikipedia.org/wiki/Billy_Jack" } + before do stub_request(:get, "https://en.wikipedia.org/wiki/Billy_Jack").to_return( status: 200, body: onebox_response(described_class.onebox_name), ) end - include_context "with engines" + include_context "with engines" do + let(:link) { wp_link } + end it_behaves_like "an engine" describe "#to_html" do @@ -24,7 +26,7 @@ RSpec.describe Onebox::Engine::WikipediaOnebox do end describe "url with section hash" do - before { @link = "http://en.wikipedia.org/wiki/Billy_Jack#Soundtrack" } + let(:wp_link) { "http://en.wikipedia.org/wiki/Billy_Jack#Soundtrack" } it "includes summary" do expect(html).to include("The film score was composed") @@ -32,9 +34,11 @@ RSpec.describe Onebox::Engine::WikipediaOnebox do end describe "url with url-encoded section hash" do - before do - @link = "https://fr.wikipedia.org/wiki/Th%C3%A9ologie#La_th%C3%A9ologie_selon_Aristote" + let(:wp_link) do + "https://fr.wikipedia.org/wiki/Th%C3%A9ologie#La_th%C3%A9ologie_selon_Aristote" + end + before do stub_request(:get, "https://fr.wikipedia.org/wiki/Th%C3%A9ologie").to_return( status: 200, body: onebox_response("wikipedia_url_encoded"), diff --git a/spec/support/onebox_helpers.rb b/spec/support/onebox_helpers.rb index 9d8955e64fc..9c3bed2b497 100644 --- a/spec/support/onebox_helpers.rb +++ b/spec/support/onebox_helpers.rb @@ -12,18 +12,16 @@ module OneboxHelpers end RSpec.shared_context "with engines" do - before do - fixture = defined?(@onebox_fixture) ? @onebox_fixture : described_class.onebox_name - stub_request(:get, defined?(@uri) ? @uri : @link).to_return( - status: 200, - body: onebox_response(fixture), - ) - end - let(:onebox) { described_class.new(link) } let(:html) { onebox.to_html } let(:data) { onebox.send(:data).deep_symbolize_keys } let(:link) { @link } + let(:uri) { defined?(@uri) ? @uri : link } + + before do + fixture = defined?(@onebox_fixture) ? @onebox_fixture : described_class.onebox_name + stub_request(:get, uri).to_return(status: 200, body: onebox_response(fixture)) + end end RSpec.shared_examples_for "an engine" do