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

Added `cli cache prune` & `cli cache clear` commands #4787

Merged
merged 10 commits into from Oct 7, 2018

Conversation

3 participants
@swissspidy
Copy link
Contributor

swissspidy commented May 2, 2018

This aims to implement wp-cli/ideas#23.

Also related: wp-cli/ideas#96

To do:

  • Add tests
  • Perhaps add some options, e.g. to only remove cached translations. (=> in separate issue if actually needed)

swissspidy added some commits May 2, 2018

@swissspidy swissspidy requested a review from wp-cli/committers May 7, 2018

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Jun 21, 2018

I'd suggest doing Perhaps add some options, e.g. to only remove cached translations. in a follow-up ticket in case someone does actually find it useful. I'd guess that the need for such a thing is pretty small.

Show resolved Hide resolved features/cli-cache.feature
@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Jun 21, 2018

The command looks good to me, but the tests do not do much yet and don't verify the behavior at all.

swissspidy added some commits Aug 4, 2018

Don't delete cached files without timestamp when pruning
For instance, WordPress core downloads don't have timestamps and shouldn't be removed in that case.
@swissspidy

This comment has been minimized.

Copy link
Contributor

swissspidy commented Aug 4, 2018

I don't yet have a good way to test the wp cache prune command, at least not with the way I wrote the code (check for timestamp in file name, not actual file modification time). IIRC I did it that way to really make sure two files with different timestamps belong together. Perhaps I have to re-think that.

The only commands I could find that use the cache directory are the core command and the language command.

In the language command, the timestamp in the file name is dynamic:

https://github.com/wp-cli/language-command/blob/67cba55d96fd8eeda24dc3ac4ab554ec2102ebe6/src/WP_CLI/LanguagePackUpgrader.php#L58-L60

Plus, there's no reliable way to update a language pack in a test in order to have multiple versions.

It would be easier if I could create some fake files in the cache directory, but as far as I can see, the Given an xy.zip file: step creates files relative to the current directory and doesn't take absolute paths.

@wojsmol

This comment has been minimized.

Copy link
Contributor

wojsmol commented Aug 4, 2018

@swissspidy see
https://github.com/wp-cli/entity-command/blob/4cbd6081319b2f4bee8d9ec6bbfe6317eaef9385/features/bootstrap/FeatureContext.php#L63-L64

  • The directory that the WP-CLI cache (WP_CLI_CACHE_DIR, normally "$HOME/.wp-cli/cache") is set to on a "Given an empty cache" step.
  • Variable SUITE_CACHE_DIR. Lives until the end of the scenario (or until another "Given an empty cache" step within the scenario).
@swissspidy

This comment has been minimized.

Copy link
Contributor

swissspidy commented Aug 4, 2018

Thanks. I'm aware of {SUITE_CACHE_DIR} since @schlessera pointed it out to me and I actually use it already.

But I can't use Given an {SUITE_CACHE_DIR}/foo/xy.zip file: because that would use the wrong path:

https://github.com/wp-cli/wp-cli-tests/blob/90f71595d52bf20301961201e9d39bb190d5ddab/features/steps/given.php#L49-L50

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Aug 4, 2018

@swissspidy Keep in mind that we always add logic to FeatureContext.php.

So, just think of the English sentences that describe the logic of how to test. We can then add implementations as needed.

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Aug 23, 2018

I'm adding a new step Given a ... *cache* file to wp-cli/wp-cli-tests: wp-cli/wp-cli-tests#22

Tests can then be something like this:

  Scenario: Remove all but newest files from cache directory
    Given an empty cache
    And a file-a-12345.tmp cache file:
      """
      -empty-
      """
    And a file-a-23456.tmp cache file:
      """
      -empty-
      """
    And a file-b-12345.tmp cache file:
      """
      -empty-
      """
    And a file-b-23456.tmp cache file:
      """
      -empty-
      """
    And a file-b-01234.tmp cache file:
      """
      -empty-
      """
    And a file-c-12345.tmp cache file:
      """
      -empty-
      """

    When I run `wp cli cache prune`
    Then STDOUT should be:
      """
      Success: Cache pruned.
      """
    And the {SUITE_CACHE_DIR}/file-a-12345.tmp file should not exist
    And the {SUITE_CACHE_DIR}/file-a-23456.tmp file should exist
    And the {SUITE_CACHE_DIR}/file-b-12345.tmp file should not exist
    And the {SUITE_CACHE_DIR}/file-b-23456.tmp file should exist
    And the {SUITE_CACHE_DIR}/file-b-01234.tmp file should not exist
    And the {SUITE_CACHE_DIR}/file-c-12345.tmp file should exist
@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Aug 25, 2018

Given a ... *cache* file is now part of wp-cli/wp-cli-tests v2.0.10+.

swissspidy added some commits Sep 14, 2018

@swissspidy

This comment has been minimized.

Copy link
Contributor

swissspidy commented Sep 14, 2018

I'm weirdly getting PHP Fatal error: Uncaught Error: Call to undefined method WP_CLI\FileCache::prune() in php/commands/src/CLI_Cache_Command.php:61 when running wp cli cache prune in tests... No idea why.

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Sep 14, 2018

I think I can top that!

When running composer test -- features/cli-cache.feature, I get the following error in the above test:

Class 'features/cli-cache' could not be found in '/Users/alain/dev/wp-cli-dev/wp-cli/features/cli-cache.feature'.
@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Sep 14, 2018

Oh, nevermind the last comment, user error. The command should have used behat, not test. The error came from PHPUnit.

@schlessera schlessera added this to the 2.1.0 milestone Oct 5, 2018

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Oct 5, 2018

@swissspidy This is still marked as WIP. I got the tests to pass now. Is this ready for merging?

@swissspidy

This comment has been minimized.

Copy link
Contributor

swissspidy commented Oct 5, 2018

Nice! In this case I think we can merge this for now :-)

@schlessera schlessera changed the title [WIP] wp cli cache commands Added `cli cache prune` & `cli cache clear` commands Oct 7, 2018

@schlessera schlessera merged commit ca8bae8 into master Oct 7, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@schlessera schlessera deleted the cache-command branch Oct 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment