Just for fun, let's test against D9. This patch adds a composer.json to allow the testbot to test against D9 because just changing the .info.yml in a patch is not sufficient for the testbot. I know there is no deprecated code in this module, but I'd like to identify anything else that may be preventing this module from being D9 compatible.

Comments

TR created an issue. See original summary.

tr’s picture

StatusFileSize
new2.61 KB

OK, just the $defaultTheme thing which was changed in D8.8 (see https://www.drupal.org/node/3083055). We can make that change now, since it's compatible with older versions of Drupal 8.

tr’s picture

StatusFileSize
new2.31 KB
tr’s picture

StatusFileSize
new3.03 KB

Getting closer ...

tr’s picture

StatusFileSize
new6.91 KB

The version of PHPUnit used by D9 is more strict than the D8 version, so we need to modify the test cases accordingly. Specifically, assertTrue() can only be used to test a boolean value - using it to test a numeric value will fail, as you can see from the test output.

tr’s picture

StatusFileSize
new7.36 KB

This is getting annoying. Same sort of issue - this time assertFalse() can't be used on a string value in D9.

tr’s picture

StatusFileSize
new7.71 KB

I missed one assertFalse().

tr’s picture

OK, so let me summarize the above. Bottom line: This module is fully D9 compatible with only a few minor changes to metadata and tests.

The patch in #7 does a few things:

  1. Modifies .info.yml to declare that the module supports both Drupal 8.7.7+ and Drupal 9
  2. Adds a composer.json because the testbot can't test changes to the .info.yml. After the changes to .info.yml are committed, the composer.json isn't needed. But going forward into D9 having a composer.json is a good idea.
  3. Adds $defaultTheme to Functional tests, which is required starting with D9.
  4. Fixes the Functional tests so that assertTrue() and assertFalse() are only used to test boolean expressions.

I propose to commit the patch in #7 then turn on automated testing for Drupal 9 and add a statement of Drupal 9 compatibility on the project page.

  • TR committed 0732b01 on 8.x-1.x
    Issue #3118827 by TR: Make module compatible with Drupal 9
    
tr’s picture

Status: Needs review » Fixed

Committed.

jordik’s picture

Well done @TR! Thank you!

tr’s picture

Status: Fixed » Needs review
StatusFileSize
new611 bytes

As a follow-up, the weekly testing against D9 that I turned on is now showing an error as of yesterday. Not sure why this didn't show up in the above tests, but it's there now. The error is:
The 'core_version_requirement' key must be present in modules/contrib/views_aggregator/views_aggregator_more_functions/views_aggregator_more_functions.info.yml
and indeed this is a change that I should have included as part of task 1 in comment #8.

Here's the follow-up patch:

  • TR committed 7b2ee0d on 8.x-1.x
    Issue #3118827 by TR: Follow-up to "Make module compatible with Drupal 9...
tr’s picture

Status: Needs review » Fixed

Committed.

Status: Fixed » Closed (fixed)

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