Compatibilité PHP 5.6 → 8.5 + suite de tests de non-régression - #6
Compatibilité PHP 5.6 → 8.5 + suite de tests de non-régression#6mhoareau wants to merge 11 commits into
Conversation
Golden-master suite pinning the current observable behaviour of static.php so later PHP-compat work can be proven regression-free. Pure PHP, no Composer, no runtime deps; runs identically on PHP 5.6 -> 8.5. - Frozen wall clock (libfaketime) since the script reads time()/date(). - Fixed, prebuilt SQLite `archive` fixtures = byte-identical golden input for every PHP version (never rebuilt by the version under test). - Captures stdout + generated StatIC_<id>.txt + exit code vs approved snapshots; warnings/deprecations routed to stderr as informational (non-gating). - 6 scenarios: recent/stale, US/METRIC/METRICWX units, null sensors, and the calm-wind case that exposes the '' != 0 PHP7<->PHP8 change. Production code (static.php, config.php) is NOT modified (Phase 1 = characterize).
GitHub Actions workflow running the characterization suite across the whole
target range via shivammathur/setup-php. Versions where the legacy code already
diverges (8.0-8.5) are continue-on-error so the build stays globally green: the
red is a documented baseline finding, not a regression to fix in Phase 1.
Baseline: 5.6-7.4 fully pass; 8.0+ diverge only on the calm-wind direction case
('' != 0); 8.1+ additionally emit a getopt(null) deprecation (non-gating).
Move the whole script into functions run only via a CLI guard (`PHP_SAPI==='cli' && realpath($argv[0])===__FILE__`), so it can be included and unit-tested without executing main. Behaviour is preserved BYTE-FOR-BYTE: the characterization snapshots stay green on the whole legacy range (5.6-7.4). - Factor the unit-conversion logic (duplicated identically across the sqlite/mysql branches) into small pure functions, covered once. - Introduce seams: injectable clock (now), FTP client (Static_Ftp interface + Static_FtpExt real adapter + fakeable), DB handle, and file output; die()-on-FTP- failure becomes a signalled return with the SAME stdout and exit code. - Preserve the legacy quirks verbatim (not "fixed" here): gmdate (sqlite) vs date (mysql), and the SUM(rain) query using the table name without the db_name prefix. - Add --config=<path> (default ./config.php, legacy unchanged) and a readable STATIC_VERSION marker (does not change the .txt output). - No runtime dependency added: still two pure PHP files. Stays 5.6-safe; PHP 8.x compatibility is deliberately NOT addressed here (Phase 3).
Rename the tracked config.php to config.example.php and gitignore config.php, so an installer/user drops their own config.php (or points --config=<path> elsewhere) without dirtying the clone and without risking a real config being committed. The default path stays ./config.php, so existing setups are unaffected.
Reach 100% line AND branch coverage of static.php on the pinned legacy PHP (7.4, Xdebug), driving static_run() through the seams: - converters/report/wind-dir/upload/config covered by unit tests (FakeFtp for the FTP branches: success + connect/login failure, no server); - the sqlite path via the committed golden fixtures + injected clock, asserting the produced StatIC_<id>.txt equals the approved snapshot and that FTP fires iff fresh; - the mysql path against a real MariaDB (env-driven; skipped when absent); - two new scenarios, unit_unknown (converter fallthroughs) and empty_db (degenerate null propagation), added to the golden-master suite too. Paths are not gated (combinatorial on the sequential collectors); the thin Static_FtpExt adapter is @codeCoverageIgnore (its logic is covered via the fake). coverage_gate.php enforces 100% lines+branches. PHPUnit is dev-only; the shipped script keeps zero runtime dependencies.
…riaDB) New `coverage` job runs PHPUnit with a MariaDB service and enforces the 100% line+branch gate via coverage_gate.php, alongside the existing 5.6-8.5 matrix.
Make the whole matrix (PHP 5.6 -> 8.5) produce byte-identical output, without any
observable change on the legacy range (the characterization snapshots are unchanged).
- Wind direction on calm wind: `$avg_wind_10 != 0` -> `$avg_wind_10 !== '' && != 0`.
Keeps the V2.5 intent ("no direction when mean wind is 0/absent") on PHP 8, where
'' != 0 is now true (it was false in 7.x).
- getopt(null, ...) -> getopt('', ...): passing null to arg AssociationInfoclimat#1 is deprecated in 8.1+.
- Guard a missing --debug: isset(...) ? ... : '' (avoids "undefined array key" in 8.0+).
- Set the timezone before the first date() call (removes the warning emitted when
date.timezone is not set in php.ini).
- Empty database: gmdate((int)$stop, ...) -- gmdate(null) means "now" in PHP 8 vs epoch
0 in 7.x; forcing int keeps the legacy rendering (01/01/1970) consistent across versions.
Legacy quirks are still preserved verbatim (gmdate vs date per backend, the un-prefixed
SUM(rain) query). Stays 5.6-safe; 100% line+branch coverage maintained.
With the Phase 3 compatibility fixes every version now matches the approved snapshots, so the characterization matrix is fully required -- remove continue-on-error.
The Lint step still ran `php -l config.php`, but config.php was renamed to config.example.php (and gitignored) in Phase 2, so on a clean checkout the file is absent -> "Could not open input file: config.php" -> exit 1 -> under `bash -e` the Characterization step is skipped and the whole matrix goes red. Lint the tracked template instead. Workflow-only fix; no runtime file changes.
Document the PHP 5.6-8.5 compatibility work, the dependency-free test suite + CI matrix with 100% line/branch coverage, the new --config option with the config.example.php template, and the STATIC_VERSION marker. Appended after V2.5 to keep the changelog's existing chronological order (oldest first), matching the upstream convention.
The version= tag of the produced StatIC .txt was hard-coded to "weewx-<db_type>-2.5", frozen during the PHP 8 compatibility work while the tool moved to 2.6.0. Derive it from STATIC_VERSION in major.minor so it now reads "weewx-<db_type>-2.6" and can no longer drift from the tool version (single source of truth). Weather data and calculations are unchanged: the only snapshot delta is the version= line (and the matching "Version script :" debug line, same string). Unit/integration tests now derive the expected version from STATIC_VERSION too. README changelog nuanced accordingly.
|
Je n'ai plus le temps ni les capacités de dépoussiérer ce projet et vérifier les modifs, mais la description semble clair, merci pour ce travail ! |
| // VERSION - NE PAS MODIFIER !! | ||
| $version = "weewx-".$db_type."-2.5"; |
There was a problem hiding this comment.
C'est écrit NE PAS MODIFIER, pourquoi est-ce que c'est modifié ? (Je ne sais pas pourquoi ça ne doit pas être modifié)
There was a problem hiding this comment.
@jlecordier c'est devenu une constante plutôt que d'être hard-codé en dur comme je l'avais fait à l'époque, cf a1099d8
L'inscription "ne pas modifier", c'était pour éviter qu'un proprio de station ne s'amuse pas à le bouger sans savoir que ça peut avoir des conséquences : je pense d'ailleurs que côté IC ça servait lors du décodage des fichiers txt et ça doit même se retrouver dans un champ source quelque part en BDD :)
There was a problem hiding this comment.
Oui, j'ai hésité aussi, d'ailleurs l'IA n'a pas voulu le modifier. vu le commentaire 🤣
C'était une décision de ma part et je me suis dit que vous me taperiez dessus au besoin. Mais le changement de code était trop conséquent pour ne pas itérer une version, de mon point de vue.
Sauf si c'est une version du "format de fichier généré" et non pas du script lui-même.
Dans ce cas en effet, vu que mon code génère le même format, il faut le laisser en 2.5 et versionner le script différemment.
|
Bonjour @mhoareau, merci pour la PR Quelques questions au préalable :
Le |
|
Bonjour,
Puis une réorganisation de code (en fonction testables) était nécessaire pour couvrir 100% du code avec les tests (avec le premier filet de sécurité des premiers tests écrits). Au début, je voulais juste viser la dernière version de PHP, en me disant que ça serait de toute façon pour l'embarquer sur les stations de notre réseau (MétéoR OI) qui auront toutes des versions de PHP récentes. Mais... après réflexion, je me suis dit que si je le reverse, il faut que ça reste compatible avec votre réseau existant par sécurité, donc finalement PHP 5.6 à 8.5. Voilà un peu le cheminement qui a amené à cela.
Donc, pour le changement minimal, ça aurait été quelques lignes modifiées pour rendre le code compatible avec les nouvelles versions de PHP, sans warning, mais en rendant le code non compatible avec d'anciennes versions. Donc sans vous le reverser. C'est toujours mieux d'avoir du code testé (à 100%) pour s'assurer que lors de futurs évolutions (qui seront de toute façon obligatoires avec les prochaines version de PHP), on ne casse rien.
Concernant l'IA, les tests sont développés à 100% avec l'IA (extrêmement efficace pour cela), la réécriture à 90% avec l'IA. En grande majorité Claude Code pour le développement des tests et du code, et le duo Claude Code/Codex pour les code reviews.
Pour les prompts, je verrais si j'ai la possibilité/le temps de vous les ressortir, mais le plan étant rédigé en fichier temporaire et faisant le ménage très régulièrement dans les fichiers mémoires/historiques pour forcer l'IA à ne pas avoir de biais, pas sûr de pouvoir vous reconstituer cela, mais je vais voir ce que je peux retrouver.
Les snapshots on été reconstitués (simulés) à partir de ce qui était généré par le script original, ils permettent de s'assurer qu'au fil des modifications/réorganisations de code, le script sort toujours le fichier attendu avec les valeurs dans les formats attendus. Voilà pour les réponses |
|
Merci pour ces réponses détaillées Concernant :
Le code actuel ne fonctionnait pas du tout ? Ou c’était juste des warnings ? Concernant :
Pourrais-tu me refaire une PR avec ces corrections minimales ? Elle ne sera pas forcément merged, mais c'est pour que je puisse y jeter un œil. Pourquoi est-ce que le nouveau code serait incompatible avec les anciennes versions ? Si ça rend incompatible, comment fait cette PR pour que ce soit compatible avec toutes les versions du coup ? |
|
Une autre solution serait d'avoir deux repos ou deux dossiers, un pour la version legacy, un avec ton nouveau code, comme ça on prend zéro risque, les déploiements actuels ne sont pas impactés, et les plus courageux peuvent utiliser la nouvelle version. Je suis vraiment une brêle en déploiement, donc j'essaie de limiter les impacts 😛 |
3 participants
Compatibilité PHP 5.6 → 8.5 + suite de tests de non-régression (sans casser les stations existantes)
Le script
static.phpn'avait plus évolué depuis 2020 (PHP 7.x) et présentait plusieursincompatibilités avec PHP 8. Comme il tourne sur de nombreuses stations encore installées
sur d'anciennes versions de PHP, l'objectif de cette PR a été de le rendre compatible du legacy
au plus récent, sans jamais changer le comportement sur les versions où il fonctionnait déjà.
Pour garantir ce « sans régression », le travail a suivi une méthode de reprise de code hérité en
trois étapes, chacune vérifiable indépendamment. Rien n'a été modifié « à l'aveugle ».
Étape 1 — Mettre le comportement actuel sous test (aucune modification de code)
Ajout d'une suite de non-régression « golden master » + une CI, sans toucher à
static.phpniconfig.php:Il exécute le script contre des bases SQLite contrôlées (fixtures figées), horloge gelée
(
libfaketime, car le script littime()/date()), et compare stdout +StatIC_<id>.txt+code de sortie à des snapshots approuvés. 8 scénarios : relevé récent / ancien (> 20 min),
unités US / METRIC / METRICWX, valeurs nulles, vent calme, unité inconnue, base vide.
.github/workflows/ci.yml: matrice PHP 5.6 → 8.5 (shivammathur/setup-php) +php -l.Constat documenté (code inchangé) : identique de 5.6 à 7.4 ; sur 8.0+, deux divergences
observables — la direction du vent par vent calme et le rendu de date sur base vide — corrigées
à l'étape 3 et prouvées par ces mêmes snapshots.
Étape 2 — Rendre le script testable (refactor minimal, comportement inchangé)
static.phpdevient incluable (garde CLIPHP_SAPI==='cli' && realpath($argv[0])===__FILE__) ;les I/O (horloge, FTP, base, écriture de fichier) sont injectables pour être testées sans
serveur réel.
est factorisée en fonctions communes.
via un service MariaDB en CI), seuil imposé dans la CI.
Étape 3 — Compatibilité PHP 5.6 → 8.5 (sans régression legacy)
Chaque correctif est borné à la compatibilité ; les snapshots de l'étape 1 garantissent qu'aucun
comportement observable ne change sur les anciennes versions (matrice CI entièrement verte 5.6 → 8.5).
Tous les correctifs sont 5.6-safe :
if ($avg_wind_10 != 0)→if ($avg_wind_10 !== '' && $avg_wind_10 != 0).Conserve l'intention de V2.5 (« pas de direction si le vent moyen est nul/absent ») sur PHP 8, où
'' != 0vaut désormaistrue.getopt(null, …)→getopt('', …): passernullau 1er paramètre est déprécié en 8.1+.--debugabsent :isset($options['debug']) ? … : ''(évite « undefined array key » en 8.0+).date()(supprime un warning quanddate.timezonen'est pas défini dans
php.ini).gmdate((int)$stop, …)—gmdate(null)vaut « maintenant » en PHP 8 vs epoch 0 en7.x ; on force un rendu cohérent, identique au legacy.
Ajouts additifs (rétro-compatibles)
--config=<chemin>(défaut./config.php, comportement inchangé sans l'option) ;config.phpest désormais ignoré par git avec un modèleconfig.example.php— les réglagesne sont plus écrasés à la mise à jour, et l'outil peut charger sa config depuis un chemin arbitraire.
STATIC_VERSION; la ligneversion=du.txten dérive désormais(
weewx-<db>-<major.minor>) au lieu d'un littéral codé en dur — une seule source de vérité,plus de désynchronisation entre la version de l'outil et le tag du fichier. (La valeur exacte
reste votre choix : c'est
STATIC_VERSIONqui la pilote.)Extensions
ext-mysqlietext-ftprestent requises.ext-ftpn'est pas retirée de PHP jusqu'en 8.5(elle est simplement absente de certaines images Docker par défaut) : aucune bascule vers cURL
n'est nécessaire.
Comment vérifier
La CI rejoue automatiquement la matrice 5.6 → 8.5 + la porte de couverture 100 %.
Le runtime reste deux fichiers PHP purs (
static.php+config.example.php), sans aucunedépendance à l'exécution — PHPUnit est un outil de développement uniquement.
Merci de votre relecture — je reste à disposition pour ajuster (découper en PR plus petites,
renommer la version, etc.).