PHP Fatal error:  Declaration of Drupal\s3fs\S3fsService::writeFolders($fol  
  ders) must be compatible with Drupal\s3fs\S3fsServiceInterface::writeFolder  
  s(array $folders) in /var/www/html/web/modules/contrib/s3fs/src/S3fsService  
  .php on line 24

Problem is the interface requires the argument to be an array, the implementation doesn't.

CommentFileSizeAuthor
#9 s3fsservice_writefol-3205964-9.patch1.89 KBcmlara
#9 interdiff-3205964-4-9.txt1.05 KBcmlara
#4 s3fsservice_writefol-3205964-4.patch862 bytescmlara

Issue fork s3fs-3205964

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

jonathanshaw created an issue. See original summary.

cmlara’s picture

cmlara
he/him
commented

Looks like I missed a couple of them in that commit. I recall writing the interface file and knowing I needed to tab over and put the changes in but looks like somewhere across the edits it did not get done.

Going to go back to that commit and make sure I didn't miss any other ones before I publish a fix out.

Mind my asking what version of PHP you are running? I didn't trigger the errors in 7.4 in my local lab when I ran phpunit so I'm suspecting this is php version related that it actually triggers the fatal errors.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

My CI is running in 7.2 or 7.3 I think. I don't get the error locally which is 7.2.

cmlara’s picture

cmlara
he/him
commented
Title: S3fsService:writeFolders is not compatble with interface » S3fsService:writeFolders is not compatible with interface
StatusFileSize
new s3fsservice_writefol-3205964-4.patch862 bytes

Might be time for me to setup a matrix on the local CI server with all the supported Drupal Versions and PHP versions to start testing this modules commits.

Looks like I only missed 2 of the ones I needed to change.

If you could run this through your CI just to be sure I caught them for you it would be appreciated.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

And another:

PHP Fatal error:  Declaration of Drupal\s3fs\Batch\S3fsRefreshCacheBatch::r  
  efreshCacheOperation($config, &$context) must be compatible with Drupal\s3f  
  s\Batch\S3fsRefreshCacheBatchInterface::refreshCacheOperation(array $config  
  , array &$context) in /var/www/html/web/modules/contrib/s3fs/src/Batch/S3fs  
  RefreshCacheBatch.php on line 16
jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Status: Active » Needs review
jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

Might be time for me to setup a matrix on the local CI server with all the supported Drupal Versions and PHP versions to start testing this modules commits.

Drupal CI can help. You can configure on your project settings what gets tested regularly.

cmlara’s picture

cmlara
he/him
commented

Drupal CI can help. You can configure on your project settings what gets tested regularly.

Unfortunately not yet, at the moment the large majority of the tests are Functional tests and they require S3 to be fully operational with working secrets to actually execute which means everything except getExternalUrl tests are actually being skipped by the DrupalCI.

I do have some thoughts about configuring up a localstack docker image for the Drupal CI so it can actually execute them, but at the moment that is still a few rungs down on the ladder, though yes that would also allow for running periodic tests against different version combos w/o needing my own local matrix.

cmlara’s picture

cmlara
he/him
commented
StatusFileSize
new interdiff-3205964-4-9.txt1.05 KB
new s3fsservice_writefol-3205964-9.patch1.89 KB

Ok 2 more in S3fsRefreshCacheBatch.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

at the moment the large majority of the tests are Functional tests and they require S3 to be fully operational with working secrets

Sounds like it might be easier to have the CI use a real s3fs bucket, with some very strict policies to protect it against being abused. I think commerce_stripe has tests that call Stripe's external API without problem.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Status: Needs review » Reviewed & tested by the community

#9 passes for me.

  • cmlara committed d32a754 on 8.x-3.x 
    Issue #3205964 by jonathanshaw, cmlara: S3fsService:writeFolders is not...
cmlara’s picture

cmlara
he/him
commented
Status: Reviewed & tested by the community » Fixed

Perfect! Thanks for testing! Committed.

I'm working on the drupalci enhancements in #3206493: Add ability for Drupal CI to execute functional tests, I did look at commerce_stripe briefly however it appeared they were mostly using mocking (I may have missed real calls to an API) which the current code base for s3fs as far as I can determine isn't really structured well for without significant rewrites.

Status: Fixed » Closed (fixed)

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