Testing testbot

Pulling over the patch from https://drupal.org/node/1486960#comment-5772004

Testing the over-all effects of the changes.

OK, I see what I'm doing wrong, I'm not diffing this code with Drupal 8 HEAD. This patch is my first attempt to do this kind of a patch.

That's what I get for trying to use format-patch. Here's a regular git diff

Thanks pifr for finding that. sorry I missed that. Hopefully this one passes.

this one should be smaller and effect fewer files. It's curious to see $_GET['q'] be replaced by current_path() since current_path() returns $_GET['q']. Is this a mechanism to replace how the current path is retrieved in the future? should I spread the change of $_GET['q'] to current_path() to every part of drupal?

forgot patch

testing a hypothesis, the only difference between this patch and 45 is that I've change 12 instances of $_GET['q'] in menu.inc and common.inc to current_path().

Testing how some changes perform.

Once again, a testbot breaker ... or at least, a testbot plugger ...

ID 	                Message 	                                        Timestamp
3,648,263 	Requested by test client #664. 	20 min 44 sec ago
3,647,833 	Test reset by client request. 	        47 min 29 sec ago
3,647,733 	Requested by test client #699. 	1 hour 10 min ago
3,647,728 	Test reset by client request. 	        1 hour 10 min ago
3,647,683 	Requested by test client #699. 	1 hour 33 min ago
3,647,678 	Test reset by client request. 	        1 hour 33 min ago 

Hopefully these can shed some light on things ... This is just a select excerpt; the first one is the most common by about 5 times.

[Sun Apr 29 06:31:08 2012] [error] [client 10.20.0.108] Uncaught PHP Exception Symfony\\Component\\HttpKernel\\Exception\\AccessDeniedHttpException: "" at /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php line 39

[Sun Apr 29 06:31:14 2012] [error] [client 10.20.0.108] Uncaught PHP Exception Symfony\\Component\\HttpKernel\\Exception\\NotFoundHttpException: "No route found for "GET /aggregator/sources/1"" at /var/lib/drupaltestbot/sites/default/files/checkout/core/vendor/Symfony/Component/HttpKernel/EventListener/RouterListener.php line 69

[Sun Apr 29 06:31:38 2012] [error] [client 10.20.0.108] Uncaught PHP Exception Symfony\\Component\\HttpKernel\\Exception\\NotFoundHttpException: "No route found for "GET /en/admin/config/regional/language"" at /var/lib/drupaltestbot/sites/default/files/checkout/core/vendor/Symfony/Component/HttpKernel/EventListener/RouterListener.php line 69

[Sun Apr 29 06:31:38 2012] [error] [client 10.20.0.108] Uncaught PHP Exception Symfony\\Component\\HttpKernel\\Exception\\NotFoundHttpException: "No route found for "GET /en"" at /var/lib/drupaltestbot/sites/default/files/checkout/core/vendor/Symfony/Component/HttpKernel/EventListener/RouterListener.php line 69

[Sun Apr 29 06:32:06 2012] [error] [client 10.20.0.108] Uncaught PHP Exception Symfony\\Component\\HttpKernel\\Exception\\NotFoundHttpException: "" at /var/lib/drupaltestbot/sites/default/files/checkout/core/includes/common.inc line 717

[Sun Apr 29 06:33:14 2012] [error] [client 10.20.0.108] Uncaught PHP Exception Symfony\\Component\\HttpKernel\\Exception\\NotFoundHttpException: "No route found for "GET /pagedoesnotexist"" at /var/lib/drupaltestbot/sites/default/files/checkout/core/vendor/Symfony/Component/HttpKernel/EventListener/RouterListener.php line 69

[Sun Apr 29 06:47:05 2012] [error] [client 10.20.0.108] Uncaught PHP Exception Exception: "Drupal is awesome" at /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/system/tests/modules/error_test/error_test.module line 56

[Sun Apr 29 06:47:05 2012] [error] [client 10.20.0.108] Uncaught PHP Exception Drupal\\Core\\Database\\DatabaseExceptionWrapper: "SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupaltestbotmysql.bananas_are_awesome' doesn't exist: SELECT * FROM bananas_are_awesome; Array\n(\n)\n" at /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Connection.php line 533

[Sun Apr 29 06:47:07 2012] [error] [client 10.20.0.108] PHP Fatal error:  Exception thrown without a stack frame in Unknown on line 0

As long as testbots aren't shielded against this attack ;)

the best way to avoid taking them down is to run at least one of the failing test cases locally.

---
That said, since the PSR-0/classloader changes in D8, my local PHP 5.3.6 frequently manages to freeze and take down my local Apache server on fatal or more severe exceptions. Perhaps something similar is happening on testbots? At least I've the impression something odd is going on in our codebase.

This appears to be the showstopper:

PHP Fatal error:  Exception thrown without a stack frame in Unknown on line 0
exception 'PDOException' with message 'SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'variable_init' for key 'PRIMARY'' in /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Statement.php:58
Stack trace:
#0 /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Statement.php(58): PDOStatement->execute(Array)
#1 /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Connection.php(506): Drupal\Core\Database\Statement->execute(Array, Array)
#2 /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Driver/mysql/Insert.php(34): Drupal\Core\Database\Connection->query('INSERT INTO {se...', Array, Array)
#3 /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Lock/DatabaseLockBackend.php(50): Drupal\Core\Database\Driver\mysql\Insert->execute()
#4 /var/lib/drupaltestbot/sites/default/files/checkout/core/includes/lock.inc(104): Drupal\Core\Lock\DatabaseLockBackend->acquire('variable_init', 1)
#5 /var/lib/drupaltestbot/sites/default/files/checkout/core/includes/bootstrap.inc(834): lock_acquire('variable_init', 1)
#6 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/simpletest/drupal_web_test_case.php(1527): variable_initialize()
#7 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/simpletest/drupal_web_test_case.php(1582): DrupalWebTestCase->refreshVariables()
#8 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/simpletest/drupal_web_test_case.php(529): DrupalWebTestCase->tearDown()
#9 /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh(369): DrupalTestCase->run()
#10 /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh(22): simpletest_script_run_one_test('1', 'ContactSitewide...')
#11 {main}

Searched through the code and found 5 or 6 instances where the frontpage settings were handled by variable_get, variable_set.
Fixed those and added some more debugging information. (which I will take out if this goes green)

woops wrong issue, sorry

#67: 1463656-drupal-kernel.patch queued for re-testing.

This is #71 plus language negotiation fixes. Depending on test results, I'll commit the interdiff to a suitable branch.

#74: 1486960-kernel-testbot-74.patch queued for re-testing.

#74 changes some bootstrap logic that affects all non-index.php scripts, like the bot runner, which is why it ran 0 tests. This adds a temporary _drupal_bootstrap_legacy() function to work around that.

Sweet. That's a 90% failure reduction and 70% exception reduction. This should be the same from a test standpoint, but with a cleaner implementation.

tiny change, a typo may have made one of the tests to fail.

Ugg ... that should have been ..., not .. -- poor testbot :/ Wrong, too. Merge it and then diff next time.

YEAY!!! GREEN!!!

But I noticed something...
Tests are slowed down by 10! minutes with this patch.

Seriously? Why?

Congrats!

OMG Green!

Reviewing...

+++ b/core/modules/system/tests/xmlrpc.test
@@ -99,7 +103,8 @@ class XMLRPCValidator1IncTestCase extends WebTestBase {
-    $xml_url = url(NULL, array('absolute' => TRUE)) . 'core/xmlrpc.php';
+    global $base_url;
+    $xml_url = $base_url . '/core/xmlrpc.php';

Is this a hack? Suppose better to add another array argument to url()

This is same as #1463656-118: Add a Drupal kernel; leverage HttpFoundation and HttpKernel but without the .htaccess change. I'm curious if that change is required for tests to pass.

Same patch as in #1463656-125: Add a Drupal kernel; leverage HttpFoundation and HttpKernel, minus xmlrpc.test changes. If this passes, I will open a separate core issue for that, and a wscci sandbox issue to remove it from the kernel work.

@effulgentsia: Mhh ... looks like this has been solved (somehow) in the mean-time. Ensured it passes with clean URLs as well. I looked at the history and made sure it indeed failed at some point. Didn't bisect it, yet. But as long as it passes that's awesome. Do we even need a follow-up?

Do we even need a follow-up?

Yes. See #1593674: Remove unrelated change to xmlrpc.test for details.

Timing testbot as part of #1578090: Benchmark/profile kernel. Will post analysis there.

#105: 1463656-drupal-kernel.patch queued for re-testing.

An attempt to reroll after all the PSR-0 tests were committed

You're missing the node test stuff

   function testNodeFeedExtraChannelElements() {
-    ob_start();
-    node_feed(array(), array('copyright' => 'Drupal is a registered trademark of Dries Buytaert.'));
-    $output = ob_get_clean();
-
-    $this->assertTrue(strpos($output, '<copyright>Drupal is a registered trademark of Dries Buytaert.</copyright>') !== FALSE);
+    $response = node_feed(array(), array('copyright' => 'Drupal is a registered trademark of Dries Buytaert.'));
+    $this->assertTrue(strpos($response->getContent(), '<copyright>Drupal is a registered trademark of Dries Buytaert.</copyright>') !== FALSE);

A random doc review so I remember all these issues I found for followups.

+++ b/core/includes/bootstrap.incundefined
@@ -4,6 +4,7 @@ use Drupal\Core\Database\Database;
 use Symfony\Component\ClassLoader\ApcUniversalClassLoader;
 use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Symfony\Component\HttpFoundation\Request;

I would group symfony stuff but thats just me

+++ b/core/includes/file.incundefined
@@ -2517,7 +2509,8 @@ function file_directory_temp() {
+ *   \Symfony\Component\HttpFoundation\StreamedResponse.

Do we need the first backslasg?

+++ b/core/lib/Drupal/Core/ContentNegotiation.phpundefined
@@ -0,0 +1,54 @@
+   * @return

probabaly "@return string"

+++ b/core/lib/Drupal/Core/EventSubscriber/RouterListener.phpundefined
@@ -0,0 +1,96 @@
+   * {@inheritdoc}

I still don't like this...

+++ b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.phpundefined
@@ -0,0 +1,128 @@
+class ViewSubscriber implements EventSubscriberInterface {

Has some undocumented functions, but is probably going to be rewritten...

+++ b/core/lib/Drupal/Core/ExceptionController.phpundefined
@@ -0,0 +1,406 @@
+class ExceptionController {

This controller has a lot of functions with missing @return documentation.

+++ b/core/lib/Drupal/Core/LegacyUrlMatcher.phpundefined
@@ -0,0 +1,164 @@
+   * @return Symfony\Component\HttpFoundation\Request $request

Request should be removed

+++ b/core/lib/Drupal/Core/LegacyUrlMatcher.phpundefined
@@ -0,0 +1,164 @@
+   * {@inheritDoc}
+   *
+   * @api

We should rewrite this in drupal api docs

+++ b/core/modules/overlay/overlay.moduleundefined
@@ -299,25 +301,48 @@ function overlay_page_alter(&$page) {
+ * Access callback; determines access to dismiss the overlay accessibility message.

Bigger than 80 chars?

26 days to next Drupal core point release.

I would group symfony stuff but thats just me

Agree.