Tests cassés sous Windows

[MattiSG]

Hello hello !

Je suis le fan numéro un d'OpenFisca, mais je viens de rencontrer un problème.

Qu'ai-je fait ?

Intégrer #1838 dans master.

À quoi m'attendais-je ?

Les tests passent sur master, comme ils l'ont fait dans la pull request.

Que s'est-il passé en réalité ?

Les tests échouent sous Windows avec l'erreur suivante :

foyer_fiscal = <openfisca_core.populations.group_population.GroupPopulation object at 0x000001A24010A760>
ir_tranche = array([0])
parameters = <bound method TaxBenefitSystem.get_parameters_at_instant of <openfisca_france.france_taxbenefitsystem.FranceTaxBenefitSystem object at 0x000001A25B9C6A90>>
period     = Period(('year', Instant((2018, 1, 1)), 1))
taux_effectif = array([0.], dtype=float32)

C:\Miniconda\envs\true\lib\site-packages\openfisca_france\model\prelevements_obligatoires\impot_revenu\ir.py:1116: AttributeError
=========================== short test summary info ===========================
FAILED tests/formulas/ir_tranche_et_taux_marginal.yaml:: - AttributeError: 'M...
!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!
=========== 1 failed, 872 passed, 67 warnings in 319.21s (0:05:19) ============

J'ai essayé de relancer les tests, ils échouent à nouveau. Ils échouent également après l'intégration de #1846.

Voici des informations qui peuvent aider à reproduire le problème :

#1786 a introduit le test test-on-windows, mais ne l'exécute qu'après publication sur Conda, donc uniquement après publication sur master. Il semblerait que #1838 ait cassé quelque chose sous Windows uniquement, mais ces tests ne sont exécutés que sur master.

Contexte

Je m'identifie plus en tant que :

• [x] Contributeur·e : je contribue à OpenFisca France.
• [x] Développeur·e : je crée des outils qui utilisent OpenFisca France.
• [ ] Économiste : je réalise des simulations avec des données.
• [x] Mainteneur·e : j'intègre les contributions à OpenFisca France.
• [ ] Autre : (ajoutez une description du contexte dans lequel vous utilisez OpenFisca).

[benoit-cty]

Pour compléter, test-on-windows est là pour valider l'utilisation du paquet Conda. Sur mon poste Windows ce test passe, il n'y a donc pas d'incompatibilité réel avec Windows, je n'arrive pas à reproduire le problème, raison pour laquelle https://github.com/openfisca/openfisca-france/pull/1863 n'avance pas...

[MattiSG]

Merci de donner cette référence que je n'avais pas !

[sandcha]

Problème reproduit sur macOS avec Miniconda (conda 4.12.0 ) :

$ openfisca test -c openfisca_france tests/formulas/ir_tranche_et_taux_marginal.yaml

...

        return (
>           (taux_effectif == 0) * bareme.rate_from_bracket_indice(ir_tranche)
            + taux_effectif
            )
E       AttributeError: 'MarginalRateTaxScale' object has no attribute 'rate_from_bracket_indice'

...

/usr/local/Caskroom/miniconda/base/envs/conda-py3913-env/lib/python3.9/site-packages/openfisca_france/model/prelevements_obligatoires/impot_revenu/ir.py:1116: AttributeError
=============================================================== short test summary info ================================================================
FAILED tests/formulas/ir_tranche_et_taux_marginal.yaml:: - AttributeError: 'MarginalRateTaxScale' object has no attribute 'rate_from_bracket_indice'
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
============================================================= 1 failed, 1 warning in 0.37s =============================================================

Testé sous Python 3.9.13 (version en cours sur le CASD) dans un environnement virtuel construit avec le minimum pour permettre l'exécution de ce test (versions de librairies gérées automatiquement et premier test de tests/formulas/ir_tranche_et_taux_marginal.yaml) :

conda create -n conda-py3913-env python=3.9.13
conda activate conda-py3913-env

# depuis openfisca-core/
python openfisca_core/scripts/openfisca_command.py 

# depuis openfisca-france/
conda install openfisca-core
conda install pytest
conda install openfisca-france
openfisca test -c openfisca_france tests/formulas/ir_tranche_et_taux_marginal.yaml

D'où :

(conda-py3913-env) [sch@msch openfisca-france]$ pip list
Package           Version
----------------- --------
attrs             22.1.0
dpath             2.0.6
iniconfig         1.1.1
nptyping          1.4.4
numexpr           2.8.3
numpy             1.20.3
OpenFisca-Core    35.7.6
OpenFisca-France  116.6.1
packaging         21.3
pip               22.2.2
pluggy            1.0.0
psutil            5.9.2
py                1.11.0
pyparsing         3.0.9
pytest            7.1.3
PyYAML            6.0
setuptools        65.3.0
sortedcontainers  2.2.2
tomli             2.0.1
typing-extensions 3.10.0.2
typish            1.9.3
wheel             0.37.1

[PiGo86]

Si ça peut aider, je comprends que le test échoue lors de l'appel de la méthode rate_from_bracket_indice de la classe MarginalRateTaxScale.

Cette méthode est récente, car elle a été introduite avec la version 35.8.0 d'OpenFisca-Core (openfisca/openfisca-core/pull/1114/).

Or, selon le retour de la commande pip list communiqué par @sandcha, l'environnement utilise la version 35.7.6 d'OpenFisca-Core, ce qui explique l'échec du test.

[benoit-cty]

Merci, je vais tester cette solution.

Le setup.py pour PyPi n'impose pas la 35.8.0, j'ai fait la modification dans la PR #1863

Après discussion avec @MattiSG nous nous sommes dit qu'il serait préférable de: Faire le build conda et le test quand l'étape check version passe.

Cela implique que:

• Le paquet conda ne peut plus être fait à partir du paquet PyPi mais doit être fait à partir des sources.
• On lance les tests OpenFisca France en utilisant le paquet
• On sauve le paquet PyPi dans un artifact ( comparing-artifacts-and-dependency-caching )
• Lors du merge sur master on récupère l'artifact et on le publie.

Un risque que je vois à cette dernière étape: L'artifact portera le numéro de version. Une personne mal intentionnée pourrait créer une branche avec le même numéro de version pour remplacer l'artifact afin qu'il soit publié lors du merge. Peut-être qu'il vaut mieux rebuild sur master.

[benoit-cty]

Les tests passent, la PR https://github.com/openfisca/openfisca-france/pull/1863 est ouverte à la revue.

[MattiSG]

créer une branche avec le même numéro de version pour remplacer l'artifact afin qu'il soit publié lors du merge

Belle identification de vulnérabilité ! On peut peut-être explorer la possibilité d'indexer le cache sur la SHA de HEAD^ afin de couvrir la majorité des cas, où le commit de merge sur master suit immédiatement le dernier passage des tests 🙂

[benoit-cty]

Finalement le build conda ne prend "que" 9 minutes finalement alors autant le refaire sur master pour être sûr de ce qui est embarqué, non ?

Par contre les tests prennent 24 minutes, donc on pourrait les retirer de master.

[MattiSG]

J'ai l'impression que le plus efficient serait de :

1. Utiliser le cache pour les builds, afin de minimiser le coût des tests sur les branches de travail.
2. En cas de succès des tests, d'utiliser les artefacts avec indexation sur le SHA du commit courant pour la publication et sur le SHA de HEAD^ pour la récupération sur master.

[MattiSG]

Et je suis en effet favorable à ne pas exécuter des tests sur master qui ont déjà été effectués par ailleurs 🙂