Dont translate 'Save and continue' in InstallerTestBase::setUpLanguage

Needs a reroll. See core/tests/Drupal/FunctionalTests/Installer/InstallerTestBase.php.

Rerolled patch #2 for 9.1.x branch, please review.

I'm trying to understand if this change makes sense to me. Looking at the existing code in InstallerTestBase.php, I see 'Save and continue' used in:

  protected function setUpLanguage() {
    $edit = [
      'langcode' => $this->langcode,
    ];
    $this->drupalPostForm(NULL, $edit, $this->translations['Save and continue']);
  }
...
  protected function setUpProfile() {
    $edit = [
      'profile' => $this->profile,
    ];
    $this->drupalPostForm(NULL, $edit, $this->translations['Save and continue']);
  }
...
  protected function setUpSettings() {
    $edit = $this->translatePostValues($this->parameters['forms']['install_settings_form']);
    $this->drupalPostForm(NULL, $edit, $this->translations['Save and continue']);
  }
...
  protected function setUpSite() {
    $edit = $this->translatePostValues($this->parameters['forms']['install_configure_form']);
    $this->drupalPostForm(NULL, $edit, $this->translations['Save and continue']);
    // If we've got to this point the site is installed using the regular
    // installation workflow.
    $this->isInstalled = TRUE;
  }

Other than the above and other tests, I see 'Save and continue' in:

./core/lib/Drupal/Core/Installer/Form/SiteConfigureForm.php:      '#value' => $this->t('Save and continue'),
./core/lib/Drupal/Core/Installer/Form/SelectLanguageForm.php:      '#value' => 'Save and continue',
./core/lib/Drupal/Core/Installer/Form/SiteSettingsForm.php:      '#value' => $this->t('Save and continue'),
./core/lib/Drupal/Core/Installer/Form/SelectProfileForm.php:      '#value' => $this->t('Save and continue'),
./core/modules/field_ui/src/Form/FieldStorageAddForm.php:      '#value' => $this->t('Save and continue'),

where SelectLanguageForm.phpis intentionally not using translation as noted in the comments:

 * Note that hardcoded text provided by this form is not translated. This is
 * because translations are downloaded as a result of submitting this form.

There are many classes that extend InstallerTestBase. For example, core/tests/Drupal/FunctionalTests/Installer/InstallerTranslationMultipleLanguageTest.php where the parent method is called after adding the translation files:

  protected function setUpLanguage() {
    // Place custom local translations in the translations directory.
    mkdir(DRUPAL_ROOT . '/' . $this->siteDirectory . '/files/translations', 0777, TRUE);
    file_put_contents(DRUPAL_ROOT . '/' . $this->siteDirectory . '/files/translations/drupal-8.0.0.de.po', $this->getPo('de'));
    file_put_contents(DRUPAL_ROOT . '/' . $this->siteDirectory . '/files/translations/drupal-8.0.0.es.po', $this->getPo('es'));

    parent::setUpLanguage();
  }

This looks like, while the translation files are written, they aren't yet loaded so the translations would not be available yet.

So, if I'm understanding correctly, the reason the code works before patching is because there is no translation yet available so English is just passed through as is. In that, the change makes sense to me. Given this and the following I'm marking RTBC:

1) Patch applies
2) Tests pass
3) Change is straightforward and addresses the problem noted in the issue summary
4) No manual testing should be need as it should be covered by automated testing

Though I'm wondering if it could use a comment so that it's clear why translation isn't used (similar to the comments for SelectLanguageForm.php).

Whether it's worth committing to core, that is update to the core maintainers. :)

Pulling this one back after thinking about it more. I think it needs a comment. I would suggest something like:

// Text is hardcoded since translations are not yet available.

Here I have addressed comment #16.

Thanks for the update.

1) Interdiff looks good.

2) Tests still pass.

3) Based on this and #15, marking RTBC.

1. +++ b/core/tests/Drupal/FunctionalTests/Installer/InstallerTestBase.php
   @@ -219,7 +219,8 @@ protected function setUpLanguage() {
   -    $this->drupalPostForm(NULL, $edit, $this->translations['Save and continue']);
   

   The translations property should be removed. no usages in child classes., edited, I was wrong.

2. +++ b/core/tests/Drupal/FunctionalTests/Installer/InstallerTestBase.php
   @@ -219,7 +219,8 @@ protected function setUpLanguage() {
   +    // Text is hardcoded since translations are not yet available.
   

   The comment is unnecessary to me, but happy to add it back.

Addressing #20. Not sure what I am doing is right, let's waiting for the testing run finishes.

diff --git a/core/tests/Drupal/FunctionalTests/Installer/InstallerTestBase.php b/core/tests/Drupal/FunctionalTests/Installer/InstallerTestBase.php
index 4268fbce6b..8f6e7c6a54 100644
--- a/core/tests/Drupal/FunctionalTests/Installer/InstallerTestBase.php
+++ b/core/tests/Drupal/FunctionalTests/Installer/InstallerTestBase.php
@@ -54,17 +54,6 @@ abstract class InstallerTestBase extends BrowserTestBase {
    */
   protected $parameters = [];
 
-  /**
-   * A string translation map used for translated installer screens.
-   *
-   * Keys are English strings, values are translated strings.
-   *
-   * @var array
-   */
-  protected $translations = [
-    'Save and continue' => 'Save and continue',
-  ];
-

$translations is used by some child class like ConfigTranslationInstallTest who extending InstallerTestBase class so not removing this and just removing $this->translations from InstallerTestBase class. Kindly review a new patch

Thinking again, I would say that this is a won't fix. The original code is ok to me. It does not call t() nor new TranslatableMarkup(), Dont translate in the title does not make sense.

+++ b/core/tests/Drupal/FunctionalTests/Installer/InstallerTestBase.php
@@ -229,7 +229,7 @@ protected function setUpProfile() {
-    $this->drupalPostForm(NULL, $edit, $this->translations['Save and continue']);
+    $this->drupalPostForm(NULL, $edit, 'Save and continue');

@@ -237,7 +237,7 @@ protected function setUpProfile() {
-    $this->drupalPostForm(NULL, $edit, $this->translations['Save and continue']);
+    $this->drupalPostForm(NULL, $edit, 'Save and continue');

@@ -257,7 +257,7 @@ protected function setUpRequirementsProblem() {
-    $this->drupalPostForm(NULL, $edit, $this->translations['Save and continue']);
+    $this->drupalPostForm(NULL, $edit, 'Save and continue');

These are all out of scope. The IS specifically says this is for \Drupal\FunctionalTests\Installer\InstallerTestBase::setUpLanguage() only.

Now the question is the 'select language' step of the install process only in English. I wasn't sure so I tested manually. Yes, it is always in English. The 'Save and continue' text is always English no matter what non-English language I selected for the installation. Based on that I would say the patch in #18 is on the right track. It just needs an improved comment.

+++ b/core/tests/Drupal/FunctionalTests/Installer/InstallerTestBase.php
@@ -219,7 +219,8 @@ protected function setUpLanguage() {
+    // Text is hardcoded since translations are not yet available.

This would be better if we included where this is hard coded (save the next dev some time). Something like this "The 'Select Language' step is always English.
@See \Drupal\Core\Installer\Form\SelectLanguageForm."

Thanks @quietone for the review.

Adjusted per @quietone's suggestion based on #18. But I still do not think the change/improvement here is necessary as I commented in #24.

This makes sense. Its a minor fix at best though :) Thanks for looking at this.

Committed 9b4c2f0 and pushed to 9.1.x. Thanks!