Add PHP 8.1 tests

Created on 25 August 2025, 20 days ago

Problem/Motivation

We saw in 🐛 Support PHP 8.1 Active that the PHP 8.1 standards where not followed and we missed this. While we do code review, these are things that static code analysis tools like PHPStan does and should do automatically.

Gitlab CI has a previous major that runs 10.5.x and PHP 8.1. We should turn this on.

Proposed resolution

Turn on previous major in testing.
Check for any failure and fix it.

📌 Task
Status

Active

Version

1.2

Component

Code

Created by

🇩🇪Germany marcus_johansson

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @marcus_johansson
  • First commit to issue fork.
  • Merge request !176Resolve #3542950 "Add php 8.1" → (Open) created by ankitv18
  • Pipeline finished with Failed
    16 days ago
    Total: 244s
    #584971
  • Pipeline finished with Failed
    16 days ago
    Total: 204s
    #584979
  • Pipeline finished with Failed
    16 days ago
    Total: 197s
    #584989
  • Pipeline finished with Failed
    16 days ago
    Total: 400s
    #584995
  • Pipeline finished with Success
    16 days ago
    Total: 163s
    #585003
  • 🇮🇳India ankitv18

    MR!176 is ready for a review

  • 🇩🇪Germany jurgenhaas Gottmadingen

    There are lots of changes in test files that seem unrelated to this issue.

    And the dev dependency on bpmn_io 3.0 is deliberate. The UI part really only works with that version and the older bpmn_io doesn't contain what's required. Background is that modeler_api provides the UI for ai_agents for all Drupal versions. But editing them with bpmn_io is only support for Drupal 11.2 and later, hence the 3.0 dependency.

    That also means that the CI config needs to remain what it was.

    The MR should then be reduced to a single line change in the Agent.php plugin.

  • 🇮🇳India ankitv18

    Then in that case we should not enable previous major and if we want to run on previous major then we should restrict the module to install on php 8.3+ application.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    Previous major is not enabled in the CI configuration. But the module needs to run on Drupal 10, which it does. It just can't be tested on D10, unless we are going to ignore some of the PHPStan error messages that would arise when bpmn_io is not available. This probably needs input from @marcus_johansson to decide which way to go.

  • 🇩🇪Germany marcus_johansson

    The test changes are needed - the group attribute was added as part of the PHPUnit version that is shipped with D11. So this is correct.

    Regarding testing version, I assume that the issue is that we can not run the composer (previous major) with D10+ with bpmn.io. I'm not sure if we could add a specific way of running that without bpmn.io and then catch the errors as mentioned?

  • 🇩🇪Germany marcus_johansson

    500 error duplicated my comments

  • 🇩🇪Germany jurgenhaas Gottmadingen

    Yes, ignoring those errors is possible by adding those error messages to the ignoreErrors in phpstan.neon. As those ignored errors will not happen in D11, this in itself will be an error (ignoring something that not an error), and therefore we also need a setting reportUnmatchedIgnoredErrors: false in that file too.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    I've just had another look and realized that we don't require the drupal/bpmn_io dev dependency any longer, after the latest refactoring. So, that can be removed from the require-dev section but should remain in the recommend section. After that, tests should be working just fine without any extras in phpstan.neon.

    Sorry for the confusion here, I totally ignored the positive side-effect of that refactoring we did before it went into 1.2.x

Production build 0.71.5 2024