Files: 
CommentFileSizeAuthor
#3 drupal8.variable-leftovers.2.patch3.77 KBsun
PASSED: [[SimpleTest]]: [MySQL] 59,808 pass(es). View
drupal8.variable-leftovers.0.patch5.48 KBsun
PASSED: [[SimpleTest]]: [MySQL] 59,798 pass(es). View

Comments

alexpott’s picture

  1. +++ b/core/modules/rdf/lib/Drupal/rdf/Tests/StandardProfileTest.php
    @@ -284,7 +284,9 @@ protected function doArticleRdfaTests() {
    -    variable_set('node_submitted_page', TRUE);
    +    $node_type = entity_load('node_type', 'page');
    +    $node_type->settings['node']['submitted'] = TRUE;
    +    $node_type->save();
    ...
         // The standard profile hides the created date on pages. Revert display to
         // true for testing.
    
    +++ b/core/modules/system/lib/Drupal/system/Tests/File/RemoteFileUnmanagedMoveTest.php
    @@ -27,6 +27,6 @@ public static function getInfo() {
    -    variable_set('file_default_scheme', 'dummy-remote');
    +    \Drupal::config('system.file')->set('default_scheme', 'dummy-remote')->save();
    

    Surely we can just remove the variable_set() nothing is using variables that are set and the tests are passing :)

  2. +++ b/core/includes/update.inc
    index b18d9cc6..8b25833 100644
    --- a/core/modules/menu/menu.api.php
    

    This change is already being attempted in #2108679: API documentation: Convert my_module_menus examples in menu.api.php to CMI how about reviewing that?

sun’s picture

sun’s picture

Assigned: Unassigned » sun
FileSize
3.77 KB
PASSED: [[SimpleTest]]: [MySQL] 59,808 pass(es). View

re: hook_menu_*()

ACK, followed up + provided new patch over there. Thanks for the pointer!

(OT: I currently have the Drupal core dev experience that 99% of the rest of the world probably has, too... there are so many issues, each one covering super-small bits only, it's impossible to make sense of what's going on [and what's not] → "Issueitis" ;))

re: RDF test:

Yes, the lack of that node type settings update should technically cause the test to fail, and because it apparently does not, we can only assume that the manipulation is not required in the first place. Happy to simply kill those lines.

re: RemoteFileUnmanagedMoveTest:

This one, however, is different — the lack of setting that override merely means that the (actually executed) parent UnmanagedMoveTest is essentially executed twice without actually testing the case of remote files/schemes/streams.

By putting the override back in place (copied 1:1 from other Remote* tests), this test actually re-starts to test what it is supposed to test. The fact that tests are still passing without and with that override just means that the functionality still works as expected. — Phew, lucky us! :)

alexpott’s picture

Status: Needs review » Reviewed & tested by the community

Looks good - thanks.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Awesome!

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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