Add test coverage for rebuild.php

Check out my comment here: https://www.drupal.org/node/2380389#comment-10376025

rebuild.php needs more than tests, IMO, so that we can rely on behavior from different systems that need to deal with the cache.

Just the first step. Tests pending.

while debugging RebuildScriptTest.php and found that cURL used to handle URLs with a fragment by including fragment in the request to the server which has been fixed in cURL 7.20 so I have reverted https://www.drupal.org/node/671520 in this patch. It could be separate out in different issue if removal of more than 5 year code is out of scope in this issue.

tstoeckler and I were looking at it together. We couldn't find any instance of cURL handling fragments in the URL. We thought about it and we are considering to remove the cURL redirect handling from core and revert #11patch and re-add custom handling of fragments in Drupal way.

tstoeckler and I were looking at it together. We couldn't find any instance of cURL handling fragments in the URL. We thought about it and we are considering to remove the cURL redirect handling from core and revert #11patch and re-add custom handling of fragments in Drupal way.

Worked off of #6patch and posting what I have so far.

This is a big deal. We almost committed the wrong class load path in #2380389: Use a single vendor directory in the root due to lacking this test coverage.

Patch still applies, the test runs and passes. Just one thing...

+++ b/core/modules/system/src/Tests/Update/RebuildScriptTest.php
@@ -0,0 +1,45 @@
+    $cache = \Drupal::cache();

Should be $this->container->get('cache.default');, fixable on commit, or ignorable given that we're not consistently using the fixture anyway.

Re-running the test because it's unclear whether the failing test was unrelated.

+++ b/core/modules/system/src/Tests/Update/RebuildScriptTest.php
@@ -0,0 +1,45 @@
+class RebuildScriptTest extends WebTestBase {

Needs to not be a WebTestBase test.

Patch for 8.6.x and fixed #22

oops, forgot to change version.
Adding again. Patch for 8.6.x and fixed #22

+++ b/core/modules/system/src/Tests/Update/RebuildScriptTest.php
@@ -0,0 +1,45 @@
+    $this->drupalGet(Url::fromUri('base:core/rebuild.php'));
...
+    $this->drupalGet(Url::fromUri('base:core/rebuild.php'));

+++ b/core/rebuild.php
@@ -54,4 +54,4 @@
-header('Location: ' . $base_path);
+header('Location: ' . $request->getSchemeAndHttpHost() . $base_path);

not all cases covered

missing schemaAndHost

+++ b/core/modules/system/src/Tests/Update/RebuildScriptTest.php
@@ -0,0 +1,45 @@
+class RebuildScriptTest extends WebTestBase {

Let's directly with extensing BrowserTestBase. Please ask if you need some help.

Setting to needs work based on #36.

Patch still applies, re-running test.

Test in #41 tests these things:

• That caches aren't cleared without the rebuild_access permission.
• That caches are cleared with the rebuild_access permission.
• That rebuild.php redirects to the front page.

It'd be great if it had inline comments to say all that. But that's a nit.

Re: #19 - I'm not sure how we'd test for using the wrong autoload.php file, other than token analysis. That seems like a long way to go for something we don't do on other scripts.

And, for the record, I ran the test in #41 after modifying rebuild.php to require vendor/autoload.php. It passed.

RTBC here so we can have some coverage rather than perfect coverage.

Committed and pushed a3270cd3d1 to 8.7.x. Thanks!

I agree with @Mile23 some test coverage is better than none.