Make WordPress Core

Opened 23 months ago

Last modified 21 months ago

#53673 assigned enhancement

Add unit tests for v5.8 widget sidebar IDs

Reported by: jeffpaul's profile JeffPaul Owned by: hellofromtonya's profile hellofromTonya
Milestone: Future Release Priority: normal
Severity: normal Version: 5.8
Component: REST API Keywords: has-patch has-unit-tests 2nd-opinion
Focuses: administration, rest-api Cc:

Description

r51239 was included in 5.8, but was intended to have included unit tests as part of the release. See parent ticket as possible starting point for a unit test patch on this.

Change History (7)

#1 @hellofromTonya
23 months ago

  • Owner set to hellofromTonya
  • Status changed from new to assigned

Adding to my TODO list.

#3 in reply to: ↑ 2 @hellofromTonya
23 months ago

Replying to strategio:

Here's the unit test proposed in the parent ticket (https://core.trac.wordpress.org/ticket/53452#comment:13):

https://github.com/WordPress/wordpress-develop/pull/1448

Hey @strategio thanks for copying the link on this ticket. Status: I'll do a deep code review of your PR and, if needed, offer guidance. I suspect it'll be next week as my focus now is on 5.8.

#4 @hellofromTonya
23 months ago

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

Updating the keywords to identify that there is a PR ready for code review.

#5 @desrosj
23 months ago

To copy my thoughts over on the current patch:

The test looks good to me, though I'm not sure about where the test class actually belongs.

It looks like we have a file in DIR_TESTDATA with some widgets, but there are only 2 and they are all tied to a specific data provider. I've also been trying to think of a way to refactor out the need for the class at all, but nothing comes to mind right now.

I'm looking for some guidance/suggestions from contributors with more knowledge about why the suite is organized how it is.

#6 follow-up: @desrosj
21 months ago

  • Keywords 2nd-opinion added
  • Milestone changed from 5.8.1 to Future Release

This has not received any attention the last few months. Going to bump to Future Release for now. @hellofromTonya is this still in your to-do list?

#7 in reply to: ↑ 6 @hellofromTonya
21 months ago

Replying to desrosj:

This has not received any attention the last few months. Going to bump to Future Release for now. @hellofromTonya is this still in your to-do list?

It is on my TODO list, though further down the list. I'll pull this one back into a milestone as soon as I get some free time to move it forward towards resolution.

Note: See TracTickets for help on using tickets.