From d636f1fe75fece002d0837b7c175a9f271b471a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 15 Sep 2024 00:06:18 +0200 Subject: [PATCH 1/5] Add missing expectations in remotely translatable tests After changing the language, we were checking that certain content isn't there. However, the content wasn't there before changing the language either, so the test will pass even if the request to change the language hasn't finished. Although this is probably OK because we aren't changing the language using an AJAX request, and so Capybara will correctly wait until the request is finished before finishing the test, confirming that the page has changed after a request is something we try to do in every test. --- spec/shared/system/remotely_translatable.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/spec/shared/system/remotely_translatable.rb b/spec/shared/system/remotely_translatable.rb index f23d7e3ac..8b564498a 100644 --- a/spec/shared/system/remotely_translatable.rb +++ b/spec/shared/system/remotely_translatable.rb @@ -38,6 +38,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume select "Español", from: "Language:" + expect(page).to have_select "Idioma:" expect(page).not_to have_button("Traducir página") end @@ -47,6 +48,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume select "Español", from: "Language:" + expect(page).to have_select "Idioma:" expect(page).not_to have_button("Traducir página") end @@ -57,9 +59,9 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume select "Español", from: "Language:" - expect(page).not_to have_button("Traducir página") expect(page).to have_content "En un breve periodo de tiempo refrescando la página " \ "podrá ver todo el contenido en su idioma" + expect(page).not_to have_button("Traducir página") end end @@ -73,6 +75,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume select "Español", from: "Language:" + expect(page).to have_select "Idioma:" expect(page).not_to have_button("Traducir página") end end @@ -97,6 +100,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume select "Español", from: "Language:" + expect(page).to have_select "Idioma:" expect(page).not_to have_button("Traducir página") end end @@ -167,10 +171,10 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume visit path select "Español", from: "Idioma:" - expect(page).not_to have_button "Traducir página" - expect(page).not_to have_content "Se han solicitado correctamente las traducciones." expect(page).to have_content "En un breve periodo de tiempo refrescando la página " \ "podrá ver todo el contenido en su idioma" + expect(page).not_to have_button "Traducir página" + expect(page).not_to have_content "Se han solicitado correctamente las traducciones." end end From 95dc70acee134b946ee7d92ecc1bd37954e5d79b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 15 Sep 2024 01:03:23 +0200 Subject: [PATCH 2/5] Remove parentheses in remove translatable expectations So it's consistent with both the rest of the file and what we usually do. --- spec/shared/system/remotely_translatable.rb | 38 ++++++++++----------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/spec/shared/system/remotely_translatable.rb b/spec/shared/system/remotely_translatable.rb index 8b564498a..6411f3300 100644 --- a/spec/shared/system/remotely_translatable.rb +++ b/spec/shared/system/remotely_translatable.rb @@ -20,7 +20,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume scenario "should not be present when current locale translation exists" do visit path - expect(page).not_to have_button("Translate page") + expect(page).not_to have_button "Translate page" end scenario "should be present when current locale translation does not exists" do @@ -28,18 +28,18 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume select "Español", from: "Language:" - expect(page).to have_button("Traducir página") + expect(page).to have_button "Traducir página" end scenario "should not be present when new current locale translation exists" do add_translations(resource, :es) visit path - expect(page).not_to have_button("Translate page") + expect(page).not_to have_button "Translate page" select "Español", from: "Language:" expect(page).to have_select "Idioma:" - expect(page).not_to have_button("Traducir página") + expect(page).not_to have_button "Traducir página" end scenario "should not be present when there are no resources to translate", if: index_path?(path_name) do @@ -49,7 +49,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume select "Español", from: "Language:" expect(page).to have_select "Idioma:" - expect(page).not_to have_button("Traducir página") + expect(page).not_to have_button "Traducir página" end describe "with delayed job active", :delay_jobs do @@ -61,7 +61,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume expect(page).to have_content "En un breve periodo de tiempo refrescando la página " \ "podrá ver todo el contenido en su idioma" - expect(page).not_to have_button("Traducir página") + expect(page).not_to have_button "Traducir página" end end @@ -71,12 +71,12 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume add_translations(resource, :es) create(:comment, commentable: resource) visit path - expect(page).not_to have_button("Translate page") + expect(page).not_to have_button "Translate page" select "Español", from: "Language:" expect(page).to have_select "Idioma:" - expect(page).not_to have_button("Traducir página") + expect(page).not_to have_button "Traducir página" end end @@ -85,23 +85,23 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume add_translations(resource, :es) create(:comment, commentable: resource) visit path - expect(page).not_to have_button("Translate page") + expect(page).not_to have_button "Translate page" select "Español", from: "Language:" - expect(page).to have_button("Traducir página") + expect(page).to have_button "Traducir página" end scenario "not display when exists resource translations but his comment has tanslations" do add_translations(resource, :es) create_comment_with_translations(resource, :es) visit path - expect(page).not_to have_button("Translate page") + expect(page).not_to have_button "Translate page" select "Español", from: "Language:" expect(page).to have_select "Idioma:" - expect(page).not_to have_button("Traducir página") + expect(page).not_to have_button "Traducir página" end end @@ -110,11 +110,11 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume add_translations(resource, :es) create_featured_debates visit path - expect(page).not_to have_button("Translate page") + expect(page).not_to have_button "Translate page" select "Español", from: "Language:" - expect(page).to have_button("Traducir página") + expect(page).to have_button "Traducir página" end end @@ -124,11 +124,11 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume add_translations(resource, :es) create_featured_proposals visit path - expect(page).not_to have_button("Translate page") + expect(page).not_to have_button "Translate page" select "Español", from: "Language:" - expect(page).to have_button("Traducir página") + expect(page).to have_button "Traducir página" end end end @@ -141,7 +141,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume click_button "Traducir página" - expect(page).not_to have_button("Traducir página") + expect(page).not_to have_button "Traducir página" end scenario "the remote translation is pending to translate" do @@ -166,7 +166,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume visit path select "Español", from: "Language:" click_button "Traducir página" - expect(page).to have_content("Se han solicitado correctamente las traducciones.") + expect(page).to have_content "Se han solicitado correctamente las traducciones." visit path select "Español", from: "Idioma:" @@ -187,7 +187,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume click_button "Traducir página" - expect(page).not_to have_button("Traducir página") + expect(page).not_to have_button "Traducir página" end scenario "the remote translation has been translated and destoyed" do From 3ab9fb1d27d33b65e0e756495bfef2baf5d824da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 21 Sep 2024 21:26:52 +0200 Subject: [PATCH 3/5] Simplify similar remotely translatable tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We were using the same setup in these tests, and we were only changing the expectations or adding an extra step. Note we're also using `refresh` to simplify the code and because we were using `select "Español", from: "Idioma:"` when that language was already selected. --- spec/shared/system/remotely_translatable.rb | 47 +++++---------------- 1 file changed, 10 insertions(+), 37 deletions(-) diff --git a/spec/shared/system/remotely_translatable.rb b/spec/shared/system/remotely_translatable.rb index 6411f3300..3ad197d49 100644 --- a/spec/shared/system/remotely_translatable.rb +++ b/spec/shared/system/remotely_translatable.rb @@ -135,15 +135,6 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume context "After click remote translations button" do describe "with delayed jobs", :delay_jobs do - scenario "the remote translation button should not be present" do - visit path - select "Español", from: "Language:" - - click_button "Traducir página" - - expect(page).not_to have_button "Traducir página" - end - scenario "the remote translation is pending to translate" do visit path select "Español", from: "Language:" @@ -151,45 +142,27 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume expect { click_button "Traducir página" }.to change { RemoteTranslation.count }.from(0).to(1) end - scenario "should be present enqueued notice and informative text" do + scenario "shows informative text when content is enqueued" do visit path select "Español", from: "Language:" click_button "Traducir página" - expect(page).to have_content "Se han solicitado correctamente las traducciones." - expect(page).to have_content "En un breve periodo de tiempo refrescando la página " \ - "podrá ver todo el contenido en su idioma" - end - - scenario "should be present only informative text when user visit page with all content enqueued" do - visit path - select "Español", from: "Language:" - click_button "Traducir página" - expect(page).to have_content "Se han solicitado correctamente las traducciones." - - visit path - select "Español", from: "Idioma:" - - expect(page).to have_content "En un breve periodo de tiempo refrescando la página " \ - "podrá ver todo el contenido en su idioma" expect(page).not_to have_button "Traducir página" + expect(page).to have_content "Se han solicitado correctamente las traducciones." + expect(page).to have_content "En un breve periodo de tiempo refrescando la página " \ + "podrá ver todo el contenido en su idioma" + + refresh + expect(page).not_to have_content "Se han solicitado correctamente las traducciones." + expect(page).not_to have_button "Traducir página" + expect(page).to have_content "En un breve periodo de tiempo refrescando la página " \ + "podrá ver todo el contenido en su idioma" end end describe "without delayed jobs" do - scenario "the remote translation button should not be present" do - response = generate_response(resource) - expect_any_instance_of(RemoteTranslations::Microsoft::Client).to receive(:call).and_return(response) - visit path - select "Español", from: "Language:" - - click_button "Traducir página" - - expect(page).not_to have_button "Traducir página" - end - scenario "the remote translation has been translated and destoyed" do response = generate_response(resource) expect_any_instance_of(RemoteTranslations::Microsoft::Client).to receive(:call).and_return(response) From 314019bee758623b584a2065b88bf3c00224fcdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 23 Sep 2024 15:10:25 +0200 Subject: [PATCH 4/5] Remove database expectations in remotely translatable tests In the past, having this kind of expectations after the process running the browser has started has resulted in flaky issues with the database connection. In one case, we're removing the test because there are controller tests covering the same scenario and a system test checking what happens from the user perspective. In the other case, we're replacing the expectations with expectations from the user's point of view. --- spec/shared/system/remotely_translatable.rb | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/spec/shared/system/remotely_translatable.rb b/spec/shared/system/remotely_translatable.rb index 3ad197d49..ee61d001d 100644 --- a/spec/shared/system/remotely_translatable.rb +++ b/spec/shared/system/remotely_translatable.rb @@ -135,13 +135,6 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume context "After click remote translations button" do describe "with delayed jobs", :delay_jobs do - scenario "the remote translation is pending to translate" do - visit path - select "Español", from: "Language:" - - expect { click_button "Traducir página" }.to change { RemoteTranslation.count }.from(0).to(1) - end - scenario "shows informative text when content is enqueued" do visit path select "Español", from: "Language:" @@ -163,17 +156,19 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume end describe "without delayed jobs" do - scenario "the remote translation has been translated and destoyed" do + scenario "content is immediately translated" do response = generate_response(resource) expect_any_instance_of(RemoteTranslations::Microsoft::Client).to receive(:call).and_return(response) visit path select "Español", from: "Language:" + expect(page).to have_select "Idioma:" + expect(page).not_to have_content response.first + click_button "Traducir página" expect(page).not_to have_button "Traducir página" - expect(RemoteTranslation.count).to eq(0) - expect(resource.translations.count).to eq(2) + expect(page).to have_content response.first end scenario "request a translation of an already translated text" do From 3e44eeaee0f3a5bcc1fe1e48a7dbd90acff32104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 23 Sep 2024 17:36:55 +0200 Subject: [PATCH 5/5] Directly select language in remotely translatable tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test "request a translation of an already translated text" was failing sometimes on our CI since August 29, maybe due to a change in GitHub Actions since the test had been passing for a year and a half and we didn't change any code around that time (we were updating the documentation). While the root cause is unknown, debugging shows that sometimes (usually the first time this test is executed on our CI, and only the first time, since running it 600 tests in a row also resulted in only one failure) the request done by clicking on "Traducir página" is done with a user session where the locale is in English. This doesn't make much sense, since both user sessions are already in Spanish (and we had either explicit or implicit expectations to confirm that), and debugging shows that the session is indeed in Spanish during the previous request. In any case, we're solving it by never using English during the test, since it wasn't necessary; it was only done that way because all the tests on this file used the language selector to get to the Spanish pages. We're simplifying some of the other tests the same way. The test failure was: ``` Failure/Error: expect(page).to have_content "Se han solicitado correctamente las traducciones" expected to find text "Se han solicitado correctamente las traducciones" in "Idioma: \n \nEnglish\nDeutsch\nEspañol\nFrançais\nNederlands\nPortuguês brasileiro\n中文\n Entrar\nRegistrarse\nDebates\nPropuestas\nVotaciones\nLegislación colaborativa\nPresupuestos participativos\nODS\nAyuda\n×\nTranslations have been correctly requested.\nPropuestas más activas\nAhora mismo no hay propuestas\nDebates más activos\nndfrrqufrp\nVer todos los debates\nProcesos abiertos\nAhora mismo no hay procesos abiertos\nGobierno abierto\nEste portal usa la aplicación CONSUL DEMOCRACY que es software de código abierto.\nParticipación\nDecide cómo debe ser la ciudad que quieres.\nCONSUL DEMOCRACY, 2024 Política de privacidad Condiciones de uso Accesibilidad" ``` Note that most of the text is in Spanish (as expected) but the flash message itself is in English. --- spec/shared/system/remotely_translatable.rb | 27 ++++++--------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/spec/shared/system/remotely_translatable.rb b/spec/shared/system/remotely_translatable.rb index ee61d001d..770d85f49 100644 --- a/spec/shared/system/remotely_translatable.rb +++ b/spec/shared/system/remotely_translatable.rb @@ -5,6 +5,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume path_arguments.transform_values { |path_to_value| resource.send(path_to_value) } end let(:path) { send(path_name, arguments) } + let(:path_in_spanish) { send(path_name, arguments.merge(locale: :es)) } let!(:resource) { create(factory_name) } before do @@ -24,9 +25,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume end scenario "should be present when current locale translation does not exists" do - visit path - - select "Español", from: "Language:" + visit path_in_spanish expect(page).to have_button "Traducir página" end @@ -44,20 +43,15 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume scenario "should not be present when there are no resources to translate", if: index_path?(path_name) do resource.destroy! - visit path + visit path_in_spanish - select "Español", from: "Language:" - - expect(page).to have_select "Idioma:" expect(page).not_to have_button "Traducir página" end describe "with delayed job active", :delay_jobs do scenario "should not be present when an equal RemoteTranslation is enqueued" do create(:remote_translation, remote_translatable: resource, locale: :es) - visit path - - select "Español", from: "Language:" + visit path_in_spanish expect(page).to have_content "En un breve periodo de tiempo refrescando la página " \ "podrá ver todo el contenido en su idioma" @@ -136,8 +130,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume context "After click remote translations button" do describe "with delayed jobs", :delay_jobs do scenario "shows informative text when content is enqueued" do - visit path - select "Español", from: "Language:" + visit path_in_spanish click_button "Traducir página" @@ -159,10 +152,8 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume scenario "content is immediately translated" do response = generate_response(resource) expect_any_instance_of(RemoteTranslations::Microsoft::Client).to receive(:call).and_return(response) - visit path - select "Español", from: "Language:" + visit path_in_spanish - expect(page).to have_select "Idioma:" expect(page).not_to have_content response.first click_button "Traducir página" @@ -176,15 +167,13 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume expect_any_instance_of(RemoteTranslations::Microsoft::Client).to receive(:call).and_return(response) in_browser(:one) do - visit path - select "Español", from: "Language:" + visit path_in_spanish expect(page).to have_button "Traducir página" end in_browser(:two) do - visit path - select "Español", from: "Language:" + visit path_in_spanish click_button "Traducir página" expect(page).to have_content "Se han solicitado correctamente las traducciones"