wp-cli / doctor-command

Diagnose problems within WordPress by running a series of checks for symptoms
MIT License
145 stars 22 forks source link

Add missing behat.yml file and update feature tests #190

Closed ernilambar closed 3 months ago

ernilambar commented 5 months ago

Fixes https://github.com/wp-cli/doctor-command/issues/189

ernilambar commented 5 months ago

@swissspidy I am trying to restore feature test here in this PR. I have fixed several broken tests but I am stuck in one test regarding core language.

Scenario: One language has an update available Ref: https://github.com/wp-cli/doctor-command/blob/main/features/check-language-update.feature#L19

This cat wp-content/languages/custom.po > wp-content/languages/ja.po does not seem to work now for triggering language update. I did some research but could not pin point the exact thing which WP uses to check for language update. Can you please suggest way to write test here?

Also in WP 3.7, there are PHP warnings in all scenarios. Any idea about that? https://github.com/wp-cli/doctor-command/actions/runs/9284069665/job/25545664214?pr=190

swissspidy commented 5 months ago

Scenario: One language has an update available Ref: main/features/check-language-update.feature#L19

This cat wp-content/languages/custom.po > wp-content/languages/ja.po does not seem to work now for triggering language update. I did some research but could not pin point the exact thing which WP uses to check for language update. Can you please suggest way to write test here?

IIRC it requires an .mo file as well, not only a .po file. Creating an empty file should suffice.

Also in WP 3.7, there are PHP warnings in all scenarios. Any idea about that? wp-cli/doctor-command/actions/runs/9284069665/job/25545664214?pr=190

Probably easiest to add some debug_print_backtrace in kses.php to see where it's being called from.

Weirdly I don't see an in_array call on line 1147 there in WP 3.7: https://github.com/WordPress/wordpress/blob/3.7.41/wp-includes/kses.php#L1147. But in other versions wp_kses_named_entities is around that line.

ernilambar commented 5 months ago

You looked at 3.7.41. There is in_array() check in version 3.7. Ref: https://github.com/WordPress/WordPress/blob/3.7/wp-includes/kses.php#L1147

Strangely we have not seen this issue in other repos. May be related to https://core.trac.wordpress.org/ticket/47357

ernilambar commented 5 months ago

Regarding language test, I copied test from language-command - https://github.com/wp-cli/language-command/blob/main/features/language-core.feature#L218

Was surprised to see this tag - @require-php-5.6 @less-than-php-7.0 for the update test. Why this check is not working in recent PHP version?

ernilambar commented 4 months ago

When WP_VERSION is set to 3.7 then exact version 3.7 should have been used, no? How did it went to 3.7.41. May be issue in https://github.com/wp-cli/wp-cli-tests ?

swissspidy commented 4 months ago

You looked at 3.7.41. There is in_array() check in version 3.7. Ref: WordPress/WordPress@3.7/wp-includes/kses.php#L1147

Ah thanks!

When WP_VERSION is set to 3.7 then exact version 3.7 should have been used, no? How did it went to 3.7.41. May be issue in wp-cli/wp-cli-tests ?

Yes it uses 3.7. 3.7.41 was just one of the versions I was looking at when I shared the link. Forget about it :)

Aside: there is already https://github.com/wp-cli/wp-cli-tests/issues/51

swissspidy commented 4 months ago

Regarding language test, I copied test from language-command - wp-cli/language-command@main/features/language-core.feature#L218

Was surprised to see this tag - @require-php-5.6 @less-than-php-7.0 for the update test. Why this check is not working in recent PHP version?

I don't know. Maybe it was added by mistake. I suggest adding a PR to remove both restrictions and then see what breaks :)

ernilambar commented 4 months ago

Regarding language test, I copied test from language-command - wp-cli/language-command@main/features/language-core.feature#L218 Was surprised to see this tag - @require-php-5.6 @less-than-php-7.0 for the update test. Why this check is not working in recent PHP version?

I don't know. Maybe it was added by mistake. I suggest adding a PR to remove both restrictions and then see what breaks :)

Looks like lower WP + higher PHP version generates lots of compatibility issues. May be such restriction was to avoid such issues.

ernilambar commented 4 months ago

In WP 3.7, value of $allowedentitynames global variable is always null. In this command, we are booting WordPress ourself. There must be something missing which has made these global variables null. And I am getting hard time understanding this. 😓

ernilambar commented 4 months ago

@wp-cli/committers Any idea about failing test in WP 3.7? Ref - https://github.com/wp-cli/doctor-command/actions/runs/9348877593/job/25728974652?pr=190

swissspidy commented 3 months ago

No idea, and unfortunately I can't easily set up 3.7 on my machine at the moment.

To unblock other PRs, we could skip that test for 3.7 for now & open a new issue to look into it. WDYT?

ernilambar commented 3 months ago

No idea, and unfortunately I can't easily set up 3.7 on my machine at the moment.

To unblock other PRs, we could skip that test for 3.7 for now & open a new issue to look into it. WDYT?

Any way to completely exclude test for 3.7? Coz lots of scenarios are failing in this version?

swissspidy commented 3 months ago

Not yet, but this would be a great addition for our wp-cli/.github repo!

We support specifying a minimum PHP version here:

https://github.com/wp-cli/.github/blob/3ec3e2825d5fec4cf9ce89f9550e26f9d92b787f/.github/workflows/reusable-testing.yml#L6-L10

We could support a minimum WP version as well. Right now we only test 3.7, 6.2, latest, and trunk though. So if someone does e.g minimum-wp: 4.8, we should not test 3.7, but test 4.8 instead. Makes sense?

ernilambar commented 3 months ago

@swissspidy PR https://github.com/wp-cli/.github/pull/101 Not sure what this is you are talking about as I am completely new about this jq stuff. 😊

ernilambar commented 3 months ago

@swissspidy Test matrix fix seems to be working but there are few new failures from the last time.

swissspidy commented 3 months ago

Trunk tests are failing because the core themes were updated yesterday in trunk, but not yet in the theme directory. This will happen later today.