Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set locales in test environment to avoid failed specs #3537

Merged

Conversation

PierreMesure
Copy link
Collaborator

@PierreMesure PierreMesure commented May 24, 2019

References

Objectives

A few different specs fail when you inactivate certain languages.

This is a problem because you want people to keep a clean test suite for their forks but they might want to disable languages.

We solve this by overriding the available languages for the test environment.

Notes

  • Right now, I am setting only the necessary languages (en, es, fr, nl, pt-BR). This is open to discussion.

@PierreMesure PierreMesure force-pushed the set-locales-in-test-environment branch from 7498de7 to 9719e08 Compare May 24, 2019 20:40
@javierm
Copy link
Member

javierm commented Sep 6, 2019

I really like the idea of setting only the necessary languages 👍. We definitely need pt-BR (or a similar language) available in the test environment so we make sure we handle language names containing a - properly. Maybe in the future we find a way for our tests to dynamically choose which languages to use among the ones enabled, or just use imaginary languages which are added during a test, but for now I think this idea is simple and does the job really well.

config/environments/test.rb Outdated Show resolved Hide resolved
config/environments/test.rb Show resolved Hide resolved
@javierm
Copy link
Member

javierm commented Sep 7, 2019

There's only a small problem after applying these changes to current master: the tests for remote translations assume a few more languages are available. @taitus What do you think about changing the remote translations spec to use less languages? For example:

diff --git a/spec/features/remote_translations_spec.rb b/spec/features/remote_translations_spec.rb
index 1ff98d9..92a16d1 100644
--- a/spec/features/remote_translations_spec.rb
+++ b/spec/features/remote_translations_spec.rb
@@ -5,8 +5,7 @@ describe "Remote Translations" do
   before do
     Setting["feature.remote_translations"] = true
     create(:proposal)
-    available_locales_response = ["ar", "de", "en", "es", "fa", "fr", "he", "it", "nl", "pl",
-                                  "pt", "sv", "zh-Hans", "zh-Hant"]
+    available_locales_response = %w[de en es fr pt zh-Hans]
     expect(RemoteTranslations::Microsoft::AvailableLocales).to receive(:available_locales).
                                                             and_return(available_locales_response)
   end
@@ -27,18 +26,14 @@ describe "Remote Translations" do
       end
 
       scenario "should display text in English" do
-        available_locales_with_fallback_en = [:ar, :de, :fa, :he, :nl, :pl, :sv]
-
-        visit root_path(locale: available_locales_with_fallback_en.sample)
+        visit root_path(locale: :de)
 
         expect(page).to have_css ".remote-translations-button"
         expect(page).to have_content "The content of this page is not available in your language"
       end
 
       scenario "should display text in English after parse key" do
-        available_locales_with_fallback_en = [:"zh-CN", :"zh-TW"]
-
-        visit root_path(locale: available_locales_with_fallback_en.sample)
+        visit root_path(locale: :"zh-CN")
 
         expect(page).to have_css ".remote-translations-button"
         expect(page).to have_content "The content of this page is not available in your language"
@@ -53,9 +48,7 @@ describe "Remote Translations" do
       end
 
       scenario "with locale that has :es fallback" do
-        available_locales_with_fallback_es = [:es, :fr, :it]
-
-        visit root_path(locale: available_locales_with_fallback_es.sample)
+        visit root_path(locale: :fr)
 
         expect(page).to have_css ".remote-translations-button"
         expect(page).to have_content "El contenido de esta página no está disponible en tu idioma"
@@ -72,9 +65,7 @@ describe "Remote Translations" do
   end
 
   scenario "Not display remote translation button when locale is not included in microsoft translate client" do
-    not_available_locales = [:val, :gl, :sq]
-
-    visit root_path(locale: not_available_locales.sample)
+    visit root_path(locale: :nl)
 
     expect(page).not_to have_css ".remote-translations-button"
   end

@PierreMesure
Copy link
Collaborator Author

PierreMesure commented Sep 8, 2019

Hey Javier, great that you're looking at this! I just applied the changes you requested!

@javierm
Copy link
Member

javierm commented Sep 9, 2019

@PierreMesure Here's a patch which will (hopefully) make the remote translation tests compatible with this pull request. You can apply it using git am 0001-Reduce-locales-used-in-remote-translations-tests.txt.

0001-Reduce-locales-used-in-remote-translations-tests.txt

Once you apply the patch, push the changes and then I think we can safely merge this pull request 🎉.

@javierm javierm added this to Doing in Roadmap Sep 10, 2019
@javierm javierm added the Specs label Sep 11, 2019
This way we can reduce the number of locales used in the test
environment as well, while still testing every possible scenario for
remote translations.
Roadmap automation moved this from Doing to Testing Sep 11, 2019
@javierm javierm merged commit cb6539d into consuldemocracy:master Sep 11, 2019
Roadmap automation moved this from Testing to Release 1.1.0 Sep 11, 2019
@javierm
Copy link
Member

javierm commented Sep 11, 2019

@PierreMesure Awesome! Thank you very much! 🎉

smarques pushed a commit to venetochevogliamo/consul that referenced this pull request Apr 29, 2020
…-in-test-environment

Set locales in test environment to avoid failed specs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Roadmap
  
Release 1.1.0
Development

Successfully merging this pull request may close these issues.

Translatable spec should pass even if some languages are disabled
3 participants