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 PHPUnit tests workflow for multisite #7321

Merged
merged 38 commits into from
Nov 24, 2022

Conversation

thelovekesh
Copy link
Collaborator

@thelovekesh thelovekesh commented Nov 1, 2022

Summary

Fixes #5701

This PR also updates deprecated set-output commands in GitHub Actions workflow files. See https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@thelovekesh thelovekesh force-pushed the add/multisite-phpunit-workflow branch from 8f6424d to b8007c0 Compare November 1, 2022 05:22
@thelovekesh thelovekesh force-pushed the add/multisite-phpunit-workflow branch from b8007c0 to 8997b11 Compare November 1, 2022 06:22
@thelovekesh thelovekesh force-pushed the add/multisite-phpunit-workflow branch from 0780fd4 to 15fa7dd Compare November 1, 2022 06:59
@thelovekesh thelovekesh force-pushed the add/multisite-phpunit-workflow branch from 15fa7dd to ac70da5 Compare November 1, 2022 07:00
@thelovekesh
Copy link
Collaborator Author

@westonruter I have configured workflow for both testsuites i.e. default and external-http. Some tests seem to be failing mainly from helper functions. Should I fix them in the same PR? Can you please take a look if any test is need to be skipped?

@westonruter
Copy link
Member

Interesting. Yes, they should be fixed in this PR. For each failure, I'd suggest checking to see what the cause of the failure is. It may be due to a regular admin user not having all the same privileges as a superadmin on multisite. Therefore the fix may be just to check if its multisite, and then grant the user being tested with superadmin privileges.

Some tests probably also should be skipped if the functionality specifically is not available in multisite. For example, installing themes from the onboarding wizard I don't think is available on multisite.

It'll be interesting to see if this uncovers actual bugs with the code and not just incomplete tests!

@thelovekesh thelovekesh force-pushed the add/multisite-phpunit-workflow branch from 5341d82 to b84213d Compare November 21, 2022 13:42
@thelovekesh
Copy link
Collaborator Author

@westonruter I believe the capability here should be install_themes. Thoughts?

if ( current_user_can( 'switch_themes' ) ) {

@thelovekesh thelovekesh force-pushed the add/multisite-phpunit-workflow branch from 4748f1f to 4dc01a4 Compare November 23, 2022 04:44
@thelovekesh thelovekesh force-pushed the add/multisite-phpunit-workflow branch from 5c956d8 to 88fb246 Compare November 23, 2022 07:02
@thelovekesh thelovekesh force-pushed the add/multisite-phpunit-workflow branch from 5e64f53 to deb6e3c Compare November 23, 2022 07:47
@codecov
Copy link

codecov bot commented Nov 23, 2022

Codecov Report

Merging #7321 (deb6e3c) into develop (9f6567c) will decrease coverage by 0.80%.
The diff coverage is n/a.

❗ Current head deb6e3c differs from pull request most recent head 17dd543. Consider uploading reports for the commit 17dd543 to get more accurate results

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #7321      +/-   ##
=============================================
- Coverage      78.60%   77.80%   -0.81%     
  Complexity      6852     6852              
=============================================
  Files            209      276      +67     
  Lines          18655    19814    +1159     
=============================================
+ Hits           14664    15416     +752     
- Misses          3991     4398     +407     
Flag Coverage Δ
javascript 64.88% <ø> (?)
php 78.60% <ø> (ø)
unit 78.60% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
assets/src/components/conditional-details/index.js 100.00% <0.00%> (ø)
assets/src/common/helpers/index.js 91.22% <0.00%> (ø)
.../src/onboarding-wizard/components/stepper/index.js 66.66% <0.00%> (ø)
...s/amp-validation-status/revalidate-notification.js 100.00% <0.00%> (ø)
assets/src/block-editor/constants.js 100.00% <0.00%> (ø)
assets/src/components/amp-drawer/index.js 73.07% <0.00%> (ø)
assets/src/components/amp-admin-notice/index.js 100.00% <0.00%> (ø)
...sets/src/block-validation/components/icon/index.js 100.00% <0.00%> (ø)
assets/src/components/dev-tools-toggle/index.js 80.00% <0.00%> (ø)
...ts/src/common/helpers/test/fixtures/mockClasses.js 100.00% <0.00%> (ø)
... and 57 more

@thelovekesh thelovekesh force-pushed the add/multisite-phpunit-workflow branch from 17dd543 to deb6e3c Compare November 23, 2022 14:12
@westonruter
Copy link
Member

I believe the capability here should be install_themes. Thoughts?

@thelovekesh yes, I think you're right. The page requires install_themes: https://github.com/WordPress/wordpress-develop/blob/04f55a430e2ee5a88e9b5b5485b75a68fabd5a83/src/wp-admin/theme-install.php#L15-L17

@westonruter westonruter merged commit 8c8564e into develop Nov 24, 2022
@westonruter westonruter deleted the add/multisite-phpunit-workflow branch November 24, 2022 05:33
@westonruter westonruter added this to the v2.3.1 milestone Nov 24, 2022
@westonruter westonruter modified the milestones: v2.3.1, v2.4 Jan 26, 2023
@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add PHPUnit test workflow for multisite
2 participants