Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XDMoD 11.0 PHP 8.1 Changes #1862

Draft
wants to merge 26 commits into
base: main
Choose a base branch
from
Draft

Conversation

ryanrath
Copy link
Contributor

@ryanrath ryanrath commented Jun 5, 2024

Description

Motivation and Context

Tests performed

Checklist:

  • The pull request description is suitable for a Changelog entry
  • The milestone is set correctly on the pull request
  • The appropriate labels have been added to the pull request

eiffel777 and others added 24 commits June 12, 2024 09:56
- acl-config
  - `join` function signature has changed in PHP8
- RealmManager.php
  - The order in which these values were being returned differed between PHP7.2
    and PHP8.0. I've just added code to force the ordering to be as we expect.
- ExportBuilder.php
  - PHP8 is more strict in what it allows in function signatures and optional
    arguments (arguments with default values) cannot come before required
    arguments (arguments without default values).
- Visualization.php
  - HSVtoRGB: PHP7.2 did more coercing of types than PHP8 does so we needed an
    explicit cast to int here.
- pdoAggregator.php
  - The previous code here expected that `current` would always return an
    `array` when it can also return a `bool`. I've just updated the code to take
    this into account.
- HpcdbHostsIngestor.php
  - updated the function signature of `transform` so that it matches it's parent
    `pdoIngestor`
- aRdbmsDestinationAction.php
  - This code previously assumed that `$item['alias']` was going to be an array
    and that it would contain a key 'as'. I just made it explicit.
- composer.json
  - Added PHP8.0 to the list of supported PHP's.
  - Updated phpoffice/phpword's version to * because the previous version
    doesn't support PHP8
  - Updated PHPUnit to 9+ as it was the most recent version that supported
    PHP8.0

PHPUnit9+ changes

The "types" of changes in this commit are all related to PHPUnit9+ and PHP8:
- PHP8 reports a larger mantissa than in PHP7.2 so a few values have been
updated to reflect that.
- PHPUnit9+ uses actual namespacing so anywhere there was
  `PHPUnit_Framework_TestCase` has been changed to
  `\PHPUnit\Framework\TestCase`. The same basic idea was applied to Errors and
  Exceptions as well.
- PHPUnit9+ has added some new functions, deprecated others, and removed yet
  others. There are numerous locations where substitutions for curently
  recommended functions were made.
- phpunit.xml.dist files have been updated

Documentation Changes
- The upgrade documentation has been upgraded to reflect the update of PHP to
  8.0. Upgrade scenarios include:
  - XDMoD 10.0 on EL7 w/ PHP 5.4
  - XDMoD 10.5 on EL7 w/ PHP 7.2
  - XDMoD 10.5 on EL8 w/ PHP 7.2

Co-authored-by: Joe White <[email protected]>
In the interest of keeping things the same I'm adding the composer dep
that will provide the PHPUnit `assertArraySubset` functionality that we
were using prior to it being removed.
This has been moved to `build.json`
These changes are to address deprecation notices generated by running
the unit tests.
So this test currently errors out because the functions that are being
mocked have a return type of `void` so it doesn't make sense to say they
will return `null`.
I'd like to see if the UI tests pass.
@ryanrath ryanrath force-pushed the xdmod11-php8-bkup branch from 71e5e76 to c2ab373 Compare June 12, 2024 13:59
@aaronweeden
Copy link
Contributor

When working on #1852, I noticed that PHP 8.1 has a new array_is_list() function. On this line, I would suggest changing array_keys($value) !== range(0, count($value) - 1) to !array_is_list($value).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants