Part of #1998638: Replace almost all remaining superglobals ($_GET, $_POST, etc.) with Symfony Request object

Files that need converting are:

  • core/modules/system/lib/Drupal/system/Tests/Bootstrap/OverrideServerVariablesUnitTest.php
  • core/modules/system/lib/Drupal/system/Tests/Common/RenderTest.php
  • core/modules/system/lib/Drupal/system/Tests/Common/TableSortExtenderUnitTest.php
  • core/modules/system/lib/Drupal/system/Tests/Database/SelectPagerDefaultTest.php
  • core/modules/system/lib/Drupal/system/Tests/Entity/EntityQueryTest.php
  • core/modules/system/lib/Drupal/system/Tests/Form/TriggeringElementTest.php
  • core/modules/system/lib/Drupal/system/Tests/Theme/RegistryTest.php
  • core/modules/system/system.admin.inc
  • core/modules/system/system.api.php
  • core/modules/system/system.install
  • core/modules/system/system.module
  • core/modules/system/tests/http.php
  • core/modules/system/tests/https.php
  • core/modules/system/tests/modules/ajax_test/ajax_test.module
  • core/modules/system/tests/modules/form_test/form_test.module
  • core/modules/system/tests/modules/menu_test/menu_test.module:
  • core/modules/system/tests/modules/system_test/system_test.module
  • core/modules/system/tests/modules/session_test/session_test.module
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

atchijov’s picture

atchijov & johnbickar working on this.

John Bickar’s picture

Per atchijov, unit tests for these changes will fail until https://drupal.org/node/1998696 is resolved.

atchijov’s picture

John Bickar’s picture

Status: Active » Needs review

Also per atchijov:

  • changes to OverrideServerVariablesUnitTest.php depends on drupal_override_server_variables() in core/includes/bootstrap.inc
  • changes to TableSortExtenderUnitTest.php depends on tablesort_init() in core/includes/tablesort.inc
  • core/modules/system/lib/Drupal/system/Tests/Database/SelectPagerDefaultTest.php does not do proper setup/teardown - poisoning $_GET
  • core/modules/system/lib/Drupal/system/Tests/Form/TriggeringElementTest.php does not use any superglobals
  • core/modules/system/tests/modules/menu_test/menu_test.module does not use any superglobals
  • core/modules/system/tests/modules/session_test/session_test.module does not use any superglobals

Status: Needs review » Needs work

The last submitted patch, drupal-use_symfony_request_for_system_module-1999434-3.patch, failed testing.

atchijov’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, drupal-use_symfony_request_for_system_module-1999434-3.patch, failed testing.

atchijov’s picture

Assigned: Unassigned » atchijov
dawehner’s picture

Assigned: atchijov » Unassigned
Status: Needs work » Needs review


+++ b/core/modules/system/lib/Drupal/system/Tests/Bootstrap/OverrideServerVariablesUnitTest.phpundefined
@@ -25,10 +25,13 @@ public static function getInfo() {
+ $request = \Drupal::request();
+ $server = $request->server;
+ $script_name = $server->get('SCRIPT_NAME');

+++ b/core/modules/system/lib/Drupal/system/Tests/Common/RenderTest.phpundefined
@@ -139,8 +139,9 @@ function testDrupalRenderSorting() {
+ $server = \Drupal::request()->server;
+ $request_method = $server->get('REQUEST_METHOD');

On a pure unit test the Drupal request will not be available here. We should use Request::createFormGlobals(); instead

atchijov’s picture

Status: Needs review » Needs work

The last submitted patch, drupal-use_symfony_request_for_system_module-1999434-3.patch, failed testing.

kim.pepper’s picture

Status: Needs work » Needs review
FileSize
1.34 KB

Only converting the system.php and system.api.php as the tests can be converted post api freeze.

larowlan’s picture

+++ b/core/modules/system/system.admin.incundefined
@@ -260,8 +260,10 @@ function system_themes_admin_form_submit($form, &$form_state) {
+  if (isset($theme) && isset($token) && drupal_valid_token($token, 'system-theme-operation-link')) {

These will always be set. I think you need !empty() now.

+++ b/core/modules/system/system.api.phpundefined
@@ -1766,9 +1766,8 @@ function hook_template_preprocess_default_variables_alter(&$variables) {
+  $request = Drupal::request();
+  return $request->query->get('theme');

Can this be combined into one line?

kim.pepper’s picture

Fixes for #13.

larowlan’s picture

Status: Needs review » Reviewed & tested by the community
YesCT’s picture

Issue tags: +RTBC July 1

This issue was RTBC and passing tests on July 1, the beginning of API freeze.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 09d7b81 and pushed to 8.x. Thanks!

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