Problem/Motivation

drupal_phpunit_contrib_extension_directory_roots does not add core/themes but it should because claro uses classes for safe callbacks.

Steps to reproduce

Try to autoload a class from core/themes in a test

Proposed resolution

Add `core/themes` to the list in drupal_phpunit_contrib_extension_directory_roots
Add a unit test that asserts that ClaroPreRender can be autoloaded ($this->assertTrue(class_exists(...))

Remaining tasks

all of the above

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 3209048-core-themes-autoload-for-tests-4--interdiff.patch464 bytesjedihe
#5 3209048-core-themes-autoload-for-tests-4.patch1.23 KBjedihe
#2 3209048-core-themes-autoload-for-tests-2.patch1.22 KBjedihe
#2 3209048-core-themes-autoload-for-tests-2--tests-only.patch834 bytesjedihe

Comments

larowlan created an issue. See original summary.

jedihe’s picture

jedihe commented
StatusFileSize
new 3209048-core-themes-autoload-for-tests-2--tests-only.patch834 bytes
new 3209048-core-themes-autoload-for-tests-2.patch1.22 KB

Let's try this patch. I added the test at core/tests/Drupal/Tests/Core/Theme/CoreThemesAutoloadedForTests.php

jedihe’s picture

jedihe commented
Status: Active » Needs review

The last submitted patch, 2: 3209048-core-themes-autoload-for-tests-2--tests-only.patch, failed testing. View results

jedihe’s picture

jedihe commented
StatusFileSize
new 3209048-core-themes-autoload-for-tests-4.patch1.23 KB
new 3209048-core-themes-autoload-for-tests-4--interdiff.patch464 bytes

No-op update for the patch: add the new item to the array following alphabetic ordering as the existing code does.

No need to trigger tests, as this is an obvious no-op from #2.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Reviewed & tested by the community

Hurray, thanks for the patch and the test. That is what we were looking for! Now we can unit test our theme classes!

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

For context, we did a variation of our test from the other issue with a unit test and ran into this bug, adding that link saved us from manually adding the class through include_once $root ...

joelpittet credited jplana.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Queued a test-run for the RTBC patch, for some reason it has not yet had one

Code looks good to me

  • larowlan committed ccc9255 on 9.2.x 
    Issue #3209048 by jedihe, jplana: Core themes are not added to the test...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Version: 9.2.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Fixed

Committed ccc9255 and pushed to 9.2.x. Thanks!

As the risk of disruption here is low, backported to 9.1.x

Thanks folks

  • larowlan committed 5b68718 on 9.1.x 
    Issue #3209048 by jedihe, jplana: Core themes are not added to the test...

Status: Fixed » Closed (fixed)

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