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

Assigned: Unassigned » mparker17

I will help!

mparker17’s picture

Assigned: mparker17 » Unassigned
Status: Active » Needs review
FileSize
2.66 KB

Try this...

dawehner’s picture

Status: Needs review » Needs work
  1. --- a/core/modules/system/tests/modules/theme_test/lib/Drupal/theme_test/ThemeTestController.php
    +++ b/core/modules/system/tests/modules/theme_test/lib/Drupal/theme_test/ThemeTestController.php
    
    @@ -31,4 +31,17 @@ function functionTemplateOverridden() {
    +    drupal_alter('theme_test_alter', $data);
    

    If the ThemeTestController would extend the ControllBase you could replace drupal_alter easily by $this->moduleHandler()->alter

  2. +++ b/core/modules/system/tests/modules/theme_test/theme_test.module
    @@ -54,8 +54,7 @@ function theme_test_menu() {
       $items['theme-test/alter'] = array(
         'title' => 'Suggestion',
    -    'page callback' => '_theme_test_alter',
    -    'access callback' => TRUE,
    +    'route_name' => 'theme_test_alter',
         'theme callback' => '_theme_custom_theme',
         'type' => MENU_CALLBACK,
       );
    

    We can't drop it yet, because there is none replacement for 'theme callback' yet.

mparker17’s picture

Try this...

mparker17’s picture

Status: Needs work » Needs review
disasm’s picture

Was rerolling for ContainerInjectionInterface, but the class isn't injecting anything, so removed create() and just extending ControllerBase now.

dawehner’s picture

+++ b/core/modules/system/tests/modules/theme_test/lib/Drupal/theme_test/ThemeTestController.php
@@ -31,4 +23,16 @@ function functionTemplateOverridden() {
+  function themeTestAlter() {

Let's make it public and add a @return statement.

disasm’s picture

Making function public and adding @return to all methods.

Status: Needs review » Needs work
Issue tags: -WSCCI-conversion

The last submitted patch, drupal8.theme_test.1987592-10.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
Issue tags: +WSCCI-conversion
dawehner’s picture

+++ b/core/modules/system/tests/modules/theme_test/lib/Drupal/theme_test/ThemeTestController.php
@@ -7,27 +7,62 @@
+   * Menu callback for testing that a theme template overrides a theme function.

Well, technically this is not a menu callback anymore, so maybe call it" Returns a theme template that overrides a theme function."

Status: Needs review » Needs work

The last submitted patch, drupal8.theme_test.1987592-10.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
1.08 KB
3.5 KB

last two patches combined a couple issues. This one fixes docs and goes back to what we had at last passing run.

dawehner’s picture

  1. +++ w/core/modules/system/tests/modules/theme_test/lib/Drupal/theme_test/ThemeTestController.php
    @@ -31,4 +26,18 @@ function functionTemplateOverridden() {
    +   * @return string
    +   */
    

    What about @return string\n Returns a string containing the altered data.

  2. +++ w/core/modules/system/tests/modules/theme_test/lib/Drupal/theme_test/ThemeTestController.php
    @@ -31,4 +26,18 @@ function functionTemplateOverridden() {
    +  function themeTestAlter() {
    

    Let's put public in wrong

  3. +++ w/core/modules/system/tests/modules/theme_test/theme_test.module
    @@ -54,8 +54,7 @@ function theme_test_menu() {
       $items['theme-test/alter'] = array(
         'title' => 'Suggestion',
    
    +++ w/core/modules/system/tests/modules/theme_test/theme_test.routing.yml
    @@ -4,3 +4,10 @@ function_template_override:
    +theme_test_alter:
    +  pattern: 'theme-test/alter'
    +  defaults:
    +    _content: '\Drupal\theme_test\ThemeTestController::themeTestAlter'
    ...
    +    _access: 'TRUE'
    

    Does it make sense to copy the title from the hook_menu bit to the .routing.yml bit?

disasm’s picture

disasm’s picture

Status: Needs review » Closed (duplicate)