Problem/Motivation

Let's convert test theme to oop.

InstallerTranslationTest does something strange during install time that blows up if hooks are OOP.

Steps to reproduce

Convert test_theme
Run core/tests/Drupal/FunctionalTests/Installer/InstallerTranslationTest.php

Proposed resolution

Convert test theme to OOP hooks and add DI
Allow install kernel container dumping, this was a relic of simple test an no longer needed
Remove test for loading classes in .theme since the nature of OOP theme hooks ensure that theme namespaces are loaded at the correct time
Add parameter types to hook class

Remaining tasks

Review

User interface changes

N/A

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3560851

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

nicxvan created an issue. See original summary.

nicxvan’s picture

nicxvan commented
Version: 11.x-dev » main

nicxvan opened merge request !14397

nicxvan’s picture

nicxvan commented

Still haven't figured out why Installer test is failing,

It fails here:

    // Verify German was configured but not English.
    $this->drupalGet('admin/config/regional/language');
    // cspell:ignore deutsch
    $this->assertSession()->pageTextContains('Deutsch');
nicxvan’s picture

nicxvan commented
Issue summary: View changes
Status: Active » Needs review

Ok, @berdir was able to find the root cause, it is due to the installer base setting dumping to false.

After converting to oop the install requires the container to build so this is a test artifact.

Berdir tracked that parameter to #2016629: Refactor bootstrap to better utilize the kernel.

Znerol in comment 221 describes it as necessary for to simpler which we no longer use.

I reached out to chx to ask his opinion and he agrees that the setting is outdated here.

Tests are happy so I think this is ready.

nicxvan’s picture

nicxvan commented
Status: Needs review » Needs work

I was premature, there is still one real failure.

berdir made their first commit to this issue’s fork.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

How about we just call this one "Convert test_theme to OOP" ? It's not "after" fixing InstallerTranslationTest, we are fixing InstallerTranslationTest here.

nicxvan’s picture

nicxvan commented
Title: Convert test_theme to OOP after fixing InstallerTranslationTest » Convert test_theme to OOP

No objection, it was a reminder to myself which test was failing.

nicxvan’s picture

nicxvan commented
Issue summary: View changes
Status: Needs work » Needs review

Ready for review again

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

I think this looks great now. added two comments as explanations for reviewers.

  • longwave committed e15e26c9 on 11.x 
    refactor: #3560851 Convert test_theme to OOP

By: nicxvan
By: berdir
By...

  • longwave committed b75cccea on main 
    refactor: #3560851 Convert test_theme to OOP

By: nicxvan
By: berdir
By...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Thanks, this is nice cleanup - especially good that test_theme_theme_test_alter_alter and test_theme_theme_suggestions_theme_test_suggestions_alter are finally gone as those names have always confused me - at least they're slightly shorter now :D

Backported to 11.x to keep things in sync.

Committed and pushed b75cccea7c8 to main and e15e26c9f0b to 11.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

longwave closed merge request !14397

Status: Fixed » Closed (fixed)

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