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

QueryDownloadControllerTest throws deprecation error #4104

Merged
merged 6 commits into from Jan 26, 2024

Conversation

paul-m
Copy link
Contributor

@paul-m paul-m commented Jan 25, 2024

Under PHP 8.2, deprecation messages related to the $dataDictionaryFields property sneak their way into the output buffer and throw off the test results.

  • Addresses all deprecation errors related to \Drupal\Tests\datastore\Unit\Controller\QueryDownloadControllerTest.
    • queryResultCompare() doesn't need to add dataDictionaryFields to the stream response.
    • Deprecated ['self', 'method'] callables changed to [self::class, 'method'] style.
  • Removes the unused queryResultReformatted() method.
  • Removes the unused getSelectFactory() method and associated code in __construct().

PHP 8.2 is not in the testing matrix because we need to fix this test before we can merge #4085

To review this PR:

  • Make a local dkan site with ddev.
  • Change the PHP version to 8.2.
  • ddev composer update
  • ddev dkan-phpunit --filter QueryDownloadControllerTest
  • You should see no deprecation messages and all tests should pass.

@paul-m paul-m marked this pull request as ready for review January 25, 2024 18:28
Copy link
Member

@dafeder dafeder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit concerned that the test still passes without dataDictionaryFields. I mean the way the property was being added it was never doing anything so it makes sense removing it has no effect -- it needed to be added to the JSON content not as an arbitrary property on the HTTP response object. So presumably there is some feature of data dictionaries lacking test coverage that we thought we had. I think for now it probably just is what it is though.

@dafeder dafeder merged commit 5a99938 into 2.x Jan 26, 2024
12 checks passed
@dafeder dafeder deleted the 18614-querydownloadcontrollertest-php82 branch January 26, 2024 20:24
@paul-m paul-m mentioned this pull request Jan 29, 2024
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.

None yet

2 participants