Follow up from #2244513-203: Move the unmanaged file APIs to the file_system service (file.inc)

Running run-tests.sh with --browser gives you this:

Fatal error: Uncaught Error: Class 'FileSystemInterface' not found in /Users/paul/projects/drupal/core/scripts/run-tests.sh:1574
Stack trace:
#0 /Users/paul/projects/drupal/core/scripts/run-tests.sh(173): simpletest_script_open_browser()
#1 {main}
  thrown in /Users/paul/projects/drupal/core/scripts/run-tests.sh on line 1574

Needs a use Drupal\Core\File\FileSystemInterface; at the top.

CommentFileSizeAuthor
#2 3037218-1.patch471 byteskim.pepper
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kim.pepper created an issue. See original summary.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
CreditAttribution: kim.pepper at PreviousNext commented
Status: Active » Needs review
FileSize
3037218-1.patch471 bytes
dww’s picture

dww
we/he/they
CreditAttribution: dww as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Confirmed the bug and the fix.

Not sure what a bot run is going to tell us, since clearly we don't have coverage for run-tests.sh --browser itself. ;)
Not sure how we could/should add tests for that.

RTBC.

Thanks!
-Derek

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented

We can't test it until we fix #2624926: Refactor run-tests.sh for Console component..

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented

And #655178: Rename run-tests.sh to run-tests.php.

dww’s picture

dww
we/he/they
CreditAttribution: dww as a volunteer commented

Sounds good. Thanks for the links. Now following. ;)

Cheers,
-Derek

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented

There are ways to refactor to test for this. And I'll gladly do that work if someone sets this back to NW and says to. But it's basically one slice of #2624926: Refactor run-tests.sh for Console component. and probably too complex for this follow-up bugfix.

dww’s picture

dww
we/he/they
CreditAttribution: dww as a volunteer commented
Related issues: +#2624926: Refactor run-tests.sh for Console component.

@Mile23 I vote we commit this as-is, and you spend your time (as desired) on the other linked issues. No sense slowing down an obvious fix for a fairly major bug in our testing world for refactoring that's been in the works for months/years.

alexpott’s picture

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

Committed 3f2ab9d and pushed to 8.7.x. Thanks!

Let's prioritise fixing this and testability can come later if necessary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented

Crediting @dww for review and @Mile23 for finding the issue.

  • alexpott committed 3f2ab9d on 8.7.x 
    Issue #3037218 by kim.pepper, dww, Mile23:  run-tests.sh with --browser...

Status: Fixed » Closed (fixed)

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