Problem/Motivation

Hit what looks to be a straight up bug in main while porting tome to D12 - this route provider in kernel tests has a return type of array https://git.drupalcode.org/project/drupal/-/blob/main/core/tests/Drupal/... but the inner provider returns an iterator https://git.drupalcode.org/project/drupal/-/blob/main/core/lib/Drupal/Co...

Failing build in tome https://git.drupalcode.org/project/tome/-/jobs/10863619

Steps to reproduce

Proposed resolution

Fix the return type

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3610170

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

larowlan created an issue. See original summary.

larowlan’s picture

Status: Active » Needs review

Not sure if this warrants a test, its test code where the bug is

godotislate’s picture

This seems fine, though I assume the code in question is not used in any core tests, otherwise an error would have surfaced by now.

One q: RouteProviderInterface is documented like this:

/**
   * Returns all the routes on the system.
   *
   * Usage of this method is discouraged for performance reasons. If possible,
   * use RouteProviderInterface::getRoutesByNames() or
   * RouteProviderInterface::getRoutesByPattern() instead.
   *
   * @return \Symfony\Component\Routing\Route[]
   *   An iterator of routes keyed by route name.
   */
  public function getAllRoutes();

We should probably update that doc, either by replacing the array return type with iterator if it's safe, or maybe making it array or iterator. Can probably be done in a follow up though?

larowlan’s picture

Status: Needs review » Needs work

You're right - I think we should fix that here as those docs are probably why we ended up with this bug
We have a kernel test for the concrete route provider so I can add a new method that exercises the get all method so we at least have some calls/coverage

larowlan’s picture

Status: Needs work » Needs review
larowlan’s picture

Test only job passes because the actual bug is in a test folder, so that doesn't get reverted

godotislate’s picture

Status: Needs review » Reviewed & tested by the community

lgtm

  • catch committed d90bbfbe on main
    fix: #3610170 Incorrect return type on kernel test route provider
    
    By:...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to main, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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