Replace calls to drupal_container()->get('module_handler') service with Drupal::moduleHandler()

Beta-testing these issues.

There's a slightly more advanced task related to these issues. TestBase should do already does a \Drupal::setContainer($this->container). So, instances of $this->container->get('servicename') could also be cleaned up in tests. Skipping them for now to stick with the program.

StatisticsSettingsForm takes an injected module handler and container, interestingly.

Not sure about the changes in DUTB; I have a sneaking suspicion there might have been an "intended" behavior here of using drupal_container() to get the original container rather than the test environment's. Ditto ModuleApiTest, ModulesDisabledUpgradePathTest, and UpgradePathTestBase. We might need to update the instructions to skip tests on a first pass and do them separately.

minor change because of..

error: patch failed: core/includes/module.inc:287
error: core/includes/module.inc: patch does not apply

-  $schema_store = drupal_container()->get('keyvalue')->get('system.schema');
+  $schema_store = Drupal::keyValue('system.schema');

#7: module_handler-1957148-7.patch queued for re-testing.

noticed this patch has incorrect issue number in filename.. changed that

Thanks @joates for your help!

A couple tips. This issue was currently assigned to me. If you want to work on a patch for an issue that's assigned to someone, it's usually a good idea to check with that person first--either in IRC or on the issue--to make sure they aren't working on it, so that you don't duplicate effort.

Also, please provide an interdiff when updating patches (yours or someone else's). This makes it easy for reviewers and the original patch authors to see the changes you are making. See: http://drupal.org/node/1488712

The next step for this issue is to debug the test failures. Clicking the "view details" link on the patch will show what tests are failing, and then the tests can be run in a local installation for debugging purposes.

@xjm apologies for that..

i was only trying to help it pass the tests with that 1 small change, i edited the patch file (because the conflict was not on a line that you had changed but on another line that needs to match so that the changed line can be located correctly -- i guess?) so i wasn't exactly sure how to "interdiff.txt" that workflow step, i *did* document the change in the comment :)

the 15 failed tests are ALL mostly HTTP 500 Internal Server Errors as far as i can see from the reports.

#11: module_handler-1957154-11.patch queued for re-testing.

rerolled the patch from #4 (there was only 1 conflict to resolve)

@xjm sorry again!
i probably messed things up in #7 by not doing it correctly -- my bad :((

uploading patch file..

(i have read xjm's blog post but i still don't know how to do an interdiff.txt when resolving a merge conflict -- because i cannot diff against a patch that won't apply !!??)

found that i can do this instead..

interdiff old.patch new.patch > ~/interdiff.txt

(edit: mainly these are just changes of the line numbers)

+++ a/core/modules/field/field.moduleundefined
@@ -533,7 +533,7 @@
 function field_sync_field_status() {
   // Refresh the 'active' and 'storage_active' columns according to the current
   // set of enabled modules.
+  $modules = array_keys(drupal_container()->get('module_handler')->getModuleList());
-  $modules = array_keys(Drupal::moduleHandler()->getModuleList());
   foreach ($modules as $module_name) {
     field_associate_fields($module_name);

i had to remove this part to resolve the conflict.. the function field_sync_field_status() now resides at line 398 in the .module file and it looks like somebody else has already implemented the change on line 399 with this line:
$module_handler = Drupal::moduleHandler();
($module_handler is then used throughout the rest of the function)

needs review if it passes the tests this time.

i'm debugging the tests locally..

the first test passes locally !!

testbot-fail

local pass

core/modules/rest/lib/Drupal/rest/Tests/CreateTest.php lines 52-54:

    // Create the entity over the REST API.
    $this->httpRequest('entity/' . $entity_type, 'POST', $serialized, $this->defaultMimeType);
    $this->assertResponse(201);

is the REST interface enabled on the testbot server (qa.drupal.org) ???

the second test also passes locally !!

testbot fail

local pass

test code

      // Make sure that field level access works and that the according field is
      // not available in the response.
      // @see entity_test_entity_field_access()
      $entity->field_test_text->value = 'no access value';
      $entity->save();
      $response = $this->httpRequest('entity/' . $entity_type . '/' . $entity->id(), 'GET', NULL, $this->defaultMimeType);
      $this->assertResponse(200);

again the REST endpoint borks !! :))
and returns 500

maybe someone from the #WSCCI team can get involved at this point ?

Nice and straightforward. Thanks!

Patch does not apply...

curl https://drupal.org/files/1957154-module_handler_0.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 32636  100 32636    0     0  24191      0  0:00:01  0:00:01 --:--:-- 30107
error: patch failed: core/includes/module.inc:289
error: core/includes/module.inc: patch does not apply

And again.

Committed and pushed to 8.x. Thanks!