Support from Acquia helps fund testing for Drupal Acquia logo

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
disasm’s picture

Title: Convert error_test_generate_warnings() to a new style controller » Convert error_tes module to a new style controller
Assigned: Unassigned » disasm
disasm’s picture

Title: Convert error_tes module to a new style controller » Convert error_test module to a new style controller
disasm’s picture

Status: Active » Needs review
FileSize
9.43 KB

attached patch converts all routes in error_test module and fixes the error messages the tests are looking for.

Status: Needs review » Needs work

The last submitted patch, drupal8.error_test_conversion.1987700-5.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
3.3 KB
10.3 KB

attached patch with interdiff fixes remaining test failures.

dawehner’s picture

Status: Needs review » Needs work
+++ b/core/modules/system/tests/modules/error_test/lib/Drupal/error_test/Controllers/ErrorTestController.php
@@ -0,0 +1,68 @@
+class ErrorTestController implements ControllerInterface {
...
+  /**
+   * Constructs a UpdateTestController object.
+   *
+   * @param \Drupal\Core\Config\ConfigFactory $config_factory
+   *   The factory for configuration objects.
+   */
+  public function __construct(Connection $database) {
+    $this->database = $database;
+  }

Let's simply extend the ControllerBase, so need for this boilerplate for tests.

disasm’s picture

Status: Needs work » Needs review
FileSize
5.47 KB
9.75 KB

renamed Controllers -> Controller.
Removed boilerplate in favor of $this->container->get

Status: Needs review » Needs work

The last submitted patch, drupal8.error_test.1987700-9.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
737 bytes
9.75 KB

Attached patch fixes missing ;

Status: Needs review » Needs work

The last submitted patch, drupal8.error_test.1987700-11.patch, failed testing.

disasm’s picture

Forgot error handlers were keying off of the class name. That's fixed too now.

disasm’s picture

Status: Needs work » Needs review
dawehner’s picture

+++ b/core/modules/system/lib/Drupal/system/Tests/Common/SimpleTestErrorCollectorTest.php
@@ -94,6 +95,7 @@ protected function error($message = '', $group = 'Other', array $caller = NULL)
+    $this->verbose(print_r($error,TRUE));

Let's remove that :)

disasm’s picture

removed!

jibran’s picture

Status: Needs review » Needs work
+++ b/core/modules/system/lib/Drupal/system/Tests/Common/SimpleTestErrorCollectorTest.php
@@ -94,6 +94,7 @@ protected function error($message = '', $group = 'Other', array $caller = NULL)
+    $this->verbose(print_r($error,TRUE));

There is one more.

disasm’s picture

Status: Needs work » Needs review
FileSize
1.05 KB
8.92 KB

removed!

Crell’s picture

Status: Needs review » Reviewed & tested by the community

Quick, before core changes again. :-)

Crell’s picture

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

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