Convert various tests that use bartik/seven to olivero/claro

I tried to reduce the number of test errors but wasn't able to fix NewDefaultThemeBlocksTest.php

Doing some debug I discovered that, by default on this test, the $default_block_names and $new_blocks (lines 61 and 64 respectively) store values like:

$default_block_names:
stark_oz0qy6f1
stark_tkycrrfd
stark_feppeaad

$new_blocks:
bartik_oz0qy6f1
bartik_tkycrrfd
bartik_feppeaad

But after changing the $new_theme (line 48) to 'olivero' we then get:

$default_block_names:
stark_l8b1ac2g
stark_vo6wra9g
stark_dcmgn8bo

$new_blocks:
olivero_page_title
olivero_primary_local_tasks
olivero_secondary_local_tasks
olivero_breadcrumbs
olivero_content
olivero_messages
olivero_powered
olivero_site_branding
olivero_account_menu
olivero_main_menu

What is weird to me is that the random characters at the end of the block names inside $default_block_names are only generated for the default (bartik) theme (lines 34-45) and are somehow passed to the stark block names which makes me suspect that the theme installation (in line 52) behaves differently on bartik and olivero.

Did some debugging to find out what is causing the behavior explained in the last paragraph of #7.

During theme installation, the ThemeInstaller does $this->moduleHandler->invokeAll('themes_installed', [$themes_installed]); and since this about blocks I looked at \block_themes_installed() which will call \block_theme_initialize. The first thing that function does is load all the blocks for the given theme. If the theme does not have blocks (Bartik does not) it will duplicate the blocks of the existing theme, Stark. Duplicate meaning replace Stark with Bartik in the block id and put the block in the same region, if it exists. If the theme has blocks (Olivero has many in olivero/config/optional) then that duplication process does not happen.

It isn't clear to me what this test is really trying to test. It seems to be the hook. If this is testing Olivero then it just needs to confirm that blocks exists but surely testing blocks are loaded is done somewhere in the configuration system tests? I agree with catch that a testing theme should be used.

This patch changes to test_theme in NewDefaultThemeBlocksTest.php.

Created the followup, #3280049: Create a test admin and front end theme.

The patch in #8 looks good to me.

However, I'm still seeing a ton of references to both Bartik and Seven:

$ git grep -E '(bartik)|(seven)' -- '*Test*' | awk -F: '{print $1}' | sort | uniq -c | sort -r
  48 core/modules/block/tests/src/Kernel/Migrate/d7/MigrateBlockTest.php
  21 core/modules/color/tests/src/Kernel/Plugin/migrate/source/d7/ColorTest.php
  15 core/modules/color/tests/src/Functional/ColorTest.php
  14 core/modules/block/tests/src/Kernel/Migrate/d6/MigrateBlockTest.php
  13 core/modules/system/tests/src/Functional/System/ThemeTest.php
   7 core/modules/shortcut/tests/src/Functional/ShortcutLinksTest.php
   7 core/modules/ckeditor/tests/src/Kernel/CKEditorTest.php
   7 core/modules/block/tests/src/Functional/BlockAdminThemeTest.php
   6 core/tests/Drupal/Tests/Core/Theme/AjaxBasePageNegotiatorTest.php
   6 core/modules/taxonomy/tests/src/Functional/ThemeTest.php
   6 core/modules/media_library/tests/src/Functional/MediaLibraryDisplayModeTest.php
   6 core/modules/color/tests/src/Kernel/Migrate/d7/MigrateColorTest.php
   6 core/modules/block/tests/src/Functional/BlockTest.php
   5 core/tests/Drupal/Tests/Core/Extension/ThemeHandlerTest.php
   5 core/tests/Drupal/FunctionalTests/Theme/BartikTest.php
   5 core/modules/shortcut/tests/src/Kernel/ShortcutSevenIntegrationTest.php
   5 core/modules/block/tests/src/Unit/Plugin/migrate/process/BlockRegionTest.php
   4 core/tests/Drupal/KernelTests/Core/Theme/ConfirmClassyCopiesTest.php
   4 core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxTest.php
   4 core/modules/system/tests/src/Kernel/Migrate/d7/MigrateThemeSettingsTest.php
   4 core/modules/system/tests/src/Functional/Theme/ThemeTokenTest.php
   4 core/modules/node/tests/src/Functional/NodeTranslationUITest.php
   4 core/modules/menu_link_content/tests/src/Functional/MenuLinkContentTranslationUITest.php
   4 core/modules/block/tests/src/Kernel/Plugin/migrate/source/d7/BlockTranslationTest.php
   3 core/tests/Drupal/Tests/Core/Extension/ExtensionDiscoveryTest.php
   3 core/tests/Drupal/KernelTests/Core/Theme/ThemeInstallerTest.php
   3 core/tests/Drupal/FunctionalTests/Theme/SevenLayoutBuilderTest.php
   3 core/modules/taxonomy/tests/src/Functional/Rest/TermResourceTestBase.php
   3 core/modules/settings_tray/tests/src/FunctionalJavascript/SettingsTrayTestBase.php
   3 core/modules/jsonapi/tests/src/Functional/TermTest.php
   3 core/modules/help_topics/tests/src/Functional/HelpTopicTranslatedTestBase.php
   3 core/modules/help_topics/tests/src/Functional/HelpTopicTest.php
   3 core/modules/block_content/tests/src/Functional/BlockContentTypeTest.php
   2 core/tests/Drupal/Tests/Core/Command/GenerateThemeTest.php
   2 core/tests/Drupal/Tests/Core/Asset/CssCollectionGrouperUnitTest.php
   2 core/tests/Drupal/KernelTests/Core/Asset/ResolvedLibraryDefinitionsFilesMatchTest.php
   2 core/tests/Drupal/FunctionalTests/Installer/MultipleDistributionsProfileTest.php
   2 core/modules/views_ui/tests/src/Functional/ViewEditTest.php
   2 core/modules/system/tests/src/Kernel/Plugin/migrate/source/d7/ThemeSettingsTest.php
   2 core/modules/system/tests/src/Functional/UpdateSystem/UpdatePathTestBaseFilledTest.php
   2 core/modules/system/tests/src/Functional/UpdateSystem/BrokenCacheUpdateTest.php
   2 core/modules/settings_tray/tests/src/FunctionalJavascript/SettingsTrayBlockFormTest.php
   2 core/modules/responsive_image/tests/src/Unit/ResponsiveImageStyleConfigEntityUnitTest.php
   2 core/modules/color/tests/src/Functional/ColorConfigSchemaTest.php
   2 core/modules/ckeditor5/tests/src/Functional/AddedStylesheetsTest.php
   2 core/modules/block/tests/src/FunctionalJavascript/BlockAddTest.php
   2 core/modules/block/tests/src/Functional/BlockDemoTest.php
   1 core/tests/Drupal/Tests/Core/Render/RendererPlaceholdersTest.php
   1 core/tests/Drupal/Tests/Core/Asset/AssetResolverTest.php
   1 core/tests/Drupal/KernelTests/Core/Config/DefaultConfigTest.php
   1 core/tests/Drupal/KernelTests/Core/Config/ConfigImporterTest.php
   1 core/tests/Drupal/FunctionalTests/Installer/StandardInstallerTest.php
   1 core/tests/Drupal/FunctionalTests/Installer/InstallerTest.php
   1 core/tests/Drupal/FunctionalTests/Installer/DistributionProfileTranslationTest.php
   1 core/tests/Drupal/FunctionalTests/Installer/DistributionProfileTranslationQueryTest.php
   1 core/tests/Drupal/FunctionalTests/Installer/DistributionProfileTest.php
   1 core/tests/Drupal/FunctionalTests/Installer/DistributionProfileExistingSettingsTest.php
   1 core/modules/views/tests/src/Functional/UserBatchActionTest.php
   1 core/modules/system/tests/src/FunctionalJavascript/OffCanvasTestBase.php
   1 core/modules/system/tests/src/Functional/Form/ValidationTest.php
   1 core/modules/search/tests/src/Functional/SearchAdminThemeTest.php
   1 core/modules/help_topics/tests/src/Functional/HelpTopicsSyntaxTest.php
   1 core/modules/editor/tests/src/Kernel/EditorImageDialogTest.php
   1 core/modules/comment/tests/src/Functional/CommentDisplayConfigurableTest.php
   1 core/modules/breakpoint/tests/src/Kernel/BreakpointDiscoveryTest.php
   1 core/modules/block/tests/src/Kernel/Migrate/d7/MigrateBlockContentTranslationTest.php
   1 core/modules/block/tests/src/Functional/NonDefaultBlockAdminTest.php

It looks like many of these are tests that don't need to rely on those themes. For example, core/modules/block/tests/src/Kernel/Migrate/d6/MigrateBlockTest.php just sets Bartik as the default theme, when it could probably be Olivero. There's a minority of tests that are actually testing Bartik or Seven themselves, like core/tests/Drupal/FunctionalTests/Theme/BartikTest.php.

Is updating all of these tests expected for this issue? Even if it was, I think it would make sense to audit the above files and create individual issues so we can work on this incrementally. We could retitle this issue to match the patch so it could be merged as is?

I've updated this issue title and summary to reflect the current patch.

I've filed (more) than a handful of new issues, but I've tried to break them down and combine them into groupings based on subsystem and estimated complexity. My hope is there's a good range in here of both easy and hard issues to be actionable by new and experienced contributors alike. Of course, I'm sure there will be areas I've got that wrong once someone actually dives into each issue.

Complex Issues

#3281427: Update Block and Theme setting migrations to not use Bartik and Seven
#3281429: Update Block non-Migration tests to not use Bartik and Seven
#3281430: Update non-migration Color tests to not use Bartik
#3281434: Update System module tests to not use Bartik and Seven

Mid-level issues

#3281444: Update Installer tests to not use Bartik and Seven
#3281446: Update Media Library tests to not use Bartik and Seven
#3281449: Update Core unit tests to not use Bartik and Seven
#3281452: Update Core kernel tests to not use Bartik and Seven
#3281451: Update Ajax FunctionalJavascript test to not use Bartik and Seven
#3281454: Update various module tests to not use Bartik and Seven

Small Issues

#3281438: Update CKEditorTest to not use Bartik and Seven
#3281439: Update Help Topics tests to not use Bartik and Seven
#3281440: Update Shortcut tests to not use Bartik and Seven
#3281441: Update Settings Tray tests to not use Bartik and Seven
#3281443: Update Taxonomy tests to not use Bartik and Seven
#3281457: Move Seven and Bartik tests in the core FunctionalTests\Theme namespace to their respective themes

As I previously reviewed the current patch, and it's passing, I think those updates are ready to be committed.

Tried to fix the typo and autocorrect got me!

I think the providers changed here...

+++ b/core/modules/node/tests/src/Functional/NodeDisplayConfigurableTest.php
@@ -165,7 +165,6 @@ protected function assertNodeHtml(NodeInterface $node, UserInterface $user, bool
   public function provideThemes() {
     return [
-      ['bartik', 'header', TRUE],
       ['claro', 'footer', TRUE],
       // @todo Remove Classy from data provider in
       // https://www.drupal.org/project/drupal/issues/3110137.
@@ -173,7 +172,6 @@ public function provideThemes() {

@@ -173,7 +172,6 @@ public function provideThemes() {
       // @todo Add coverage for olivero after fixing
       // https://www.drupal.org/project/drupal/issues/3215220.
       // ['olivero', 'footer', TRUE],
-      ['seven', 'footer', TRUE],
       ['stable', 'footer', FALSE],
       ['stable9', 'footer', FALSE],
     ];

+++ b/core/modules/system/tests/src/FunctionalJavascript/OffCanvasTestBase.php
@@ -110,7 +110,7 @@ protected function getOffCanvasDialog() {
-    return ['bartik', 'classy', 'olivero', 'seven', 'stable', 'stark'];
+    return ['classy', 'olivero', 'seven', 'stable', 'stark'];

+++ b/core/modules/views/tests/src/Kernel/Plugin/DisplayPageTest.php
@@ -244,7 +244,7 @@ public function testEmptyRow() {
-    $themes = ['bartik', 'classy', 'olivero', 'seven', 'stable', 'stark'];
+    $themes = ['classy', 'olivero', 'claro', 'stable', 'stark'];

should be done in a different issue. If we do that then we can backport all the other changes to 9.4.x which helps merge fixes across the different branches. Especially since we have so many other issues.

Undid some of the changes as suggested by @alexpott in #17.

I tried to save, and andregp beat me to it!

• I moved NodeDisplayConfigurableTest and DisplayPageTest to #3281454: Update various module tests to not use Bartik and Seven.
• OffCanvasTestBase is being handled already in #3281449: Update Core unit tests to not use Bartik and Seven.

Assign this issue to the parent #3285205: [META] Convert test that use Bartik/Seven to Olivero/Claro.

reroll for 10.x, looks good to me.

Olivero is a bit too optimistic about color values :) should we have the fix to make _olivero_hex_to_hsl more resilient as part of this patch or a followup is better?

removed the problematic test and opened a follow-up

Patch in #29 looks good to me, flipping the ConfigUIImportTest tests back to bartik. I've confirmed the only changes in the 9.5.x patch are line numbers.