8000 Make tests compatible with newer phpunit versions by williamdes · Pull Request #7727 · matomo-org/device-detector · GitHub 10BC0
[go: up one dir, main page]
Skip to content

Make tests compatible with newer phpunit versions #7727

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.

Sign up for GitHub

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

Jump to bottom
Merged
sanchezzzhak merged 11 commits into from
Dec 15, 2025
Merged

Make tests compatible with newer phpunit versions #7727

sanchezzzhak merged 11 commits into from
Dec 15, 2025
+152 −47

Conversation

@williamdes
Copy link
Contributor
@williamdes williamdes commented Jul 2, 2024 •
edited
Loading

Description:

  • Non breaking changes for newer phpunit versions (running on PHPUnit 11.1.3 at Debian)
  • Fixing the parameter names for newer PHP versions (Error: Unknown named parameter $check)
  • Upgrade GitHub actions
  • Allow newer PHPunit versions up to 12
  • Fix deprecations in tests

Review

@williamdes

This comment was marked as resolved.

Sign in to view
@sgiehl
Copy link
Member
sgiehl commented Jul 2, 2024

Why should the variable name need to contain a _? What is the problem with useragent? This doesn't comply with our coding style.

@williamdes

This comment was marked as resolved.

Sign in to view
@williamdes

This comment was marked as resolved.

Sign in to view
sanchezzzhak
sanchezzzhak reviewed Jul 12, 2024
View reviewed changes
@williamdes williamdes marked this pull request as draft July 17, 2024 10:41
@williamdes williamdes force-pushed the phpunit-upgrade branch 3 times, most recently from 76b7e67 to 7298a44 Compare July 17, 2024 11:19
@williamdes williamdes marked this pull request as ready for review July 17, 2024 11:19
@williamdes williamdes requested a review from sanchezzzhak July 17, 2024 11:20
@williamdes
Copy link
Contributor Author
williamdes commented Jul 17, 2024 •
edited
Loading

Weird CI failures, new commits broke something or is it my diff ?

@liviuconcioiu
Copy link
Collaborator
liviuconcioiu commented Jul 17, 2024 •
edited
Loading

Not sure, but probably the PHPUnit workflow should be updated too so it can use composer update? Now it tries to run PHPUnit 11 on PHP 7.2.

I installed PHP 7.2.5 (this is the lowest version required by symfony/yaml 5.1.7) and the tests pass.

@williamdes
Copy link
Contributor Author
williamdes commented Jul 18, 2024

Not sure, but probably the PHPUnit workflow should be updated too so it can use composer update? Now it tries to run PHPUnit 11 on PHP 7.2.

I installed PHP 7.2.5 (this is the lowest version required by symfony/yaml 5.1.7) and the tests pass.

Oh snap, let's revert that part. I only need the PHP diff anyway
PRs like #7570 can update that

williamdes
williamdes commented Jul 18, 2024
View reviewed changes
@williamdes
Copy link
Contributor Author
williamdes commented Jul 18, 2024

I dropped the last commit, it should help

@liviuconcioiu liviuconcioiu mentioned this pull request Jul 18, 2024
Closed
@liviuconcioiu
Copy link
Collaborator
liviuconcioiu commented Dec 20, 2024

@williamdes can you fix the conflicts? And also you should also add static to getFixturesDeviceTypeFromClientHints. It was added after your PR.

@williamdes williamdes force-pushed the phpunit-upgrade branch 2 times, most recently from fa38ec4 to 5ad901f Compare December 22, 2024 19:45
@williamdes
Copy link
Contributor Author
williamdes commented Dec 22, 2024

I did some ajustements as I am quite sure the ignore should not have been used for all versions and this way.

@sanchezzzhak
Copy link
Collaborator
sanchezzzhak commented Dec 24, 2024

Run local php vendor/bin/phpcbf for fix phpcs

@williamdes

This comment was marked as resolved.

Sign in to view
@liviuconcioiu
Copy link
Collaborator
liviuconcioiu commented Mar 25, 2025

@williamdes is it possible to update the workflow so it does look like this?

ok

Right now it looks like this:

notok

Those are missing (pending but aren't running):

PHPUnit (windows-latest, 8.4)
PHPUnit (macos-latest, 8.4)

2

@williamdes
Copy link
Contributor Author
williamdes commented Apr 12, 2025 •
edited
Loading

@williamdes is it possible to update the workflow so it does look like this?

Not really (I have no options to change the text), can it be merged as is @sgiehl ?
I have one more PR for phpunit 12 afterwards

@williamdes
Copy link
Contributor Author
williamdes commented May 16, 2025

Hi @sgiehl
Can you check this one please ?

sgiehl
sgiehl reviewed May 16, 2025
View reviewed changes
@williamdes williamdes requested a review from sgiehl December 8, 2025 22:37
@williamdes

This comment was marked as resolved.

Sign in to view
@liviuconcioiu
Copy link
Collaborator
liviuconcioiu commented Dec 9, 2025

  1. You should run ./vendor/bin/phpcbf to fix the phpcs errors - https://github.com/matomo-org/device-detector/actions/runs/20045151285/job/57507861303?pr=7727

  2. You can add to composer, so we get rid of scrapbook deprecations

        "psr/cache": "^1.0.1 || ^2.0 || ^3.0",
        "psr/simple-cache": "^1.0.1 || ^2.0 || ^3.0",
Lock file operations: 0 installs, 3 updates, 0 removals
  - Upgrading matthiasmullie/scrapbook (1.4.9 => 1.5.4)
  - Upgrading psr/cache (1.0.1 => 3.0.0)
  - Upgrading psr/simple-cache (1.0.1 => 3.0.0)
  1. On PHP 8.5 there are some deprecations - https://github.com/matomo-org/device-detector/actions/runs/20045151287/job/57507861394?pr=7727
3) E:\GitHub\device-detector\Tests\Parser\Client\BrowserTest.php:118
Method ReflectionProperty::setAccessible() is deprecated since 8.5, as it has no effect

4) E:\GitHub\device-detector\Tests\Parser\Client\BrowserTest.php:122
Method ReflectionProperty::setAccessible() is deprecated since 8.5, as it has no effect

5) E:\GitHub\device-detector\Tests\Parser\Client\BrowserTest.php:145
Method ReflectionMethod::setAccessible() is deprecated since 8.5, as it has no effect

Probably is a good idea to add this

if (PHP_VERSION_ID < 80500) {
    $browserProperty->setAccessible(true);
}

@williamdes
Copy link
Contributor Author
williamdes commented Dec 9, 2025 •
edited
Loading

Thank you @liviuconcioiu !
Can someone approve the workflows ?

Edit: phpcs is fixed, all will go green now

@sanchezzzhak
Copy link
Collaborator
sanchezzzhak commented Dec 15, 2025

The tests are completed.
The code has been verified.
Everything looks good, I'm combining it.
Thank you all for involved.

@sanchezzzhak sanchezzzhak enabled auto-merge (squash) December 15, 2025 09:59
@sanchezzzhak sanchezzzhak merged commit 31a4c59 into matomo-org:master Dec 15, 2025
19 checks passed
@williamdes williamdes deleted the phpunit-upgrade branch December 17, 2025 08:56
@williamdes
Copy link
Contributor Author
williamdes commented Dec 17, 2025

Can you tag a new release some time soon so I can import the version into Debian?

@liviuconcioiu
Copy link
Collaborator
liviuconcioiu commented Jan 21, 2026

Can you tag a new release some time soon so I can import the version into Debian?

@williamdes a new version has just been released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sanchezzzhak sanchezzzhak sanchezzzhak approved these changes

@sgiehl sgiehl Awaiting requested review from sgiehl

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@williamdes @sgiehl @liviuconcioiu @sanchezzzhak
0