diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a87129ee36..ac7e9d6076 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,9 +32,23 @@ jobs: --health-retries 5 ports: - 5432:5432 + # As a service container Elasticsearch boots in parallel with the rest + # of the setup instead of as a serial step. + elasticsearch: + image: docker.elastic.co/elasticsearch/elasticsearch:8.10.0 + env: + discovery.type: single-node + xpack.security.enabled: false + ES_JAVA_OPTS: -Xms512m -Xmx512m + options: >- + --health-cmd "curl -fs http://localhost:9200/_cluster/health || exit 1" + --health-interval 10s + --health-timeout 5s + --health-retries 10 + ports: + - 9200:9200 steps: - uses: actions/checkout@v3 - - uses: nanasess/setup-chromedriver@v2 - name: Install system dependencies run: | set -eux @@ -57,12 +71,6 @@ jobs: wget --quiet --output-document - https://github.com/esimov/pigo/releases/download/v1.4.5/pigo-1.4.5-linux-amd64.tar.gz | tar --extract --gunzip --directory=/tmp/ sudo mv /tmp/pigo-1.4.5-linux-amd64/pigo /usr/local/bin/ - - name: Runs Elasticsearch - uses: elastic/elastic-github-actions/elasticsearch@master - with: - stack-version: 8.10.0 - security-enabled: false - - uses: actions/setup-node@v3 with: node-version: 18 diff --git a/app/models/search/connection.rb b/app/models/search/connection.rb index 16ac6c47c1..60e39f276f 100644 --- a/app/models/search/connection.rb +++ b/app/models/search/connection.rb @@ -188,7 +188,7 @@ def update_alias(alias_name:, old_indexes:, new_index:) end def reindex(index, mappings:, &block) - new_index = "#{index}-#{Time.now.to_i}" + new_index = "#{index}-#{Time.now.strftime("%s%L")}" request(:put, new_index, json: mappings) begin yield(new_index) @@ -196,7 +196,10 @@ def reindex(index, mappings:, &block) delete_index(new_index) raise end - old_indexes = get_indexes_from_alias(index) + # If a reindex reuses a still-aliased name (two runs in the same clock + # second), treating it as an old index would delete it right after the + # alias swap points at it. + old_indexes = get_indexes_from_alias(index) - [new_index] update_alias(alias_name: index, old_indexes: old_indexes, new_index: new_index) old_indexes.each { delete_index(_1) } end diff --git a/config/initializers/bcrypt.rb b/config/initializers/bcrypt.rb index f2b8de5d2a..f2f568e8be 100644 --- a/config/initializers/bcrypt.rb +++ b/config/initializers/bcrypt.rb @@ -1 +1,7 @@ -BCrypt::Engine.cost = ENV["BCRYPT_COST"] ? ENV["BCRYPT_COST"].to_i : 12 +BCrypt::Engine.cost = if ENV["BCRYPT_COST"] + ENV["BCRYPT_COST"].to_i +elsif Rails.env.test? + BCrypt::Engine::MIN_COST +else + 12 +end diff --git a/config/initializers/elasticsearch.rb b/config/initializers/elasticsearch.rb index 1ba3a6f6fa..886b0f55b1 100644 --- a/config/initializers/elasticsearch.rb +++ b/config/initializers/elasticsearch.rb @@ -10,7 +10,7 @@ def configure! shared_settings = { index: { - number_of_shards: "6", + number_of_shards: Rails.env.test? ? "1" : "6", }, analysis: { analyzer: { @@ -236,7 +236,7 @@ def setup end -unless Rails.env.production? +if Rails.env.development? ActiveSupport::Notifications.subscribe("request.search") do |name, start, finish, id, payload| Rails.logger.info(search: "request", method: payload.safe_dig(:response).request.verb, path: payload.safe_dig(:response).request.uri.to_s) json = JSON.pretty_generate(JSON.parse(payload.safe_dig(:response)&.request&.body&.source)) rescue nil diff --git a/config/routes.rb b/config/routes.rb index c05c118279..a3a3041904 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -147,7 +147,7 @@ mount Sidekiq::Web, at: "/sidekiq" end - constraints lambda { |request| Rails.env.development? } do + constraints lambda { |request| Rails.env.development? || Rails.env.test? } do get :auto_sign_in, to: "site#auto_sign_in" get :onboarding, to: "onboarding#show" end diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb index 91eaefba2c..647f336ae7 100644 --- a/test/application_system_test_case.rb +++ b/test/application_system_test_case.rb @@ -12,11 +12,10 @@ class ApplicationSystemTestCase < ActionDispatch::SystemTestCase parallelize(workers: 1) + # Sign in via the dev/test auto_sign_in route rather than the login form — + # same session, none of the form driving. LoginTest covers the real form. def login_as(user) - visit login_path - fill_in "Email", with: user.email - fill_in "Password", with: default_password - click_button "Sign In" + visit auto_sign_in_path(email: user.email) end def show_article_setup @@ -33,8 +32,14 @@ def show_article click_link(@entries.first.title) end - def wait_for_ajax(duration: 0.1) - sleep duration + # Polls rather than sleeping a fixed interval; most requests settle in a + # few milliseconds. + def wait_for_ajax(duration: Capybara.default_max_wait_time) + deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + duration + until finished_all_ajax_requests? + raise "timed out waiting for ajax" if Process.clock_gettime(Process::CLOCK_MONOTONIC) > deadline + sleep 0.01 + end end def finished_all_ajax_requests? diff --git a/test/controllers/api/podcasts/v1/feeds_controller_test.rb b/test/controllers/api/podcasts/v1/feeds_controller_test.rb index 3365b7571b..e14754aed0 100644 --- a/test/controllers/api/podcasts/v1/feeds_controller_test.rb +++ b/test/controllers/api/podcasts/v1/feeds_controller_test.rb @@ -37,7 +37,7 @@ class Api::Podcasts::V1::FeedsControllerTest < ApiControllerTestCase end test "show serializes every item" do - 30.times { create_entry(@feed) } + bulk_create_entries(@feed, 30) get :show, params: {id: hex_encode(@feed.feed_url)}, format: :json diff --git a/test/controllers/entries_search_controller_test.rb b/test/controllers/entries_search_controller_test.rb index b6bce7386e..7218bddafb 100644 --- a/test/controllers/entries_search_controller_test.rb +++ b/test/controllers/entries_search_controller_test.rb @@ -10,6 +10,9 @@ class EntriesSearchControllerTest < ActionController::TestCase test "should get search" do login_as @user + # A unique token: search index contents survive across tests in a run, so + # a Faker sentence can collide with a stale document from another test. + @entry.update!(title: "searchtoken #{SecureRandom.hex}") reindex_search get :search, params: {query: "\"#{@entry.title}\""}, xhr: true assert_response :success diff --git a/test/jobs/entry_deleter_test.rb b/test/jobs/entry_deleter_test.rb index 3928591e87..2cdd77d854 100644 --- a/test/jobs/entry_deleter_test.rb +++ b/test/jobs/entry_deleter_test.rb @@ -8,12 +8,7 @@ class EntryDeleterTest < ActiveSupport::TestCase @user = users(:ben) @feed = @user.feeds.first Feed.reset_counters(@feed.id, :subscriptions) - @entries = (ENV["ENTRY_LIMIT"].to_i + count.sample).times.map { - @feed.entries.create!( - content: Faker::Lorem.paragraph, - public_id: SecureRandom.hex - ) - } + @entries = bulk_create_entries(@feed, ENV["ENTRY_LIMIT"].to_i + count.sample) end test "should limit total entries" do diff --git a/test/jobs/feed_crawler/downloader_test.rb b/test/jobs/feed_crawler/downloader_test.rb index fb0e829678..4c1778c2a5 100644 --- a/test/jobs/feed_crawler/downloader_test.rb +++ b/test/jobs/feed_crawler/downloader_test.rb @@ -234,6 +234,10 @@ def test_should_follow_redirects end def test_should_save_redirected_to + swap_const(RedirectCache, :PERSIST_AFTER, 5) { save_redirected_to } + end + + def save_redirected_to last_url = URI.join(@feed.feed_url, "/final") response = { diff --git a/test/jobs/feed_crawler/redirect_cache_test.rb b/test/jobs/feed_crawler/redirect_cache_test.rb index cc0d804c8e..44ad0c7ad5 100644 --- a/test/jobs/feed_crawler/redirect_cache_test.rb +++ b/test/jobs/feed_crawler/redirect_cache_test.rb @@ -8,6 +8,10 @@ def setup end def test_should_collapse_stable_redirects + swap_const(RedirectCache, :PERSIST_AFTER, 5) { collapse_stable_redirects } + end + + def collapse_stable_redirects feed_id = 2 redirect1 = Feedkit::Redirect.new(status: 301, from: "http://example.com", to: "http://example.com/second") diff --git a/test/jobs/image_crawler/timer_test.rb b/test/jobs/image_crawler/timer_test.rb index 7288aaf489..16b03e92af 100644 --- a/test/jobs/image_crawler/timer_test.rb +++ b/test/jobs/image_crawler/timer_test.rb @@ -10,7 +10,7 @@ def test_should_time_out assert timer.expired? elapsed = time + 0.01 - assert [elapsed].include?(timer.elapsed), "Around #{elapsed}s should have elapsed." + assert_in_delta elapsed, timer.elapsed, 0.05, "Around #{elapsed}s should have elapsed." end end end \ No newline at end of file diff --git a/test/jobs/search/search_server_setup_test.rb b/test/jobs/search/search_server_setup_test.rb index c82fedce50..c562d394ff 100644 --- a/test/jobs/search/search_server_setup_test.rb +++ b/test/jobs/search/search_server_setup_test.rb @@ -7,12 +7,7 @@ class SearchServerSetupTest < ActiveSupport::TestCase clear_search @user = users(:ben) feed = @user.feeds.first - @entries = (1..10).to_a.sample.times.map { - feed.entries.create!( - content: Faker::Lorem.paragraph, - public_id: SecureRandom.hex - ) - } + @entries = bulk_create_entries(feed, (1..10).to_a.sample) end test "should bulk index entries in elasticsearch" do diff --git a/test/jobs/unread_limiter_test.rb b/test/jobs/unread_limiter_test.rb index 06d1e5e3fb..8254da1264 100644 --- a/test/jobs/unread_limiter_test.rb +++ b/test/jobs/unread_limiter_test.rb @@ -8,13 +8,7 @@ class UnreadLimiterTest < ActiveSupport::TestCase @user = users(:ben) @feed = @user.feeds.first Feed.reset_counters(@feed.id, :subscriptions) - @entries = @count.times.map { - @feed.entries.create!( - content: Faker::Lorem.paragraph, - public_id: SecureRandom.hex, - published: Time.now - ) - } + @entries = bulk_create_entries(@feed, @count, users: @user) end test "should remove UnreadEntries" do diff --git a/test/models/search/connection_test.rb b/test/models/search/connection_test.rb index 29176f736b..e9193e1dcc 100644 --- a/test/models/search/connection_test.rb +++ b/test/models/search/connection_test.rb @@ -15,9 +15,8 @@ class ConnectionTest < ActiveSupport::TestCase # search matching more than the cap silently returned only the first N ids # -- which is why "mark all search results as read" left the rest unread. test "all_matches returns every id when the reported total is capped" do - entries = 25.times.map { create_entry(@feed).tap { _1.update!(title: "#{@token} #{SecureRandom.hex}") } } - entries.each { SearchIndexStore.new.perform("Entry", _1.id) } - Search.client { _1.refresh } + entries = bulk_create_entries(@feed, 25, attributes: {title: "#{@token} #{SecureRandom.hex}"}) + index_entries(entries) query = { track_total_hits: 10, diff --git a/test/models/source/json_feed_test.rb b/test/models/source/json_feed_test.rb index b4c31c478d..6fb0221b2c 100644 --- a/test/models/source/json_feed_test.rb +++ b/test/models/source/json_feed_test.rb @@ -3,7 +3,7 @@ class JsonFeedTest < ActiveSupport::TestCase test "should create feed" do url = "https://example.com/feed.json" - stub_request_file("feed.json", url) + stub_request_file("feed_single.json", url) response = Feedkit::Request.download(url) assert_difference "Feed.count", +1 do Source::Xml.find(response) diff --git a/test/models/source/known_pattern_test.rb b/test/models/source/known_pattern_test.rb index da7710cef5..45b20d2889 100644 --- a/test/models/source/known_pattern_test.rb +++ b/test/models/source/known_pattern_test.rb @@ -16,7 +16,9 @@ class KnownPatternTest < ActiveSupport::TestCase } urls.each do |known_pattern, destination| stub_request_file("index.html", known_pattern) - stub_request_file("atom.xml", destination) + # The mapping is what matters here; a single-entry feed keeps the ten + # resulting feed creations cheap. + stub_request_file("atom_single.xml", destination) response = Feedkit::Request.download(known_pattern) assert_difference "Feed.count", +1 do Source::KnownPattern.find(response) diff --git a/test/models/source/meta_links_test.rb b/test/models/source/meta_links_test.rb index b72536b710..e7c6d8f501 100644 --- a/test/models/source/meta_links_test.rb +++ b/test/models/source/meta_links_test.rb @@ -46,7 +46,7 @@ class MetaLinksTest < ActiveSupport::TestCase feed_url = "/feeds/json/" stub_request(:get, url) .to_return(body: %()) - stub_request_file("feed.json", url + feed_url) + stub_request_file("feed_single.json", url + feed_url) response = Feedkit::Request.download(url) assert_difference "Feed.count", +1 do Source::MetaLinks.find(response) diff --git a/test/support/factory_helper.rb b/test/support/factory_helper.rb index 8d7c40fdaa..c0b7fd91e6 100644 --- a/test/support/factory_helper.rb +++ b/test/support/factory_helper.rb @@ -9,14 +9,67 @@ def create_feeds(users, count = 3) users.map do |user| user.subscriptions.where(feed: feed).first_or_create end - entry = create_entry(feed) - Search::SearchIndexStore.new.perform("Entry", entry.id) end } - Search.client { _1.refresh } + entries = feeds.flat_map { bulk_create_entries(_1, 1, users: users) } + index_entries(entries) feeds end + # One bulk request rather than Search::SearchIndexStore per entry — the + # store job also percolates each document against the actions index (three + # extra requests per entry), which no create_feeds caller depends on. Tests + # that assert percolation invoke SearchIndexStore themselves. + def index_entries(entries) + records = entries.map do |entry| + Search::BulkRecord.new( + action: "index", + index: Search.index_name(Entry.table_name), + id: entry.id, + document: entry.search_data + ) + end + Search.client do |client| + client.bulk(records) + client.refresh + end + end + + # Bulk-inserts bare entries in one statement, skipping Entry's per-create + # callbacks — an order of magnitude cheaper than create! for tests that + # only need rows to exist. Pass users: to also mark the entries unread for + # them, the way the mark_as_unread callback would have. Tests that depend + # on other callback side effects should use create_entry. + def bulk_create_entries(feed, count, users: [], attributes: {}) + now = Time.now + rows = count.times.map do |index| + { + feed_id: feed.id, + title: Faker::Lorem.sentence, + url: Faker::Internet.url, + author: SecureRandom.hex, + content: Faker::Lorem.paragraph, + public_id: SecureRandom.hex, + entry_id: SecureRandom.hex, + data: {enclosure_url: Faker::Internet.url}, + published: now + index, + created_at: now, + updated_at: now + }.merge(attributes) + end + ids = Entry.insert_all(rows, returning: [:id]).rows.flatten + entries = Entry.where(id: ids).order(:id).to_a + + unreads = [*users].flat_map { |user| + entries.map { + UnreadEntry.new(user_id: user.id, feed_id: feed.id, entry_id: _1.id, published: _1.published, entry_created_at: _1.created_at) + } + } + UnreadEntry.import(unreads, validate: false, on_duplicate_key_ignore: true) if unreads.present? + + entries + end + def create_entry(feed) feed.entries.create!( title: Faker::Lorem.sentence, diff --git a/test/support/www/atom_single.xml b/test/support/www/atom_single.xml new file mode 100644 index 0000000000..693ad7b161 --- /dev/null +++ b/test/support/www/atom_single.xml @@ -0,0 +1,53 @@ + + + Feedbin + A fast, simple RSS feed reader that delivers a great reading experience. + + + + 2016-06-03T13:25:02-05:00 + https://feedbin.com + + Feedbin + support@feedbin.com + + + + + Subscribe to Email Newsletters in Feedbin + + Ben Ubois + + + + 2016-02-03T15:37:25-06:00 + + + 2016-02-03T00:00:00-06:00 + + /2016/02/03/subscribe-to-email-newsletters-in-feedbin + You can now receive email newsletters in Feedbin. Newsletter Subscriptions To use this feature, go to the settings page and find your secret Feedbin email address. Use this email address whenever you sign up for an email newsletter. Anything sent to it will show up as a feed in Feedbin,... + <p>You can now receive email newsletters in Feedbin.</p> + +<figure> + <a href="https://dhy5vgj5baket.cloudfront.net/assets-site/blog/2016-02-03/email-large-ce9a0b27d895b4cb89429bb1d5773e0d1a394ef225dab5c0ced8c4419ed05bae.png"><img src="https://dhy5vgj5baket.cloudfront.net/assets-site/blog/2016-02-03/email-large-ce9a0b27d895b4cb89429bb1d5773e0d1a394ef225dab5c0ced8c4419ed05bae.png" /></a> + <figcaption>Newsletter Subscriptions</figcaption> +</figure> + +<p>To use this feature, go to the <a href="https://feedbin.com/settings">settings</a> page and find your secret Feedbin email address. Use this email address whenever you sign up for an email newsletter. Anything sent to it will show up as a feed in Feedbin, grouped by sender.</p> + +<p>Reading email in an email app feels like work to me. However, there’s a certain class of email that I <em>want</em> to enjoy reading, and Feedbin is where I go when I want to read for pleasure.</p> + +<p>For example, many great websites offer subscription content, usually with an email newsletter component. Not only do I enjoy the premium content from these sites, but I believe this a great way forward for people to support writers. Personally I have paid subscriptions to four of these including:</p> + +<ul> + <li><a href="https://www.macstories.net">MacStories</a></li> + <li><a href="http://www.aboveavalon.com">Above Avalon</a></li> + <li><a href="https://stratechery.com">Stratechery</a></li> + <li><a href="https://sixcolors.com">Six Colors</a></li> +</ul> + +<p>This feature is also great for mailing lists and product announcement emails and since it’s just a regular feed, it will sync with your favorite native app as well.</p> + + + diff --git a/test/support/www/feed_single.json b/test/support/www/feed_single.json new file mode 100644 index 0000000000..01913eacba --- /dev/null +++ b/test/support/www/feed_single.json @@ -0,0 +1,27 @@ +{ + "version": "https://jsonfeed.org/version/1", + "title": "The Record", + "description": "The stories you should know about the Mac and Cocoa developer community. Hosted by Brent Simmons and Chris Parrish.", + "home_page_url": "http://therecord.co/", + "feed_url": "http://therecord.co/feed.json", + "user_comment": "This feed allows you to read the posts from this site in any feed reader that supports the JSON Feed format. To add this feed to your reader, copy the following URL — http://therecord.co/feed.json — and add it your reader.", + "author": { + "name": "Brent Simmons and Chris Parrish" + }, + "items": [ + { + "id": "http://therecord.co/2014/05/29/brent_simmons", + "url": "http://therecord.co/2014/05/29/brent_simmons", + "title": "Special #2 - Brent Simmons", + "content_html": "

This episode was recorded 26 May 2014 live and in person at Brent’s office in sunny, lovely Ballard.

\n\n

You can download the m4a file or subscribe in iTunes. (Or subscribe to the podcast feed.)

\n\n

Brent has worked at UserLand Software and NewsGator and as an indie at his company Ranchero Software. These days he’s one-third of Q Branch, where he writes Vesper. He is also the co-host of this podcast.

\n\n

This episode is sponsored by Tagcaster. Tagcaster is not just another podcast client — it solves the age-old problem of linking to specific parts of a podcast. You can make clips — short audio excerpts — and share them and link to them. After all these years, that problem is finally solved.

\n\n\n\n\n

This episode is also sponsored by Igloo. Igloo is an intranet you’ll actually like, with shared calendars, microblogs, file-sharing, social networking, and more. It’s free for up 10 users — give it a try for your company or your team today.

\n\n\n\n\n

This episode is also sponsored by Hover. Hover makes domain name management easy. And it’s a snap to transfer domains from other registrars using their valet service. Get 10% off your first purchase with the promotional code MANILA. (Manila was the name of the blogging system worked on at UserLand.) Take a look.

\n\n\n

Things we mention, more or less in order of appearance:

\n\n", + "date_published": "2014-05-29T05:00:00-07:00", + "attachments": [ + { + "url": "http://therecord.co/downloads/TheRecord-sp1e2-BrentSimmons.m4a", + "mime_type": "audio/x-m4a", + "size_in_bytes": 75344101 + } + ] + } + ] +} \ No newline at end of file diff --git a/test/system/article_test.rb b/test/system/article_test.rb index d3a60f6ffd..4c1ec4d270 100644 --- a/test/system/article_test.rb +++ b/test/system/article_test.rb @@ -51,7 +51,6 @@ class ArticleTest < ApplicationSystemTestCase click_link(@entries.first.title) - sleep 1 wait_for_ajax assert_text "Samsung Galaxy Note 9 Impressions: Underrated!" @@ -72,7 +71,6 @@ class ArticleTest < ApplicationSystemTestCase click_link(@entries.first.title) - sleep 1 wait_for_ajax url = RemoteFile.signed_url("https://pbs.twimg.com/profile_images/659486593649012736/-TGFT8rs.png") diff --git a/test/system/login_test.rb b/test/system/login_test.rb index 465aa3b028..e285286850 100644 --- a/test/system/login_test.rb +++ b/test/system/login_test.rb @@ -1,9 +1,14 @@ require "application_system_test_case" class LoginTest < ApplicationSystemTestCase + # The one place the real login form gets driven end-to-end — everywhere + # else uses the auto_sign_in shortcut. test "Login" do user = users(:ben) - login_as(user) + visit login_path + fill_in "Email", with: user.email + fill_in "Password", with: default_password + click_button "Sign In" find("[data-behavior~=show_subscribe]") end end diff --git a/test/system/newsletter_test.rb b/test/system/newsletter_test.rb index 92c8d42ce3..0a24120be9 100644 --- a/test/system/newsletter_test.rb +++ b/test/system/newsletter_test.rb @@ -14,9 +14,9 @@ class NewsletterTest < ApplicationSystemTestCase assert find("button[type=submit]").disabled? - wait_for_ajax(duration: 1) - - assert_not find("button[type=submit]").disabled? + # The availability check debounces before it fires, so wait on the button + # itself rather than on the request being in flight. + assert_selector "button[type=submit]:not([disabled])" numbers = find("[data-behavior~=token_suffix]").text() @@ -32,9 +32,14 @@ class NewsletterTest < ApplicationSystemTestCase fill_in "authentication_token[description]", with: description - wait_for_ajax(duration: 0.5) - + # The description autosaves per keystroke behind a debounce, so poll the + # record rather than the request counter. token = AuthenticationToken.last + deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + Capybara.default_max_wait_time + until token.reload.description == description + break if Process.clock_gettime(Process::CLOCK_MONOTONIC) > deadline + sleep 0.05 + end assert_equal(description, token.description) end diff --git a/test/system/search_test.rb b/test/system/search_test.rb index 334e22e93f..803059a23c 100644 --- a/test/system/search_test.rb +++ b/test/system/search_test.rb @@ -16,11 +16,9 @@ class SearchTest < ApplicationSystemTestCase find("[data-search-token-target~=query]").fill_in with: @feed.title - wait_for_ajax - - all("[data-search-token-index-param]")[1].click - - wait_for_ajax + # The autocomplete fires behind a debounce; all(minimum:) waits for the + # suggestions to actually render. + all("[data-search-token-index-param]", minimum: 2)[1].click token = find("[data-action='search-token#deleteToken:prevent']") assert token.visible? diff --git a/test/system/xss_test.rb b/test/system/xss_test.rb index b0e89780e7..dfb7c01b7a 100644 --- a/test/system/xss_test.rb +++ b/test/system/xss_test.rb @@ -24,7 +24,9 @@ class XssTest < ApplicationSystemTestCase find("[data-event-identifier-param=toggle-search]").click wait_for_ajax find("[data-search-token-target~=query]").fill_in with: "XSS137" - wait_for_ajax + + # Wait for the debounced autocomplete to render before inspecting it. + all("[data-search-token-index-param]", minimum: 1) assert_equal "not executed", page.evaluate_script("window.__xss137"), "the tag name executed as markup" @@ -54,10 +56,10 @@ class XssTest < ApplicationSystemTestCase find("[data-event-identifier-param=toggle-search]").click wait_for_ajax find("[data-search-token-target~=query]").fill_in with: "XSS140" - wait_for_ajax - all("[data-search-token-index-param]")[1].click - wait_for_ajax + # The autocomplete fires behind a debounce; all(minimum:) waits for the + # suggestions to actually render. + all("[data-search-token-index-param]", minimum: 2)[1].click token = find("[data-action='search-token#deleteToken:prevent']") assert_equal title, token.text(:all), "the token should show the title, not its entities" diff --git a/test/test_helper.rb b/test/test_helper.rb index dc3e17b663..b092431dad 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -161,6 +161,17 @@ def outside_transaction yield OutsideTransaction end + # Temporarily replaces a constant for the block — for thresholds a test + # would otherwise have to loop hundreds of real iterations to cross. + def swap_const(mod, name, value) + original = mod.send(:remove_const, name) + mod.const_set(name, value) + yield + ensure + mod.send(:remove_const, name) + mod.const_set(name, original) + end + def support_file(file) File.join(Rails.root, "test/support/www", file) end @@ -224,12 +235,31 @@ def create_stripe_plan(plan) Stripe::Plan.create(name: plan.name, id: plan.stripe_id, amount: plan.price.to_i, currency: "USD", interval: "day") end + # Empties the search indexes without deleting them — index creation is far + # more expensive than a match_all delete_by_query, and this runs in the + # setup of every search-adjacent test. Falls back to recreating via + # Search.setup when an alias is missing (a test deleted or swapped the + # physical index behind it). def clear_search - Search.client { _1.request(:delete, $search[:config][:aliases][:entries]) } - Search.client { _1.request(:delete, $search[:config][:aliases][:actions]) } - Search.client { _1.request(:delete, $search[:config][:aliases][:feeds]) } + Search.client do |client| + # delete_by_query only sees documents a refresh has made visible, so + # without this a doc indexed by an earlier test and never refreshed + # would survive the wipe and haunt a later search. + client.refresh + [Entry, Action, Feed].map { Search.index_name(_1.table_name) }.each do |alias_name| + response = clear_index(client, alias_name) + if response.key?("error") + Search.setup + clear_index(client, alias_name) + end + end + end + end - Search.setup + def clear_index(client, alias_name) + client.request(:post, "/#{alias_name}/_delete_by_query", + params: {refresh: "true", conflicts: "proceed"}, + json: {query: {match_all: {}}}) end def newsletter_params(recipient, signature, title = nil, from = nil)