WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 8 months ago

#38540 closed defect (bug) (fixed)

Add PHP unit tests for theme starter content

Reported by: westonruter Owned by: westonruter
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: Customize Keywords: needs-patch
Focuses: Cc:

Description

Needing tests:

  • \WP_Customize_Manager::import_theme_starter_content()
  • \get_theme_starter_content()
  • The clearing of the fresh_site flag.

See #38114, #38533.

Attachments (4)

38540.diff (1.3 KB) - added by welcher 9 months ago.
38540.2.diff (9.2 KB) - added by welcher 8 months ago.
Patch with tests for get_theme_starter_content()
38540.3.diff (6.1 KB) - added by westonruter 8 months ago.
38540.4.diff (10.3 KB) - added by westonruter 8 months ago.

Download all attachments as: .zip

Change History (17)

This ticket was mentioned in Slack in #core by helen. View the logs.


9 months ago

#2 @welcher
9 months ago

I'm writing up some tests for \get_theme_starter_content and noticed that it is possible to do this add_theme_support( 'starter-content' ) which causes a PHP invalid foreach error.

I am not sure how common that would be but there is no check in get_theme_starter_content to ensure that $theme_support is an array. I've attached a patch with tests to address this and introduce a new test file for the method.

Last edited 9 months ago by welcher (previous) (diff)

@welcher
9 months ago

This ticket was mentioned in Slack in #core by welcher. View the logs.


9 months ago

#4 @welcher
9 months ago

  • Keywords has-patch added; needs-patch removed

#5 @westonruter
9 months ago

@welcher yeah, you're right. If $config is anything other than an array then it should be overridden to be an empty array.

#6 @welcher
9 months ago

@westonruter the patch I added addresses the issue - should that get committed before the tests are completed? I can also add a separate patch for just the tests if that makes sense as well.

#7 @westonruter
9 months ago

@welcher you can include that fix with your unity tests patch.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


9 months ago

#9 follow-up: @westonruter
8 months ago

@welcher are you going to flesh out the unit tests? Note that some key fixes were done as part of #38541.

#10 in reply to: ↑ 9 @welcher
8 months ago

Replying to westonruter:

@welcher are you going to flesh out the unit tests? Note that some key fixes were done as part of #38541.

@westonruter I am working on updating my tests for reflect the key changes.

@welcher
8 months ago

Patch with tests for get_theme_starter_content()

#11 @welcher
8 months ago

@westonruter I've added some tests for the default content that is generated. The patch also contains the fix if for not passing the second parameter to add_theme_support.

I'm hoping to keep working on the tests for the other items in this ticket but time is in short supply lately so if there is anyone that wants to jump in and help that would be awesome.

#12 @westonruter
8 months ago

  • Keywords needs-patch added; has-patch removed
  • Owner set to westonruter
  • Status changed from new to accepted

@westonruter
8 months ago

@westonruter
8 months ago

#13 @westonruter
8 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 39276:

Customize: Add unit tests for importing theme starter content.

Props welcher, westonruter.
See #38114, #38533, #38615.
Fixes #38540.

Note: See TracTickets for help on using tickets.