Skip to content

Cache language packs#3595

Merged
danielbachhuber merged 10 commits into
wp-cli:masterfrom
ocean90:cache-language-packs
Nov 23, 2016
Merged

Cache language packs#3595
danielbachhuber merged 10 commits into
wp-cli:masterfrom
ocean90:cache-language-packs

Conversation

@ocean90
Copy link
Copy Markdown
Contributor

@ocean90 ocean90 commented Nov 23, 2016

Fixes #3594.

Comment thread php/WP_CLI/LanguagePackUpgrader.php Outdated
return $package; //must be a local file..

if ( empty( $package ) )
return new WP_Error( 'no_package', $this->strings['no_package'] );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs the same namespace fix.

Comment thread features/core-language.feature Outdated
Then the wp-content/languages/admin-en_GB.po file should exist
And the wp-content/languages/en_GB.po file should exist
And the {SUITE_CACHE_DIR}/translation/core-default-([\d\.]+)-en_AU-([\d]+).zip file should exist
And the {SUITE_CACHE_DIR}/translation/core-default-([\d\.]+)-en_GB-([\d]+).zip file should exist
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is still failing locally:

@require-wp-4.0
  Scenario: Core translation CRUD                                                                    # features/core-language.feature:4
    Given a WP install                                                                               # features/steps/given.php:47
    And an empty cache                                                                               # features/steps/given.php:14
    When I run `wp core language list --fields=language,english_name,status`                         # features/steps/when.php:29
    Then STDOUT should be a table containing rows:                                                   # features/steps/then.php:44
      | language | english_name            | status      |
      | ar       | Arabic                  | uninstalled |
      | az       | Azerbaijani             | uninstalled |
      | en_US    | English (United States) | active      |
      | en_GB    | English (UK)            | uninstalled |
    When I run `wp core language install en_GB`                                                      # features/steps/when.php:29
    And I run `wp core language install en_AU`                                                       # features/steps/when.php:29
    Then the wp-content/languages/admin-en_GB.po file should exist                                   # features/steps/then.php:149
    And the wp-content/languages/en_GB.po file should exist                                          # features/steps/then.php:149
    And the {SUITE_CACHE_DIR}/translation/core-default-([\d\.]+)-en_AU-([\d]+).zip file should exist # features/steps/then.php:149
      $ wp core language install en_AU
      Downloading translation from https://downloads.wordpress.org/translation/core/4.6.1/en_AU.zip...
      Unpacking the update...
      Installing the latest version...
      Translation updated successfully.
      Success: Language installed.

      cwd: /var/folders/vr/z6wqt2xs6ys87x5rl53_xc900000gn/T/wp-cli-test-run-5835a8892966f0.10018847/
      exit status: 0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ocean90 the file should exist step doesn't accept regex for the filename, afaik.

You could instead run grep on the translation cache dir to verify the files are there. Pseudo code:

When I run `ls {SUITE_CACHE_DIR}/translation | grep core-default-`
Then STDOUT should contain:
  """
  en_AU
  """
And STDOUT should contain:
  """
  en_GB
  """

@ocean90
Copy link
Copy Markdown
Contributor Author

ocean90 commented Nov 23, 2016

@danielbachhuber The output of the install command is now more verbose. Example:

      $ wp core language install en_AU
      Downloading translation from https://downloads.wordpress.org/translation/core/4.6.1/en_AU.zip...
      Unpacking the update...
      Installing the latest version...
      Translation updated successfully.
      Success: Language installed.

The upgrader is currently using the Automatic_Upgrader_Skin skin class. Using Utils\get_upgrader() forces the use of UpgraderSkin. I don't see that as an issue though. What are your thoughts?

@danielbachhuber
Copy link
Copy Markdown
Member

The output of the install command is now more verbose.

Great!

I don't see that as an issue though. What are your thoughts?

I don't see it as a problem either. In fact, it may be the eventual solution for #1501

@danielbachhuber danielbachhuber added this to the 1.0.0 milestone Nov 23, 2016
`file should exist` doesn't support regex.
This accommodates the now more verbose output of the commands due to the change of the upgrader skin.
@danielbachhuber
Copy link
Copy Markdown
Member

@ocean90 Happy with this?

@ocean90
Copy link
Copy Markdown
Contributor Author

ocean90 commented Nov 23, 2016

@danielbachhuber I'm happy if you are! :)

Code styling could be improved a bit, but this should probably be done in a separate PR because it affects CoreUpgrader too.

@danielbachhuber
Copy link
Copy Markdown
Member

I'm happy if you are! :)

Yep, looks good :)

Code styling could be improved a bit, but this should probably be done in a separate PR because it affects CoreUpgrader too.

Sounds good. I'm generally fine with code style changes as long as there's good test coverage around code being changed.

@danielbachhuber danielbachhuber merged commit 40e8b72 into wp-cli:master Nov 23, 2016
@ocean90 ocean90 deleted the cache-language-packs branch November 23, 2016 18:49
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.

2 participants