From 4d65507cbbd5dfc0c0ca776570e272e20df6054c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 27 May 2020 18:49:31 +0200 Subject: [PATCH 1/3] Check for exact text in `have_ckeditor` If we don't use the `exact` option, tests will pass even if filling in CKEditor adds the content twice or adds the new content to the existing content, which has actually happened and has gone mostly unnoticed while testing several ways to fill in CKEditor with Capybara (particularly, when using Capybara's `send_keys` method). The problem was detected by just one test, which checked the original content wasn't present anymore after updating a record. --- spec/support/matchers/have_ckeditor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/matchers/have_ckeditor.rb b/spec/support/matchers/have_ckeditor.rb index 8072ad511..e25df236d 100644 --- a/spec/support/matchers/have_ckeditor.rb +++ b/spec/support/matchers/have_ckeditor.rb @@ -15,7 +15,7 @@ RSpec::Matchers.define :have_ckeditor do |label, with:| return false unless has_ckeditor? page.within(ckeditor_id) do - within_frame(0) { has_content?(with) } + within_frame(0) { has_content?(with, exact: true) } end end From 8408bfdcf00f7cb97b21b578e2caca4779cf1d6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 27 May 2020 18:51:43 +0200 Subject: [PATCH 2/3] Don't use ckeditor.setData in specs After upgrading to chromedriver 80, tests checking CKEditor's content were causing chromedriver to hang. That's why we were configuring webdrivers to use an older chromedriver. Version 80 of chromedriver introduced several issues regarding frames. Debugging shows in this case chromedriver froze when we used `setData` and then `within_frame`. Since adding a `sleep` call made it work, we think `within_frame` was being executed before `setData` had finished. The fact that `setData` causes the browser to enter the frame having CKEditor is probably the reason. Even though the `setData` method provides a callback when it's finished, configuring it so the rest of the Ruby code isn't executed until that happens leads to complex code. Using Capybara's `set` to fill in the editor is IMHO a bit easier to understand. After this change, since we're using a method provided by Capybara instead of executing asynchronous JavaScript code, we don't have to check CKEditor has been filled anymore. The "Admin Active polls add" test, which failed on my machine without that check, now passes. --- spec/support/common_actions/verifications.rb | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/spec/support/common_actions/verifications.rb b/spec/support/common_actions/verifications.rb index 92ef462e0..d9c4b2c68 100644 --- a/spec/support/common_actions/verifications.rb +++ b/spec/support/common_actions/verifications.rb @@ -51,14 +51,8 @@ module Verifications sleep 0.01 end - # Fill the editor content - page.execute_script <<-SCRIPT - var ckeditor = CKEDITOR.instances.#{locator} - ckeditor.setData("#{with}") - ckeditor.focus() - ckeditor.updateElement() - SCRIPT - - expect(page).to have_ckeditor label, with: with + within("#cke_#{locator}") do + within_frame(0) { find("body").set(with) } + end end end From 72c2b87227d502c8a2875492a7480ada4e432884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 27 May 2020 20:24:54 +0200 Subject: [PATCH 3/3] Wait till CKEditor is ready before checking it With chromedriver >= 80, the tests are freezing sometimes, particularly when the same editor is loaded again. We don't know whether it's a CKEditor issue or a chromedriver issue. In the past we've had some errors related to CKEditor trying to load the same instance twice and we aren't sure they have been fixed since we could never reproduce them. It could be a coincidence, though. If we modify the views so the only content of the `` tag is a textarea with the `html-area` class, chromedriver freezes even if we only access the page once. So maybe we're only detecting the problem on the second visit because the second request is faster than the first one. Since chromedriver no longer hangs after this change, we don't have to force any chromedriver version anymore. --- spec/rails_helper.rb | 1 - spec/support/matchers/have_ckeditor.rb | 4 ++++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 28abc03d9..3de8fd85d 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -53,6 +53,5 @@ Capybara.register_driver :headless_chrome do |app| end Capybara.exact = true -Webdrivers::Chromedriver.required_version = "2.38" OmniAuth.config.test_mode = true diff --git a/spec/support/matchers/have_ckeditor.rb b/spec/support/matchers/have_ckeditor.rb index e25df236d..6a5a8585d 100644 --- a/spec/support/matchers/have_ckeditor.rb +++ b/spec/support/matchers/have_ckeditor.rb @@ -14,6 +14,10 @@ RSpec::Matchers.define :have_ckeditor do |label, with:| match do return false unless has_ckeditor? + until page.execute_script("return CKEDITOR.instances.#{textarea_id}.status === 'ready';") do + sleep 0.01 + end + page.within(ckeditor_id) do within_frame(0) { has_content?(with, exact: true) } end