Problem/Motivation

In build tests, you can provide an iterator for copyCodebase().

If none is provided, BuildTestBase uses a symfony Finder object to generate an iterator. It creates the finder object internally, so you can't just add extra filters on the finder, you have to create a whole different one.

This makes it difficult to remain consistent in what copyCodebase() will copy, because everyone has to re-invent this particular wheel.

Proposed resolution

Add a factory method to BuildTestBase which makes the finder object. This allows test authors to use the same finder constraints that copyCodebase() would, but also add their own.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 interdiff.txt959 bytesMile23
#5 3095809_5.patch2.05 KBMile23
#2 3095809_2.patch1.73 KBMile23
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mile23 created an issue. See original summary.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Status: Active » Needs review
FileSize
3095809_2.patch1.73 KB

A patch.

You can see the use case here: https://github.com/paul-m/drupal/blob/3047468/core/tests/Drupal/BuildTes...

joachim’s picture

joachim CreditAttribution: joachim commented

Just a nitpick:

+++ b/core/tests/Drupal/BuildTests/Framework/BuildTestBase.php
@@ -565,6 +555,25 @@ public function copyCodebase(\Iterator $iterator = NULL, $working_dir = NULL) {
+   * @return \Symfony\Component\Finder\Finder

Missing return description.

Otherwise, looks good, and makes sense to allow this to be tweaked / overridden in test classes.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Tag1 Consulting for European Commission and European Union Institutions, Agencies and Bodies, Drupal Association commented
Status: Needs review » Needs work

+1 from me. Just needs some nits fixed up from per #3.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Status: Needs work » Needs review
FileSize
3095809_5.patch2.05 KB
interdiff.txt959 bytes

Thanks.

Added some clarifying documentation including @return annotation.

joachim’s picture

joachim CreditAttribution: joachim as a volunteer commented
Status: Needs review » Reviewed & tested by the community

LGTM!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed cd1325c397 to 9.0.x and 384187f418 to 8.9.x and 7ddd396c07 to 8.8.x. Thanks!

Backported to 8.8.x as a test-only fix.

  • alexpott committed cd1325c on 9.0.x 
    Issue #3095809 by Mile23, joachim: Add a factory method for...

  • alexpott committed 384187f on 8.9.x 
    Issue #3095809 by Mile23, joachim: Add a factory method for...

  • alexpott committed 7ddd396 on 8.8.x 
    Issue #3095809 by Mile23, joachim: Add a factory method for...

Status: Fixed » Closed (fixed)

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