fix(test): prevent error by upgrading suites #512
Conversation
joerivanveen
force-pushed
the
fix/test/prevent-error-by-upgrading-suites
branch
from
6ce52e9 to
4965d42
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #512 +/- ##
============================================
- Coverage 59.04% 58.91% -0.14%
+ Complexity 1804 1636 -168
============================================
Files 110 112 +2
Lines 4283 4756 +473
============================================
+ Hits 2529 2802 +273
- Misses 1754 1954 +200 ☔ View full report in Codecov by Sentry. |
composer.json
Outdated
| }, | ||
| "require": { | ||
| "php": ">=7.1.0" | ||
| "php": ">=7.4.0" |
There was a problem hiding this comment.
Dit is een breaking change. In de README staat dat we momenteel PHP 7.1 ondersteunen. Voor klanten die < 7.4 hebben zou dan ineens de package niet meer installable of updateable zijn.
There was a problem hiding this comment.
Ja weet even niet waarom deze is aangepast, zal ik terugzetten.
| @@ -33,7 +33,7 @@ jobs: | |||
| if: github.actor != 'dependabot[bot]' && steps.cache-coverage.outputs.cache-hit != 'true' | |||
| id: docker | |||
| with: | |||
| image: ghcr.io/myparcelnl/php-xd:7.2 | |||
| image: ghcr.io/myparcelnl/php-xd:8.2 | |||
There was a problem hiding this comment.
Eigenlijk was de testsuite al niet geweldig want
A) We testen niet op alle PHP versies
B) We testen alleen op PHP 7.2 en niet op 7.1 welke volgens onze composer & README wel zou moeten werken
Hoe dan ook skippen we hier nu zoveel PHP versies dat het onmogelijk is om te weten of de boel in PHP < 8.2 nog werkt. Ik zou dit dan ook niet willen veranderen naar iets anders dan wat we in onze composer.json als versie meegeven (zie mijn comment daarbij)
There was a problem hiding this comment.
Het klopt dat die niet geweldig is, wat ook niet geweldig is is dat die api verzoeken doet met random data, waardoor sommige tests soms falen maar niet altijd. Ik denk niet dat ik in een story van 0 punten dat allemaal kan fixen, daarom koos ik voor een upgrade zodat alles weer werkte en de andere SDK stories door kunnen.
| @@ -2,7 +2,7 @@ version: '3.9' | |||
|
|
|||
| # volumes are not in common, because phpstorm doesn't understand it | |||
| x-common: &common | |||
| image: ghcr.io/myparcelnl/php-xd:7.2 | |||
| image: ghcr.io/myparcelnl/php-xd:8.2 | |||
There was a problem hiding this comment.
Ditto als mijn opmerking in de test workflow, alleen dan toepasbaar op lokale ontwikkeling impact.
There was a problem hiding this comment.
zelfde antwoord
BREAKING CHANGE: Bumps minimal required PHP version to 8.2
INT-743