@pablobm commented on this pull request.


> +                          .uniq
+                          .filter_map do |target|
+      r = RichText.new("text", "foo #{target} bar")
+      links = Nokogiri::HTML::DocumentFragment.parse(r.to_html).css("a")
+      target if links.empty?
+    end
+    key_links = WIKI_PAGES.values
+                          .flat_map { |l| l["key"]&.keys }
+                          .filter { |key| key && !%r{\*:|/|\*$}.match?(key) }
+                          .map { |l| "#{l}=*" }
+                          .uniq
+                          .filter_map do |target|
+      r = RichText.new("text", "foo #{target} bar")
+      links = Nokogiri::HTML::DocumentFragment.parse(r.to_html).css("a")
+      target if links.empty?
+    end

I understand you are being thorough with these tests, but I have two problems 
with this approach:
1. It's not readable. There's too much transformation going on to easily see 
exactly what we are testing.
2. It's a long test to run, clocking at over 15s in my computer and causing the 
fan to kick off.

Instead I would test a handful of hand-picked examples, so that it's clear what 
it does and doesn't spend too much effort on it.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6612#pullrequestreview-3593459082
You are receiving this because you are subscribed to this thread.

Message ID: 
<openstreetmap/openstreetmap-website/pull/6612/review/[email protected]>
_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev

Reply via email to