TopicEmbed.import 的一个 spec 在 Ruby 3 下失败

yahonda · 2021 年10 月 6 日 14:31

我想获得关于 Ruby 3 下 TopicEmbed.import 规格测试失败的帮助。

复现步骤

cd discourse
bundle exec rspec ./spec/models/topic_embed_spec.rb:154

预期行为

它应该成功完成。

Ruby 3.0.2 下的实际行为

$ bundle exec rspec ./spec/models/topic_embed_spec.rb:154
Run options: include {:locations=>{"./spec/models/topic_embed_spec.rb"=>[154]}}

Randomized with seed 41701
F

Failures:

  1) TopicEmbed.import embedded content truncation keeps everything in the imported post when truncation is disabled
     Failure/Error: expect(post.raw).to include(long_content)

       expected "<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...for the original entry at <a href=\"http://eviltrout.com/123\">http://eviltrout.com/123</a></small>" to include "<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...for the original entry at <a href='http://eviltrout.com/123'>http://eviltrout.com/123</a></small>\n"
       Diff:
       @@ -1,4 +1,7 @@
       -<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa</p>\n<p>more</p>\n<hr>\n<small>This is a companion discussion topic for the original entry at <a href='http://eviltrout.com/123'>http://eviltrout.com/123</a></small>\n
       +<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa</p>
       +<p>more</p>
       +<hr>
       +<small>This is a companion discussion topic for the original entry at <a href="http://eviltrout.com/123">http://eviltrout.com/123</a></small>

     # ./spec/models/topic_embed_spec.rb:158:in `block (4 levels) in <top (required)>'
     # ./spec/rails_helper.rb:279:in `block (2 levels) in <top (required)>'

Finished in 2.48 seconds (files took 2.73 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/models/topic_embed_spec.rb:154 # TopicEmbed.import embedded content truncation keeps everything in the imported post when truncation is disabled

Randomized with seed 41701

$

Ruby 2.7.4 下的实际行为

$ ruby -v
ruby 2.7.4p191 (2021-07-07 revision a21a3b7d23) [x86_64-linux]
$ bundle exec rspec ./spec/models/topic_embed_spec.rb:154
Run options: include {:locations=>{"./spec/models/topic_embed_spec.rb"=>[154]}}

Randomized with seed 4126
.

Finished in 2.51 seconds (files took 2.31 seconds to load)
1 example, 0 failures

Randomized with seed 4126

$

我目前发现的

添加了一些打印调试代码，发现 topic_embed_spec.rb 中的 long_content 被 app/models/topic_embed.rb 中的 contents 覆盖了。

打印调试代码

$ git diff
diff --git a/app/models/topic_embed.rb b/app/models/topic_embed.rb
index 1e9f06a322..ab407d8005 100644
--- a/app/models/topic_embed.rb
+++ b/app/models/topic_embed.rb
@@ -28,6 +28,7 @@ class TopicEmbed < ActiveRecord::Base

   # Import an article from a source (RSS/Atom/Other)
   def self.import(user, url, title, contents, category_id: nil, cook_method: nil, tags: nil)
+    p "contents.length at the begining of self.import: #{contents.length}"
     return unless url =~ /^https?\:\//

     if SiteSetting.embed_truncate && cook_method.nil?
@@ -102,6 +103,7 @@ class TopicEmbed < ActiveRecord::Base
       end
     end

+    p "contents.length at the end of self.import: #{contents.length}"
     post
   end

diff --git a/spec/models/topic_embed_spec.rb b/spec/models/topic_embed_spec.rb
index 8659eb7d56..1266441da7 100644
--- a/spec/models/topic_embed_spec.rb
+++ b/spec/models/topic_embed_spec.rb
@@ -153,7 +153,9 @@ describe TopicEmbed do

       it 'keeps everything in the imported post when truncation is disabled' do
         SiteSetting.embed_truncate = false
+        p "long_content.length before calling TopicEmbed.import: #{long_content.length}"
         post = TopicEmbed.import(user, url, title, long_content)
+        p "long_content.length after calling TopicEmbed.import: #{long_content.length}"

         expect(post.raw).to include(long_content)
       end
$

Ruby 3.0.2 下的结果

$ bundle exec rspec ./spec/models/topic_embed_spec.rb:154
Run options: include {:locations=>{"./spec/models/topic_embed_spec.rb"=>[154]}}

Randomized with seed 24988
"long_content.length before calling TopicEmbed.import: 119"
"contents.length at the begining of self.import: 119"
"contents.length at the end of self.import: 267"
"long_content.length after calling TopicEmbed.import: 267"
F

Failures:

  1) TopicEmbed.import embedded content truncation keeps everything in the imported post when truncation is disabled
     Failure/Error: expect(post.raw).to include(long_content)

       expected "<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...for the original entry at <a href=\"http://eviltrout.com/123\">http://eviltrout.com/123</a></small>" to include "<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...for the original entry at <a href='http://eviltrout.com/123'>http://eviltrout.com/123</a></small>\n"
       Diff:
       @@ -1,4 +1,7 @@
       -<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa</p>\n<p>more</p>\n<hr>\n<small>This is a companion discussion topic for the original entry at <a href='http://eviltrout.com/123'>http://eviltrout.com/123</a></small>\n
       +<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa</p>
       +<p>more</p>
       +<hr>
       +<small>This is a companion discussion topic for the original entry at <a href="http://eviltrout.com/123">http://eviltrout.com/123</a></small>

     # ./spec/models/topic_embed_spec.rb:160:in `block (4 levels) in <top (required)>'
     # ./spec/rails_helper.rb:279:in `block (2 levels) in <top (required)>'

Finished in 2.55 seconds (files took 3.4 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/models/topic_embed_spec.rb:154 # TopicEmbed.import embedded content truncation keeps everything in the imported post when truncation is disabled

Randomized with seed 24988

$

Ruby 2.7.4 下的结果

$ bundle exec rspec ./spec/models/topic_embed_spec.rb:154
Run options: include {:locations=>{"./spec/models/topic_embed_spec.rb"=>[154]}}

Randomized with seed 1802
"long_content.length before calling TopicEmbed.import: 119"
"contents.length at the begining of self.import: 119"
"contents.length at the end of self.import: 267"
"long_content.length after calling TopicEmbed.import: 119"
.

Finished in 2.56 seconds (files took 3.06 seconds to load)
1 example, 0 failures

Randomized with seed 1802

$

yahonda · 2021 年10 月 8 日 06:00

我知道这个测试用例为什么仅在 Ruby 3.0.2 中失败。

自 Ruby 3 起，即使启用了 frozen-string-literal 魔术注释，插值字符串也不会被冻结。

因此，以下是可能的修复方法。

使用 contents.dup 代替 +contents

github.com/discourse/discourse

app/models/topic_embed.rb

016524e24


      
          end
          
          # Import an article from a source (RSS/Atom/Other)
          def self.import(user, url, title, contents, category_id: nil, cook_method: nil, tags: nil)
            return unless url =~ /^https?\:\/\//
          
            if SiteSetting.embed_truncate && cook_method.nil?
              contents = first_paragraph_from(contents)
            end
            contents ||= ''
            contents = +contents << imported_from_html(url)
          
            url = normalize_url(url)
          
            embed = TopicEmbed.find_by("lower(embed_url) = ?", url)
            content_sha1 = Digest::SHA1.hexdigest(contents)
            post = nil
          
            # If there is no embed, create a topic, post and the embed.
            if embed.blank?
              Topic.transaction do

$ git diff
diff --git a/app/models/topic_embed.rb b/app/models/topic_embed.rb
index 1e9f06a322..502409d3c1 100644
--- a/app/models/topic_embed.rb
+++ b/app/models/topic_embed.rb
@@ -34,7 +34,7 @@ class TopicEmbed < ActiveRecord::Base
       contents = first_paragraph_from(contents)
     end
     contents ||= ''
-    contents = +contents << imported_from_html(url)
+    contents = contents.dup << imported_from_html(url)

     url = normalize_url(url)

$

冻结插值的 long_content

github.com/discourse/discourse

spec/models/topic_embed_spec.rb

016524e24


      
              expect(post).to be_present
          
              # It uses regular rendering
              expect(post.cook_method).to eq(Post.cook_methods[:regular])
            end
          end
          
          describe 'embedded content truncation' do
            MAX_LENGTH_BEFORE_TRUNCATION = 100
          
            let(:long_content) { "<p>#{'a' * MAX_LENGTH_BEFORE_TRUNCATION}</p>\n<p>more</p>" }
          
            it 'truncates the imported post when truncation is enabled' do
              SiteSetting.embed_truncate = true
              post = TopicEmbed.import(user, url, title, long_content)
          
              expect(post.raw).not_to include(long_content)
            end
          
            it 'keeps everything in the imported post when truncation is disabled' do
              SiteSetting.embed_truncate = false

diff --git a/spec/models/topic_embed_spec.rb b/spec/models/topic_embed_spec.rb
index 8659eb7d56..a25351a586 100644
--- a/spec/models/topic_embed_spec.rb
+++ b/spec/models/topic_embed_spec.rb
@@ -142,7 +142,7 @@ describe TopicEmbed do
     describe 'embedded content truncation' do
       MAX_LENGTH_BEFORE_TRUNCATION = 100

-      let(:long_content) { "<p>#{'a' * MAX_LENGTH_BEFORE_TRUNCATION}</p>\n<p>more</p>" }
+      let(:long_content) { "<p>#{'a' * MAX_LENGTH_BEFORE_TRUNCATION}</p>\n<p>more</p>".freeze }

       it 'truncates the imported post when truncation is enabled' do
         SiteSetting.embed_truncate = true
$

我倾向于提交一个拉取请求来使用 contents.dup，并发现该想法曾被实现但随后通过 Revert "FIX: Use #dup instead of #+@ since content could be an instan… · discourse/discourse@d01c938 · GitHub 被回滚。

我没有找到关于此次回滚的其他背景信息，希望听到贡献者/维护者的意见。

yahonda · 2021 年10 月 10 日 13:23

已打开 FIX: Ruby 3 does not freeze interpolated string by yahonda · Pull Request #14567 · discourse/discourse · GitHub

话题		回复	浏览量
Embed poll_feed fail with UTF8 title Bug	6	1574	2022 年7 月 21 日
phpBB 3 Importer (old) Feature	90	17302	2014 年10 月 8 日
TopicEmbed.embed_url length limit 255 characters Bug	3	991	2015 年4 月 21 日
Embedding comments widget on localhost causes FrozenError Support	1	457	2020 年5 月 16 日
"Show Full Post" button doesn't work in subfolder installations Support embedding , subfolder	33	506	2026 年1 月 2 日