Discovered over in #2477337: Saving a menu item with internal path instead of entity reference loses hierarchy. The tests within doTestMenuHierarchy have not been getting run since #2104123: Consolidate test methods in MenuRouterTest.

CommentFileSizeAuthor
#3 menu-hierarchy-tests-2479819-03.patch1.73 KBjhedstrom
#3 interdiff.txt1.67 KBjhedstrom
#1 menu-hierarchy-tests-2479819-01.patch2.34 KBjhedstrom

Comments

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Active » Needs review
StatusFileSize
new menu-hierarchy-tests-2479819-01.patch2.34 KB

This gets all tests passing but one, and I'm not immediately clear if the failing test is actually correct in its assumption.

Status: Needs review » Needs work

The last submitted patch, 1: menu-hierarchy-tests-2479819-01.patch, failed testing.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt1.67 KB
new menu-hierarchy-tests-2479819-03.patch1.73 KB

This gets the tests green, but fundamentally changes the nature of the last assertion (previously looking for the top-level parent, now it looks for the immediate parent).

grendzy’s picture

grendzy commented
Status: Needs review » Reviewed & tested by the community

Yes, I agree that MenuLinkInterface::getParent is intended to return the immediate parent. RTBC!

xjm’s picture

xjm
she/her
Primary language English
commented
Priority: Major » Normal
Status: Reviewed & tested by the community » Fixed

Nice find! And thank you for documenting when the regression was introduced; definitely helpful for reviewing this. This issue only improves test coverage, so per https://www.drupal.org/core/beta-changes, this can be completed any time during the Drupal 8 beta phase. Committed and pushed to 8.0.x.

  • xjm committed 02ffc9a on 8.0.x 
    Issue #2479819 by jhedstrom, grendzy: Menu hierarchy tests are not being...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.