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

(REF) Extract LocaleTestTrait. Tighten up multilingual testing. #24291

Merged
merged 10 commits into from
Aug 17, 2022

Conversation

totten
Copy link
Member

@totten totten commented Aug 17, 2022

Overview

This is a test-only change. Generally, it tightens up the way in which multilingual mode is enabled/disabled -- with an aim to (a) improving realism, (b) reducing duplication, (c) recovering better from test-errors.

Technical Details

There are multiple steps to setting up a valid multilingual environment, such as:

  • Marking ML as enabled
  • Rearranging DB columns for the original locale
  • Adding DB columns for 1-2 additional locales
  • Setting the list of active ML locales

Additionally, it is important to reverse these steps at the end. If there are scenarios where it doesn't cleanup (eg due to a failed test-assertion) or where it only partially cleans up

Before

Tests use some mix of:

  • $this->enableMultilingual() or \CRM_Core_I18n_Schema::makeMultilingual()
  • Maybe \CRM_Core_I18n_Schema::makeSinglelingua()l
  • Maybe update Civi settings for ML
  • Maybe add languages to ML

Some tests also don't do cleanup in a safe way (eg a failed assertion will cascade to break other tests)

If you try to run the entire phpunit --group locale, it doesn't work.

After

More consistent code-style. Either:

  • Use the pair $this->enableMultilingual(['srcLocale' => 'newLocale']) and $this->disableMultilingual() (with try/finally or setup/teardown)
  • Use the autoclean $cleanup = $this->useMultilingual(['srcLocale' => 'newLocale'])

If you try to run the entire phpunit --group locale, it does work.

@civibot
Copy link

civibot bot commented Aug 17, 2022

(Standard links)

@eileenmcnaughton
Copy link
Contributor

test only change - MOP

@demeritcowboy demeritcowboy merged commit a8c80a1 into civicrm:master Aug 17, 2022
@totten totten deleted the master-ml-test branch August 17, 2022 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants