Currently the unit test flysystem_s3/tests/src/Unit/AwsCacheAdapterTest.php fails after upgrading to Drupal 9 using version 2.0.0-rc1.

PHP Fatal error: Uncaught Error: Class 'PHPUnit_Framework_TestCase' not found in /var/www/site/web/modules/contrib/flysystem_s3/tests/src/Unit/AwsCacheAdapterTest.php

Would we be able to transition to using PHPUnit\Framework\Testcase?

CommentFileSizeAuthor
#10 3254472-update-tests-to-work-with-drupal-9-10.patch8.66 KBleon kessler
#8 3254472-update-tests-to-work-with-drupal-9-8.patch5.97 KBleon kessler
#7 3254472-update-tests-to-work-with-drupal-9-7.patch5.85 KBleon kessler
#6 3254472-update-tests-to-work-with-drupal-9-6.patch1.34 KBleon kessler
#2 flysystem_s3-testcase-3254472.patch662 bytesjnlar

Issue fork flysystem_s3-3254472

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

jonarcher created an issue. See original summary.

jnlar’s picture

jnlar
Primary language English
Location Sydney, Australia
commented
StatusFileSize
new flysystem_s3-testcase-3254472.patch662 bytes

Attached patch.

jnlar’s picture

jnlar
Primary language English
Location Sydney, Australia
commented
leon kessler’s picture

leon kessler commented
Title: Uncaught Error: Class 'PHPUnit_Framework_TestCase' not found » Update tests to work with Drupal 9
Version: 2.0.0-rc1 » 2.0.x-dev
Assigned: jnlar » leon kessler
Category: Bug report » Task

Ran the patch through the test runner on the new 2.0.x-dev branch, there's another fail. Updating the issue summary to cover this one as well.

Leon Kessler opened merge request !6

leon kessler’s picture

leon kessler commented
StatusFileSize
new 3254472-update-tests-to-work-with-drupal-9-6.patch1.34 KB

Seems to be different errors running through gitlab than as a patch. Uploading changes from the MR as a patch file to confirm.

leon kessler’s picture

leon kessler commented
Status: Active » Needs review
StatusFileSize
new 3254472-update-tests-to-work-with-drupal-9-7.patch5.85 KB

Okay I'm hoping this will be enough to get the tests to pass, let's see...

leon kessler’s picture

leon kessler commented
StatusFileSize
new 3254472-update-tests-to-work-with-drupal-9-8.patch5.97 KB

Previous patch didn't include the file rename ModuleInstallUninstallWebTest correctly.

leon kessler’s picture

leon kessler commented

Woohooo! I think this can be merged in. But the tests could really do with a cleanup, I'll probably open another issue for that.

A few notes:

  • I've used 'public' => TRUE on anything that calls $plugin->getExternalUrl, as otherwise it hits FlysystemUrlTrait::getExternalUrl(), which in turn uses the UrlGenerator service. Whilst this can be mocked, it means we're not really testing that much functionality from the flysystem_s3 module. I'm not exactly sure how the tests were originally passing without 'public' => TRUE being set, but I feel like anything calling getExternalUrl() on the S3 plugin probably should be public. And that probably we need separate tests for private and public config.
  • I'm not really sure what the NoDrupal namespaces were about, but these were causing errors so I updated them to Drupal.
  • The tests are lacking comments and some don't have particularly great names (e.g. public function test())
leon kessler’s picture

leon kessler commented
StatusFileSize
new 3254472-update-tests-to-work-with-drupal-9-10.patch8.66 KB

Made a few small improvements in latest patch:

  • Added comments to each test
  • Updated name of function test() to testGetExternalUrl
  • Removed function testIamAuth that had no assertions.

  • Leon Kessler authored 66bb7ac on 2.0.x 
    Issue #3254472 by Leon Kessler, jonarcher: Update tests to work with...
leon kessler’s picture

leon kessler commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

Eli-T closed merge request !6