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 missing behat.yml file and update feature tests #190

Merged
merged 6 commits into from
Jul 16, 2024
Merged

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

merged 6 commits into from
Jul 16, 2024

Conversation

ernilambar
Copy link
Member

@ernilambar ernilambar commented May 28, 2024

Fixes #189

@ernilambar ernilambar changed the title Add missing behat.yml file Add missing behat.yml file and update feature tests May 28, 2024
@ernilambar
Copy link
Member Author

ernilambar commented May 29, 2024

@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
Copy link
Member

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
Copy link
Member Author

ernilambar commented May 31, 2024

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member

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 wp-cli/wp-cli-tests#51

@swissspidy
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

@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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member Author

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

@ernilambar ernilambar marked this pull request as ready for review July 16, 2024 11:35
@ernilambar ernilambar requested a review from a team as a code owner July 16, 2024 11:35
@ernilambar
Copy link
Member Author

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

@swissspidy
Copy link
Member

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.

Copy link
Member

@swissspidy swissspidy left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Let's rerun the tests once the default themes are properly released today after the 6.6 release

@ernilambar ernilambar added this to the 2.2.0 milestone Jul 16, 2024
@ernilambar ernilambar merged commit a5ac769 into wp-cli:main Jul 16, 2024
36 of 38 checks passed
@ernilambar ernilambar deleted the add-behat-file branch July 17, 2024 05:04
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.

Feature tests are not running in Actions
2 participants