matomo-org
Make tests compatible with newer phpunit versions
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
- [x] Functional review done
- [x] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
- [x] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
- [x] Security review done
- [x] Wording review done
- [x] Code review done
- [x] Tests were added if useful/possible
- [x] Reviewed for breaking changes
- [x] Developer changelog updated if needed
- [x] Documentation added if needed
- [x] Existing documentation updated if needed
Variable "user_agent" is not in valid camel caps format
Can someone give me the best way to fix this. Not by changing the variable name for sure 😄
Why should the variable name need to contain a _? What is the problem with useragent? This doesn't comply with our coding style.
Why should the variable name need to contain a
_? What is the problem withuseragent? This doesn't comply with our coding style.
Nope, because as you can see getTypeMethodFixtures is directly fed from \Spyc::YAMLLoad($fixturePath);
So the variable names should match the yaml data key names
One solution is that I can put an array map to re-format the key name
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.
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 bysymfony/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 can you fix the conflicts? And also you should also add static to getFixturesDeviceTypeFromClientHints. It was added after your PR.
I did some ajustements as I am quite sure the ignore should not have been used for all versions and this way.
Run local php vendor/bin/phpcbf for fix phpcs
Unrelated, see CI log
PHP Parse error: syntax error, unexpected '|', expecting variable (T_VARIABLE) in /home/runner/work/device-detector/device-detector/vendor/phpunit/phpunit/src/Framework/Assert/Functions.php on line 83
Probably some issue with the php version installed
Edit: pushed a fix, maybe it will work
@williamdes is it possible to update the workflow so it does look like this?
Right now it looks like this:
Those are missing (pending but aren't running):
PHPUnit (windows-latest, 8.4) PHPUnit (macos-latest, 8.4)
@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