mirror of
https://github.com/discourse/discourse.git
synced 2024-12-02 16:04:55 +08:00
916495e0a1
What is the problem? In the test environement, we were calling `SiteSetting.setting` directly to introduce new site settings. However, this leads to changes in state of the SiteSettings hash that is stored in memory as test runs. Changing or leaking states when running tests is one of the major contributors of test flakiness. An example of how this resulted in test flakiness is our `spec/integrity/i18n_spec.rb` spec file which had a test case that would fail because a new "plugin_setting" site setting was registered in another test case but the site setting did not have translations for the site setting set. What is the fix? There are a couple of changes being introduced in this commit: 1. Make `SiteSetting.setting` a private method as it is not safe to be exposed as a public method of the `SiteSetting` class 2. Change test cases to use existing site settings in Discourse instead of creating custom site settings. Existing site settings are not removed often so we don't really need to dynamically add new site settings in test cases. Even if the site settings being used in test cases are removed, updating the test cases to rely on other site settings is a very easy change. 3. Set up a plugin instance in the test environment as a "fixture" instead of having each test create its own plugin instance.
451 lines
13 KiB
Ruby
451 lines
13 KiB
Ruby
# frozen_string_literal: true
|
|
|
|
RSpec.describe StaticController do
|
|
fab!(:upload) { Fabricate(:upload) }
|
|
|
|
describe "#favicon" do
|
|
let(:filename) { "smallest.png" }
|
|
let(:file) { file_from_fixtures(filename) }
|
|
|
|
let(:upload) { UploadCreator.new(file, filename).create_for(Discourse.system_user.id) }
|
|
|
|
after { Discourse.redis.scan_each(match: "memoize_*").each { |key| Discourse.redis.del(key) } }
|
|
|
|
context "with local store" do
|
|
it "returns the default favicon if favicon has not been configured" do
|
|
get "/favicon/proxied"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.media_type).to eq("image/png")
|
|
expect(response.body.bytesize).to eq(SiteIconManager.favicon.filesize)
|
|
end
|
|
|
|
it "returns the configured favicon" do
|
|
SiteSetting.favicon = upload
|
|
|
|
get "/favicon/proxied"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.media_type).to eq("image/png")
|
|
expect(response.body.bytesize).to eq(upload.filesize)
|
|
end
|
|
end
|
|
|
|
context "with external store" do
|
|
let(:upload) do
|
|
Upload.create!(
|
|
url: "//s3-upload-bucket.s3-us-east-1.amazonaws.com/somewhere/a.png",
|
|
original_filename: filename,
|
|
filesize: file.size,
|
|
user_id: Discourse.system_user.id,
|
|
)
|
|
end
|
|
|
|
before { setup_s3 }
|
|
|
|
it "can proxy a favicon correctly" do
|
|
SiteSetting.favicon = upload
|
|
|
|
stub_request(:get, "https:/#{upload.url}").to_return(status: 200, body: file)
|
|
|
|
get "/favicon/proxied"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.media_type).to eq("image/png")
|
|
expect(response.body.bytesize).to eq(upload.filesize)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "#brotli_asset" do
|
|
it "returns a non brotli encoded 404 if asset is missing" do
|
|
get "/brotli_asset/missing.js"
|
|
|
|
expect(response.status).to eq(404)
|
|
expect(response.headers["Content-Encoding"]).not_to eq("br")
|
|
expect(response.headers["Cache-Control"]).to match(/max-age=1/)
|
|
end
|
|
|
|
it "can handle fallback brotli assets" do
|
|
begin
|
|
assets_path = Rails.root.join("tmp/backup_assets")
|
|
|
|
GlobalSetting.stubs(:fallback_assets_path).returns(assets_path.to_s)
|
|
|
|
FileUtils.mkdir_p(assets_path)
|
|
|
|
file_path = assets_path.join("test.js.br")
|
|
File.write(file_path, "fake brotli file")
|
|
|
|
get "/brotli_asset/test.js"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.headers["Cache-Control"]).to match(/public/)
|
|
ensure
|
|
File.delete(file_path)
|
|
end
|
|
end
|
|
|
|
it "has correct headers for brotli assets" do
|
|
begin
|
|
assets_path = Rails.root.join("public/assets")
|
|
|
|
FileUtils.mkdir_p(assets_path)
|
|
|
|
file_path = assets_path.join("test.js.br")
|
|
File.write(file_path, "fake brotli file")
|
|
|
|
get "/brotli_asset/test.js"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.headers["Cache-Control"]).to match(/public/)
|
|
ensure
|
|
File.delete(file_path)
|
|
end
|
|
end
|
|
|
|
it "has correct cors headers for brotli assets" do
|
|
begin
|
|
assets_path = Rails.root.join("public/assets")
|
|
|
|
FileUtils.mkdir_p(assets_path)
|
|
|
|
file_path = assets_path.join("test.js.br")
|
|
File.write(file_path, "fake brotli file")
|
|
GlobalSetting.stubs(:cdn_url).returns("https://www.example.com/")
|
|
|
|
get "/brotli_asset/test.js"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.headers["Access-Control-Allow-Origin"]).to match("*")
|
|
ensure
|
|
File.delete(file_path)
|
|
end
|
|
end
|
|
|
|
it "can serve sourcemaps on adjacent paths" do
|
|
assets_path = Rails.root.join("public/assets")
|
|
|
|
FileUtils.mkdir_p(assets_path)
|
|
|
|
file_path = assets_path.join("test.map")
|
|
File.write(file_path, "fake source map")
|
|
GlobalSetting.stubs(:cdn_url).returns("https://www.example.com/")
|
|
|
|
get "/brotli_asset/test.map"
|
|
|
|
expect(response.status).to eq(200)
|
|
ensure
|
|
File.delete(file_path)
|
|
end
|
|
end
|
|
|
|
describe "#cdn_asset" do
|
|
let (:site) {
|
|
RailsMultisite::ConnectionManagement.current_db
|
|
}
|
|
|
|
it "can serve assets" do
|
|
begin
|
|
assets_path = Rails.root.join("public/assets")
|
|
|
|
FileUtils.mkdir_p(assets_path)
|
|
|
|
file_path = assets_path.join("test.js.br")
|
|
File.write(file_path, "fake brotli file")
|
|
|
|
get "/cdn_asset/#{site}/test.js.br"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.headers["Cache-Control"]).to match(/public/)
|
|
ensure
|
|
File.delete(file_path)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "#show" do
|
|
before do
|
|
post = create_post
|
|
SiteSetting.tos_topic_id = post.topic.id
|
|
SiteSetting.guidelines_topic_id = post.topic.id
|
|
SiteSetting.privacy_topic_id = post.topic.id
|
|
end
|
|
|
|
context "with a static file that's present" do
|
|
it "should return the right response for /faq" do
|
|
get "/faq"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.body).to include(I18n.t("js.faq"))
|
|
expect(response.body).to include("<title>FAQ - Discourse</title>")
|
|
end
|
|
end
|
|
|
|
[
|
|
["tos", :tos_url, I18n.t("js.tos")],
|
|
["privacy", :privacy_policy_url, I18n.t("js.privacy")],
|
|
].each do |id, setting_name, text|
|
|
context "with #{id}" do
|
|
context "when #{setting_name} site setting is NOT set" do
|
|
it "renders the #{id} page" do
|
|
get "/#{id}"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.body).to include(text)
|
|
end
|
|
end
|
|
|
|
context "when #{setting_name} site setting is set" do
|
|
before { SiteSetting.set(setting_name, "http://example.com/page") }
|
|
|
|
it "redirects to the #{setting_name}" do
|
|
get "/#{id}"
|
|
|
|
expect(response).to redirect_to("http://example.com/page")
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
context "with a missing file" do
|
|
it "should respond 404" do
|
|
get "/static/does-not-exist"
|
|
expect(response.status).to eq(404)
|
|
end
|
|
|
|
context "with modal pages" do
|
|
it "should return the right response for /signup" do
|
|
get "/signup"
|
|
expect(response.status).to eq(200)
|
|
end
|
|
|
|
it "should return the right response for /password-reset" do
|
|
get "/password-reset"
|
|
expect(response.status).to eq(200)
|
|
end
|
|
end
|
|
end
|
|
|
|
it "should redirect to / when logged in and path is /login" do
|
|
sign_in(Fabricate(:user))
|
|
get "/login"
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
|
|
it "should display the login template when login is required" do
|
|
SiteSetting.login_required = true
|
|
|
|
get "/login"
|
|
|
|
expect(response.status).to eq(200)
|
|
|
|
expect(response.body).to include(
|
|
PrettyText.cook(I18n.t("login_required.welcome_message", title: SiteSetting.title)),
|
|
)
|
|
end
|
|
|
|
context "when login_required is enabled" do
|
|
before { SiteSetting.login_required = true }
|
|
|
|
%w[faq guidelines rules conduct].each do |page_name|
|
|
it "#{page_name} page redirects to login page for anon" do
|
|
get "/#{page_name}"
|
|
expect(response).to redirect_to "/login"
|
|
end
|
|
|
|
it "#{page_name} page redirects to login page for anon" do
|
|
get "/#{page_name}"
|
|
expect(response).to redirect_to "/login"
|
|
end
|
|
|
|
it "#{page_name} page loads for logged in user" do
|
|
sign_in(Fabricate(:user))
|
|
|
|
get "/#{page_name}"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.body).to include(I18n.t("js.guidelines"))
|
|
end
|
|
end
|
|
end
|
|
|
|
context "with crawler view" do
|
|
it "should include correct title" do
|
|
get "/faq", headers: { "HTTP_USER_AGENT" => "Googlebot" }
|
|
expect(response.status).to eq(200)
|
|
expect(response.body).to include("<title>FAQ - Discourse</title>")
|
|
end
|
|
end
|
|
|
|
context "with plugin api extensions" do
|
|
after do
|
|
Rails.application.reload_routes!
|
|
StaticController::CUSTOM_PAGES.clear
|
|
end
|
|
|
|
it "adds new topic-backed pages" do
|
|
routes = Proc.new { get "contact" => "static#show", :id => "contact" }
|
|
Discourse::Application.routes.send(:eval_block, routes)
|
|
|
|
topic_id = Fabricate(:post, cooked: "contact info").topic_id
|
|
SiteSetting.test_some_topic_id = topic_id
|
|
|
|
Plugin::Instance.new.add_topic_static_page("contact", topic_id: "test_some_topic_id")
|
|
|
|
get "/contact"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.body).to include("contact info")
|
|
end
|
|
|
|
it "replaces existing topic-backed pages" do
|
|
topic_id = Fabricate(:post, cooked: "Regular FAQ").topic_id
|
|
SiteSetting.test_some_topic_id = topic_id
|
|
|
|
polish_topic_id = Fabricate(:post, cooked: "Polish FAQ").topic_id
|
|
SiteSetting.test_some_other_topic_id = polish_topic_id
|
|
|
|
Plugin::Instance
|
|
.new
|
|
.add_topic_static_page("faq") do
|
|
current_user&.locale == "pl" ? "test_some_other_topic_id" : "test_some_topic_id"
|
|
end
|
|
|
|
get "/faq"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.body).to include("Regular FAQ")
|
|
|
|
sign_in(Fabricate(:user, locale: "pl"))
|
|
get "/faq"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.body).to include("Polish FAQ")
|
|
end
|
|
end
|
|
|
|
it "does not pollute SiteSetting.title (regression)" do
|
|
SiteSetting.title = "test"
|
|
SiteSetting.short_site_description = "something"
|
|
|
|
expect do
|
|
get "/login"
|
|
get "/login"
|
|
end.to_not change { SiteSetting.title }
|
|
end
|
|
end
|
|
|
|
describe "#enter" do
|
|
context "without a redirect path" do
|
|
it "redirects to the root url" do
|
|
post "/login.json"
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
end
|
|
|
|
context "with a redirect path" do
|
|
it "redirects to the redirect path" do
|
|
post "/login.json", params: { redirect: "/foo" }
|
|
expect(response).to redirect_to("/foo")
|
|
end
|
|
end
|
|
|
|
context "with a full url" do
|
|
it "redirects to the correct path" do
|
|
post "/login.json", params: { redirect: "#{Discourse.base_url}/foo" }
|
|
expect(response).to redirect_to("/foo")
|
|
end
|
|
end
|
|
|
|
context "with a redirect path with query params" do
|
|
it "redirects to the redirect path and preserves query params" do
|
|
post "/login.json", params: { redirect: "/foo?bar=1" }
|
|
expect(response).to redirect_to("/foo?bar=1")
|
|
end
|
|
end
|
|
|
|
context "with a period to force a new host" do
|
|
it "redirects to the root path" do
|
|
post "/login.json", params: { redirect: ".org/foo" }
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
end
|
|
|
|
context "with a full url to someone else" do
|
|
it "redirects to the root path" do
|
|
post "/login.json", params: { redirect: "http://eviltrout.com/foo" }
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
end
|
|
|
|
context "with an invalid URL" do
|
|
it "redirects to the root" do
|
|
post "/login.json", params: { redirect: "javascript:alert('trout')" }
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
end
|
|
|
|
context "with an array" do
|
|
it "redirects to the root" do
|
|
post "/login.json", params: { redirect: ["/foo"] }
|
|
expect(response.status).to eq(400)
|
|
json = response.parsed_body
|
|
expect(json["errors"]).to be_present
|
|
expect(json["errors"]).to include(I18n.t("invalid_params", message: "redirect"))
|
|
end
|
|
end
|
|
|
|
context "when the redirect path is the login page" do
|
|
it "redirects to the root url" do
|
|
post "/login.json", params: { redirect: login_path }
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "#service_worker_asset" do
|
|
it "works" do
|
|
get "/service-worker.js"
|
|
expect(response.status).to eq(200)
|
|
expect(response.content_type).to start_with("application/javascript")
|
|
expect(response.body).to include("workbox")
|
|
end
|
|
|
|
it "replaces sourcemap URL" do
|
|
Rails
|
|
.application
|
|
.assets_manifest
|
|
.stubs(:find_sources)
|
|
.with("service-worker.js")
|
|
.returns([<<~JS])
|
|
someFakeServiceWorkerSource();
|
|
//# sourceMappingURL=service-worker-abcde.js.map
|
|
JS
|
|
|
|
{
|
|
"/assets/service-worker.js" => "/assets/service-worker-abcde.js.map",
|
|
"/assets/service-worker.js.br" => "/assets/service-worker-abcde.js.map",
|
|
"/assets/service-worker.br.js" => "/assets/service-worker-abcde.js.map",
|
|
"/assets/service-worker.js.gz" => "/assets/service-worker-abcde.js.map",
|
|
"/assets/service-worker.gz.js" => "/assets/service-worker-abcde.js.map",
|
|
"https://example.com/assets/service-worker.js" =>
|
|
"https://example.com/assets/service-worker-abcde.js.map",
|
|
"https://example.com/subfolder/assets/service-worker.js" =>
|
|
"https://example.com/subfolder/assets/service-worker-abcde.js.map",
|
|
}.each do |asset_path, expected_map_url|
|
|
ActionController::Base
|
|
.helpers
|
|
.stubs(:asset_path)
|
|
.with("service-worker.js")
|
|
.returns(asset_path)
|
|
|
|
get "/service-worker.js"
|
|
expect(response.status).to eq(200)
|
|
expect(response.content_type).to start_with("application/javascript")
|
|
expect(response.body).to include("sourceMappingURL=#{expected_map_url}\n")
|
|
end
|
|
end
|
|
end
|
|
end
|