Inconsistent behaviour with drupalGet() when site is installed in a subdirectory

Adding more info to the summary.

Created an MR with the commit from https://git.drupalcode.org/project/drupal/-/merge_requests/6047/commits, which theoretically should have worked but didn't seem to.

One thing I wonder here is:

Do we even want node/1 to be aliased when we pass it to ::drupalGet()? What if I really want to hit the unaliased path? Why not require people to generate the alias before passing it in if they want that behaviour?

@catch - it's all a bit tricky but we rely on code in \Drupal\Tests\UiHelperTrait::buildUrl() to add the base directory and language... see:

      // Strip $base_path, if existent.
      $length = strlen($base_path);
      if (substr($path, 0, $length) === $base_path) {
        $path = substr($path, $length);
      }

      $force_internal = isset($options['external']) && $options['external'] == FALSE;
      if (!$force_internal && UrlHelper::isExternal($path)) {
        return Url::fromUri($path, $options)->toString();
      }
      else {
        $uri = $path === '<front>' ? 'base:/' : 'base:/' . $path;
        // Path processing is needed for language prefixing.  Skip it when a
        // path that may look like an external URL is being used as internal.
        $options['path_processing'] = !$force_internal;
        return Url::fromUri($uri, $options)
          ->setAbsolute()
          ->toString();

Changing this behaviour would be extremely tricky.

I think the current different behaviour for sites with and without a base directory is wrong and should be consistent but I think buildUrl should continue to:
1. Add the base directory
2. And do path processing.

I think we could improve things by making it only set the path_processing option if it is not set. Thereby allowing callers to take control of this.

The thing I don't get is why adding

      if (strlen($path) > 1) {
        $path = ltrim($path, '/');
      }

didn't actually fix OpenTelemetryNodePagePerformanceTest::testNodePageHotCache() - v confused about that.

Yes I'm also very confused about that. Maybe we should do some dump() debugging on gitlab to make sure it's really hitting the URLs we think it is with that change.

Ah I see why it did not work. The code was added in the wrong place. Fixed on the MR attached here. Let's see if something else then fails... this will be interesting.

Well, that would explain it.

There is a theoretical risk that tests which previously passed on gitlab ci (and hadn't been run locally for ages) could have started failing with this if they were relying on the non-aliased URL being hit, but since zero core tests are, this is probably fine.

I guess remaining question is whether we look at allowing non-aliased requests per #6 here or open another follow-up for that.

I would also revert the performance test changes here since they may as well be consistent even if we can make them pass without it.

Opened a new branch for a test-only change because all the changes are in the test system so the magic test-only job doesn't work.

Tests fail as expected... https://git.drupalcode.org/project/drupal/-/pipelines/146418/test_report...

Updated issue summary slightly to include solution and which MR should be committed

The change of stripping the forward slash seems straight forward.

Test-only MR shows coverage

I didn't do anything useful here so I think I can commit it.

Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks!

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