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

Add list of files to report of 'wp doctor check cache-flush' command #191

Merged
merged 6 commits into from
Jul 18, 2024

Conversation

kiranpotphode
Copy link
Contributor

@kiranpotphode kiranpotphode commented Jun 19, 2024

Fixes #184

This PR adds enhancement for the wp doctor check cache-flush command as mentioned in issue #184

The output will show the list of files where wp_cache_flush() is used.

@kiranpotphode kiranpotphode changed the title feat: add list of files to report of wp_cache_flush function usage Add list of files to report of 'wp doctor check cache-flush' command Jun 19, 2024
@kiranpotphode kiranpotphode marked this pull request as ready for review June 20, 2024 04:41
@kiranpotphode kiranpotphode requested a review from a team as a code owner June 20, 2024 04:41
@wojtekn
Copy link

wojtekn commented Jul 15, 2024

@kiranpotphode, thanks for adjusting the output format, it's now more readable.

I've just checked tests and found out we will need to update those as they rely on the old message format:

composer behat -- features/check-cache-flush.feature

@kiranpotphode
Copy link
Contributor Author

@kiranpotphode , thanks for adjusting the output format, it's now more readable.

I've just checked tests and found out we will need to update those as they rely on the old message format:

composer behat -- features/check-cache-flush.feature

@wojtekn Thanks for pointing that out.
I have updated the test now. I also updated the doc block comment for the class.

Copy link

@wojtekn wojtekn left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for updating the test!

@ernilambar
Copy link
Member

@kiranpotphode Can you please rebase the PR with main branch? Now behat.yml file has been added. So GitHub actions will run with all functional tests.

@kiranpotphode
Copy link
Contributor Author

@kiranpotphode (https://profiles.wordpress.org/kiranpotphode) Can you please rebase the PR with main branch? Now behat.yml file has been added. So GitHub actions will run with all functional tests.

@ernilambar
I have updated my branch with the latest code from the main branch.
However, I noticed that some tests are failing on GitHub actions.

But my tests on local are passing for the following command.
composer behat -- features/check-cache-flush.feature

Can you guide what might the problem?

@ernilambar
Copy link
Member

ernilambar commented Jul 17, 2024

@kiranpotphode Yah, I noticed. wp_cache_flush() is there in sqlite-database-integration which is used for Sqlite implementation. Probably you have tested with mysql in your setup. That is why you are not seeing error in local.

Instead of STDOUT should be a table containing rows:, we could use STDOUT should contain:

When I run `wp doctor check cache-flush`
Then STDOUT should contain:
  """
  cache-flush
  """
And STDOUT should contain:
  """
  warning
  """
And STDOUT should contain:
  """
  Use of wp_cache_flush() detected
  """
And STDOUT should contain:
  """
  mu-plugins/plugin.php
  """
  

@kiranpotphode
Copy link
Contributor Author

I have updated the tests. Now wp_cache_flush() related issues are fixed.

Still noticing some other failed tests for WP trunk on PHP 8.4 Scenario though.

@ernilambar
Copy link
Member

@kiranpotphode WP is still not compatible to PHP 8.4. So we can ignore that now.

@ernilambar ernilambar added this to the 2.2.0 milestone Jul 18, 2024
@ernilambar ernilambar merged commit 0a7eb00 into wp-cli:main Jul 18, 2024
32 of 34 checks passed
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.

wp_cache_flush warning without explanation. Happens with checksums verifier also
3 participants