Make WordPress Core

Opened 10 months ago

Last modified 5 weeks ago

#59390 assigned defect (bug)

Incorrect method / function args used in bin/tests/core/tests/rest-api/wpRestMenuItemsController.php

Reported by: davidbinda's profile david.binda Owned by: hellofromtonya's profile hellofromTonya
Milestone: Future Release Priority: normal
Severity: normal Version: 5.9
Component: Menus Keywords: has-patch changes-requested
Focuses: rest-api Cc:


The Tests_REST_WpRestMenuItemsController::check_get_menu_item_response method accepts 2 arguments, however 3 are passed to the method in

It feels like the Tests_REST_WpRestMenuItemsController::check_menu_item_data method was meant to be used here.

Attachments (1)

59390.diff (702 bytes) - added by david.binda 10 months ago.

Download all attachments as: .zip

Change History (13)

10 months ago

#1 @SergeyBiryukov
10 months ago

  • Component changed from General to Menus
  • Focuses rest-api added
  • Milestone changed from Awaiting Review to 6.4
  • Version set to 5.9

Good catch, introduced in [52079] / #40878.

#2 @mukesh27
10 months ago

  • Keywords has-patch added

@SergeyBiryukov, you beat me to it! I was just about to add the ticket and changeset entry for when it was introduced. 😅

Last edited 10 months ago by mukesh27 (previous) (diff)

#3 @nicolefurlan
10 months ago

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested:


  • OS: macOS 13.4.1
  • PHP: 8.2.11
  • WordPress: trunk

Actual Results

  • ✅ Issue resolved with patch.

Additional Notes

  • PHPUnit tests continue to pass after applying the patch.

#4 @nicolefurlan
10 months ago

With just over a week left until 6.4 RC1, do we think this ticket will make it? I submitted a passing test report last week – do we think this ticket is ready for commit?

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

9 months ago

#6 @oglekler
9 months ago

@audrasjb you are this component maintainer, please take a look, it should be ready for commit.

#7 @SergeyBiryukov
9 months ago

  • Keywords changes-requested added

Thanks for the patch!

I'm a bit confused here. The Tests_REST_WpRestMenuItemsController::check_menu_item_data() method requires four arguments, so unless I'm missing something, passing only three would cause a fatal error, and the test only passes accidentally because that branch is never hit.

This could use some more investigation.

#8 @nicolefurlan
9 months ago

I think you're absolutely right @SergeyBiryukov.

@davidbinda could you take a look at

@audrasjb pinging you as well as the component maintainer to see if you have any input on this.

#9 @oglekler
9 months ago

  • Milestone changed from 6.4 to 6.5

Better safe than sorry, I am moving this to the next milestone to get everyone time to check the patch properly.

#10 @swissspidy
5 months ago

  • Milestone changed from 6.5 to 6.6

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

6 weeks ago

#12 @hellofromTonya
5 weeks ago

  • Milestone changed from 6.6 to Future Release
  • Owner set to hellofromTonya
  • Status changed from new to assigned

Hmm, this one is not a straightforward swap. It will need investigation to go back into the PR/patch history to determine the intent of the parent check.

I've added to my TODO list. Moving it out of the 6.6 milestone, for now, as fixing a test is not bound to a milestone. As soon as there's a clear understanding, the fix can be committed.

Note: See TracTickets for help on using tickets.