Skip to content
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.

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

Revalorisation du complément familial (CF) au 1er avril 2018 #1171

Merged
merged 4 commits into from
Oct 19, 2018

Conversation

mtifarine
Copy link
Contributor

@mtifarine mtifarine commented Oct 17, 2018

  • Changement mineur.
  • Périodes concernées : à partir du 01/04/2018.
  • Zones impactées : tests/formulas/cf_2018_04.
  • Détails :
    • Ajoute des tests pour la revalorisation du complément familial (CF) au 1er avril 2018.

@mtifarine mtifarine requested a review from frtomas October 17, 2018 09:08
@Morendil
Copy link
Contributor

Merci @mtifarine ! Je me suis permis d'amender le descriptif de la PR pour faire le lien avec l'issue en rapport, cela permettra de fermer automatiquement l'issue lors du merge, te t'invite à le faire pour les prochaines PR sur le périmètre MSA.

@Morendil
Copy link
Contributor

Morendil commented Oct 17, 2018

Pour le setup.py et CHANGELOG je te recommande d'attendre le dernier moment pour les mettre à jour, les tests automatisés sont désormais ventilés en plusieurs catégories: non-régression sur Python 2 et 3, vérifications statique du code (linting), et séparément vérification du CHANGELOG.

Il n'est donc plus nécessaire de les mettre à jour pour s'assurer que la PR n'introduit pas de régression, et comme une étape de review et un rebase sont nécessaires de toutes façons avant de merger, cela peut être la responsabilité de la personne qui réalise la review finale de la PR. Evidemment la responsabilité de la rédaction d'un descriptif complet reste celle de la personne qui crée la PR.

@@ -0,0 +1,575 @@
- name: "Complement familial - Cas N°1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je préfèrerais des éléments plus informatifs pour ces noms et descriptifs, par exemple sommes-nous ici en dessous du plafond ? Dans ce cas le nom pourrait être "Complément familial accordé car en-dessous du plafond." Un autre cas de test sera "CF non accordé car ressoures au-dessus du plafond", etc.

@mtifarine mtifarine added the contrib:msa Identification des sujets MSA label Oct 17, 2018
cf: 256.09 / (1 - 0.005)
# cf net - crds

- name: "Complement familial - Cas Non Passant N°1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je renouvelle mes réserves sur la nomenclature "non passant", dans notre contexte on va plus l'interpréter comme un test en échec ce qui n'est pas le cas; là aussi il est souhaitable de détailler plutôt le cas de figure (CF non attribué) et les raisons.

@Morendil
Copy link
Contributor

Morendil commented Oct 17, 2018

Désolé c'est moi qui suis à l'origine de la typo:
ressoures -> ressources

@Morendil
Copy link
Contributor

Je ne comprends pas pourquoi nous avons besoin de 15 cas de test différents, qu'est-ce qui différencie les différents cas, comment arrive-t-on à cette décomposition ?

@mtifarine
Copy link
Contributor Author

La différence entre les cas tests est liée aux ressources de la famille, au nombre de personnes à charge et à l'âge des personnes à charge (enfant ou jeune).

@mtifarine
Copy link
Contributor Author

Ce sont les mêmes cas de test du 01/01/2018

@Morendil
Copy link
Contributor

Ce sont les mêmes cas de test du 01/01/2018

C'est bien le souci, ces tests sont en grande partie redondants. Aucune évolution structurelle n'est intervenue entre janvier et avril 2018, seul le montant est revalorisé. Les tests qui donnent le même montant font partie d'une même "classe d'équivalence" (pour invoquer le jargon technique) vis-à-vis de ce qu'on cherche à tester, à savoir la revalorisation. On a 3 classes d'équivalence donc on peut couvrir avec 3 tests, voire 2 si on considère que l'annulation du CF pour dépassement du montant est déjà testé ailleurs.

Pour le dire autrement, il n'y a pas de modification envisageable du code d'OpenFisca France qui serait susceptible de faire échouer un seul test de la série cumulée cf_2018 et 2018_04_cf. (Au passage, ce nommage des fichiers manque de cohérence: cf_2018_01 et cf_2018_04 seraient plus cohérents. C'est un détail mais la cohérence facilite grandement la navigation dans une base de code complexe.)

En poussant plus loin l'analyse (on est ici dans du test "boite blanche" où on peut s'appuyer sur le code pour guider le test) on voit que le nombre d'enfants n'intervient que pour l'éligibilité et le plafond de ressources, mais n'entre pas dans le calcul du montant, on peut donc tester ces notions de façon orthogonale.

On peut même argumenter que le bon nombre de tests pour une PR qui ne modifierait qu'un paramètre législatif à l'exclusion de toute autre modification du code est de 0, car dans ce cas on ne teste que la fonctionnalité "OpenFisca sait remonter la bonne valeur d'un paramètre pour une période" et celle-ci est déjà abondamment testée. (Ce n'est pas le cas ici, puisque les montants de base et majorés du CF n'apparaissent pas directement dans les paramètres législatifs, et aussi parce qu'on réalise ce test bien après l'intégration de l'évolution des paramètres à l'occasion d'autres PR. On peut considérer que 2 tests supplémentaires par prudence sont justifiés.)

Nous devons être attentifs à la prolifération de tests qui ne se justifieraient pas par leur valeur ajoutée indivudelle, car le temps d'exécution de chaque test individuel, s'il est faible individuellement, finit par se cumuler en une intégration continue qui prend des dizaines de minutes, et complique encore plus le travail des contributeurs qui souhaitent pratiquer le développement piloté par les tests.

Peut-être notre stratégie de test pour OpenFisca France mériterait-elle d'être mieux explicitée voire documentée, en attendant il serait peut-être utile de nous solliciter pour discuter rapidement de cette question des tests pertinents à chaque évolution, en avance de phase de la réalisation de la PR. Cela nous ferait peut-être plus gagner de temps qu'on n'en consacrerait à la discussion. En tout cas je suis disponible pour cela!

@mtifarine
Copy link
Contributor Author

De notre part et pour la réalisation d'un PR, on suis une fiche de demande fournie par l'experts qui contient aussi la liste des tests à vérifie.
Donc la je sais pas si a nous de faire la sélection ou c'est plutôt l'expert.

@Morendil
Copy link
Contributor

@mtifarine Je suis disponible pour parler à l'expert alors :) de qui s'agit-il ?

@mtifarine
Copy link
Contributor Author

Voir avec @JenniferTelep ou @ThibaultCCMSA.

@JenniferTelep
Copy link
Collaborator

Bonjour @Morendil
En effet les cas de test pour la revalorisation de janvier 2018 du CF testent les mêmes conditions d'éligibilité que les tests pour la revalorisation d'avril 2018.
Ce qu'on propose pour cette PR c'est qu'exceptionnellement vous choisissiez dans les cas proposés les 2/3 à tester.
Et à l'avenir pour les revalorisations "sèches" sans réforme, on ne demandera que 2/3 cas passants à l'expert plus pertinents et différents des cas des précédentes PR revalorisation.

Pour info: @ThibaultCCMSA , @jmdallais

@mtifarine
Copy link
Contributor Author

mtifarine commented Oct 19, 2018

@Morendil peux-tu m'appeler quand tu sera disponible pour qu'on puisse discuter rapidement de cette question des tests et de sélectionner les cas des tests à garder.
Mon numéro 05 61 16 25 36.

@Morendil
Copy link
Contributor

OK fin de matinée sans doute

@mtifarine mtifarine force-pushed the msa_revalorisation_cf branch from bd72c6c to 06b40b1 Compare October 19, 2018 11:59
@mtifarine mtifarine requested a review from Morendil October 19, 2018 13:33
@Morendil Morendil force-pushed the msa_revalorisation_cf branch from a409f4c to c2ec1b1 Compare October 19, 2018 18:51
@Morendil Morendil merged commit 8290f34 into master Oct 19, 2018
@Morendil Morendil deleted the msa_revalorisation_cf branch October 23, 2018 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contrib:msa Identification des sujets MSA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants