Problem/Motivation

One of the advantages of GitLab CI is the ability to test against multiple Drupal versions.

This change is to adopt tests for the previous and next major versions of Drupal.

It also tests against the maximum supported PHP version, so while the default tests run on 8.1, this will also test against PHP 8.3.

We also have the ability to test against the previous and next major releases. Since we don't support Drupal 9, there's no point in testing against previous Major. We can look at testing against next major after this, but that will require checking compatibility of dependencies.

Steps to reproduce

N/A

Proposed resolution

  • Enable testing against previous and next minor releases (currently 10.3)
  • RendererInterface::renderPlain() is deprecated
  • Test failure of: CoreExperimentalLabelTest::testCoreExperimentalLabel
  • Text changed in 10.3 Module Project Browser has been enabled installed
  • #3450499: project_browser_test.time service tests fail on 10.3

Issue fork project_browser-3423697

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

lostcarpark created an issue. See original summary.

lostcarpark opened merge request !439

lostcarpark’s picture

lostcarpark commented
Title: GitLab CI - test for previous and next minor versions » GitLab CI - test for previous and next minor versions, and maximum PHP version
Issue summary: View changes
chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Issue tags: +core-mvp, +stable blocker
sime’s picture

sime
any/any
Location Melbourne
commented
Issue summary: View changes
sime’s picture

sime
any/any
Location Melbourne
commented
Related issues: +#3450499: project_browser_test.time service tests fail on 10.3
sime’s picture

sime
any/any
Location Melbourne
commented 
 ------ ----------------------------------------------------------------------- 
  Line   src/Controller/InstallerController.php                                 
 ------ ----------------------------------------------------------------------- 
  311    Call to deprecated method renderPlain() of class                       
         Drupal\Core\Render\Renderer:                                           
         in drupal:10.3.0 and is removed from drupal:12.0.0. Use                
           \Drupal\Core\Render\RendererInterface::renderInIsolation() instead.  
 ------ -----------------------------------------------------------------------

The fix for this is suggested in comments https://www.drupal.org/node/3407994, please confirm if you want to proceed with this.

sime’s picture

sime
any/any
Location Melbourne
commented
Issue summary: View changes
sime’s picture

sime
any/any
Location Melbourne
commented

I've listed the failures in the Proposed Resolution section. The main thing i don't know is how to variegate a test for two different outcomes - eg where the status message has changed after you install a module.

sime’s picture

sime
any/any
Location Melbourne
commented
Status: Active » Needs work

I temporarily disabled OPT_IN_TEST_PREVIOUS_MINOR coverage (10.1 becomes unsupported July) just to have better clarity on what needs to be done.

I have fixed everything except InstallerControllerTest::testInstallUnlockMessage. I don't know what is happening there on 10.3. It should be running into a lock in this test but nothing is returned from the tempstore.

Note that I merged the fixes on #3450499: project_browser_test.time service tests fail on 10.3 into this (to make it easier to track progress), so please merge that first to give credit to @kimpepper and @mstrelan.

sime’s picture

sime
any/any
Location Melbourne
commented
Related issues: +#3446528: Run tests on 11.x and declare compatibility with it

Adding relationship to #3446528: Run tests on 11.x and declare compatibility with it as there is overlapping work.

sime’s picture

sime
any/any
Location Melbourne
commented
Status: Needs work » Needs review

Passing, I picked a few things that seemed like improvements/fixes over head. Please also credit kimpepper and mstrelan for the deprecation helper help.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Needs work

I love that DeprecationHelper thing. Wish I'd known about it before! But hey, you gotta learn sometime.

The only thing is that we shouldn't be using Workspaces to test experimental stuff.

sime’s picture

sime
any/any
Location Melbourne
commented

I'm not sure about Workspaces, the test is pre-existing and seems to just be a canary test. But now that Workspaces is marked stable in 10.3 do we even need this test?

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Yes. We do want to test that Project Browser handles experimental modules in a particular way. The use of Workspaces was probably just a convenience thing at the time.

sime’s picture

sime
any/any
Location Melbourne
commented

Ok i see where I've misunderstood the purpose of the test. Lemme fix.

sime’s picture

sime
any/any
Location Melbourne
commented
Status: Needs work » Needs review
phenaproxima’s picture

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

This rocks!

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Reviewed & tested by the community » Fixed

when I bump you bump we bump (version numbers)

Status: Fixed » Closed (fixed)

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