Tests are currently failing on Drupal 9; see https://www.drupal.org/pift-ci-job/1559096. Let's see if we can't improve this a bit.

CommentFileSizeAuthor
#8 3115602-8-interdiff.txt1.5 KBberdir
#8 3115602-8.patch2.98 KBberdir
#5 3115602-5.patch1.96 KBphenaproxima
#4 3115602-4.patch654 bytesphenaproxima
#2 3115602-2.patch1.15 KBphenaproxima

Comments

phenaproxima created an issue. See original summary.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Active » Needs review
StatusFileSize
new 3115602-2.patch1.15 KB

Let's see if this mitigates things.

Status: Needs review » Needs work

The last submitted patch, 2: 3115602-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Needs review
StatusFileSize
new 3115602-4.patch654 bytes

Huh, what went wrong?

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
StatusFileSize
new 3115602-5.patch1.96 KB

I think this will pass on D8 and D9, but it would need to wait until Drupal 8.8 is the oldest supported version of core.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Status: Needs review » Reviewed & tested by the community

Looks good.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Reviewed & tested by the community » Needs work 
  1. +++ b/crop.install
@@ -18,7 +18,7 @@ function crop_requirements($phase) {
   $incompatible = FALSE;
   $drupal_version = explode('.', \Drupal::VERSION);
-  if ($drupal_version[1] < 4) {
+  if ($drupal_version[0] == 8 && $drupal_version[1] < 4) {
     $incompatible = TRUE;
   }

    just like in #3111564: Update/Remove EntityBrowserUpdateHookTest for D9 compatibility and other test fixes this check is bogus at this point when we also require 8.8 anyway. 

  2. +++ b/crop.module
@@ -186,7 +187,7 @@ function crop_file_url_alter(&$uri) {
       // will be encoded.
       // @see file_create_url()
-      $scheme = \Drupal::service('file_system')->uriScheme($uri);
+      $scheme = StreamWrapperManager::getScheme($uri);
       if ($scheme && !in_array($scheme, ['http', 'https', 'data'])) {
         if ($wrapper = \Drupal::service('stream_wrapper_manager')->getViaUri($uri)) {
           $uri = $wrapper->getExternalUrl();
diff --git a/tests/src/Functional/UpdatePathTest.php b/tests/src/Functional/UpdatePathTest.php

    lets update .info.yml then accordingly.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review
StatusFileSize
new 3115602-8.patch2.98 KB
new 3115602-8-interdiff.txt1.5 KB

Cleaning that up.

  • phenaproxima committed de11768 on 8.x-2.x 
    Issue #3115602 by phenaproxima, Berdir: Fix tests on Drupal 9
phenaproxima’s picture

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

Well, now that Drupal 9 is in beta, I think the time to land this has come. Committed and pushed to 8.x-2.x.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Reviewed & tested by the community » Fixed

I meant fixed.

Status: Fixed » Closed (fixed)

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

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented

Fyi, I was looking at the usage of core_version_requirement for various projects and see that the committed code used quotes around the versions whereas all the other projects (except one other) I've reviewed so far (about 50) do not use quotes and the change record doesn't use quotes.

https://git.drupalcode.org/project/crop/blob/de117680c5ce3a6fa0c3a7824ec...

core_version_requirement: '^8.8 || ^9'

Change record: https://www.drupal.org/node/3070687

Nitpick: Although this technically works fine, I would suggest updating it at some point to not include the quotes for consistency with the bulk of other projects.