Comments

vijaycs85’s picture

Status: Active » Closed (won't fix)

Need to rewrite the whole module to make test sync with current test implementation. For more details, please refer: #1988802: [META] Rewrite test modules in system to provide better unit testing.

ayelet_Cr’s picture

Status: Closed (won't fix) » Active
Sean Buscay’s picture

Assigned: Unassigned » Sean Buscay
disasm’s picture

Title: Convert system_test_authorize_init_page() to a new style controller » Convert system_test callbacks to a new style controller
Assigned: Sean Buscay » disasm
Status: Active » Needs review
FileSize
10.44 KB
FAILED: [[SimpleTest]]: [MySQL] 58,071 pass(es), 11 fail(s), and 2 exception(s). View

attached is a first run at this patch.

Status: Needs review » Needs work

The last submitted patch, drupal8.system_test-module.1987832-4.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
1.37 KB
10.4 KB
FAILED: [[SimpleTest]]: [MySQL] 58,284 pass(es), 8 fail(s), and 2 exception(s). View

I doubt this will fix the tests, but it makes the class a little cleaner. Someone that knows how drupal_register_shutdown_function works is probably going to have to dig into or give guidance on this issue.

Status: Needs review » Needs work

The last submitted patch, drupal8.system_test.1987832-6.patch, failed testing.

xjm’s picture

Thanks for your work on this issue! Please see #1971384-43: [META] Convert page callbacks to controllers for an update on the routing system conversion process.

vijaycs85’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
7.45 KB
FAILED: [[SimpleTest]]: [MySQL] 59,711 pass(es), 25 fail(s), and 8 exception(s). View

Re-issuing patch with current code base.

Status: Needs review » Needs work

The last submitted patch, 9: 1987832-system_test-controller-9.patch, failed testing.

Mile23’s picture

Issue tags: +Needs reroll
vks7056’s picture

Issue tags: -WSCCI-conversion, -Needs reroll +SprintWeekend2015
FileSize
7.45 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 1987832-system_test-controller-10.patch. Unable to apply patch. See the log in the details link for more information. View
valthebald’s picture

@vks7056: if you feel that patch is ready for review, please change issue status to 'Needs review'

Mile23’s picture

Status: Needs work » Needs review

Setting to 'needs review' for the testbot.

Status: Needs review » Needs work

The last submitted patch, 12: 1987832-system_test-controller-10.patch, failed testing.

Mile23’s picture

Assigned: disasm » Unassigned
Issue tags: +Needs reroll

Needs reroll.

Mile23’s picture

Issue tags: +Novice
adci_contributor’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
6.82 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,079 pass(es), 11 fail(s), and 2 exception(s). View

Trying to reroll.

Status: Needs review » Needs work

The last submitted patch, 18: system_test-controller- 1987832-18.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review
FileSize
7.69 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 85,262 pass(es), 4 fail(s), and 0 exception(s). View

Status: Needs review » Needs work

The last submitted patch, 20: 1987832-20.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review
FileSize
7.81 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 85,266 pass(es). View

Trying to fix the tests

Status: Needs review » Needs work

The last submitted patch, 22: 1987832-22.patch, failed testing.

Status: Needs work » Needs review

aspilicious queued 22: 1987832-22.patch for re-testing.

andypost’s picture

  1. +++ b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php
    @@ -76,17 +77,30 @@ public function drupalSetMessageTest() {
    +    if (\Drupal::lock()->acquire('system_test_lock_acquire')) {
    +      \Drupal::lock()->release('system_test_lock_acquire');
    ...
    +    if (\Drupal::lock()->acquire('system_test_lock_exit', 900)) {
    

    lock service needs injection

  2. +++ b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php
    @@ -132,10 +146,14 @@ public static function preRenderCacheTags($elements) {
    +    $authorize_url = $GLOBALS['base_url'] . '/core/authorize.php';
    

    suppose globals access should be removed

aspilicious’s picture

FileSize
8.99 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 85,593 pass(es), 5 fail(s), and 1 exception(s). View

Status: Needs review » Needs work

The last submitted patch, 26: 1987832-26.patch, failed testing.

aspilicious’s picture

FileSize
9 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 85,567 pass(es), 6 fail(s), and 2 exception(s). View

Let's try again

aspilicious’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 28: 1987832-28.patch, failed testing.

aspilicious’s picture

I need help

+    $authorize_url = Url::fromUri('base:core/authorize.php')->toString();

is not the same as

$authorize_url = $GLOBALS['base_url'] . '/core/authorize.php';

So what should it be?

webflo’s picture

Status: Needs work » Needs review
FileSize
9.03 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 85,950 pass(es). View
946 bytes
+++ b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php
@@ -132,10 +157,14 @@ public static function preRenderCacheTags($elements) {
   public function authorizeInit($page_title) {
-    return system_test_authorize_init_page($page_title);
+    $authorize_url = Url::fromUri('base:core/authorize.php')->toString();
+    system_authorized_init('system_test_authorize_run', drupal_get_path('module', 'system_test') . '/system_test.module', array(), $page_title);
+    return new RedirectResponse($authorize_url);
   }

RedirectRepsonse expects absolute urls.

Status: Needs review » Needs work

The last submitted patch, 32: 1987832-29.patch, failed testing.

webflo queued 32: 1987832-29.patch for re-testing.

webflo’s picture

Status: Needs work » Needs review
aspilicious’s picture

Whoeha THNX!

andypost’s picture

Status: Needs review » Reviewed & tested by the community

Great!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This is test only code and part of the WSCCI conversions hence allowable during beta. Committed e132a97 and pushed to 8.0.x. Thanks!

  • alexpott committed e132a97 on 8.0.x
    Issue #1987832 by aspilicious, disasm, webflo, vijaycs85,...

Status: Fixed » Closed (fixed)

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