Run country & extension template tests systematically

[bonjourmauko]

Relates to #1021 openfisca/openfisca-france#1649

Technical changes

• Run openfisca-core & country/extension template tests systematically
  • Changes to openfisca-core can unintendedly break packages depending on it
  • The country/extension templates are useful to avoid most of these situations
  • These changes provide a way to run them systematically, both locally and in CI
  • Bonus: it only uses openfisca test 😉

[MattiSG]

Thank you @maukoquiroga! 👏

This works for me with pip install openfisca-core[dev] && make test, I confirm that all of Core, Country Template and Extension Template tests are run.

This adds a risk: if Country or Extension template tests are broken, Core tests will break as well even though this would not be related to Core changes. I believe it is an acceptable risk considering that these repository have CI and should never fail anyway, against the added value of checking that Core changes don't break the templates.

[MattiSG]

Thank you for having taken my suggestions into account 🙂

However, the changeset that was originally reviewed has considerably increased, and this PR seems to have suffered from some feature creep… What was initially a way to “run country & extension template tests systematically” now seems to offer an integral rewrite of the makefile and its logs.

This is exciting, but unfortunately means many more checks have to be done, notably on the cross-platform aspect since some shell-specific syntax is used. It would have to be checked against Windows console, CI, zsh-enabled macOS… Could we please refocus this PR on its original scope? I understand this might be frustrating 😟 but I would really love to see the original proposal land in Core, while minimising risks and enabling a proper conversation around build tools evolution 🙂

[bonjourmauko]

Ok with the feature creep 😄 , I'll split things up.

[MattiSG]

Thanks @maukoquiroga!

[MattiSG]

Nice! I'm still a bit appalled at the complexity of the Makefile syntax, I must say, but thank you so much for mastering it and improving it!

[benjello]

Hello @MattiSG @maukoquiroga : this PR stands in the way of some other PRs.
Shouldn't we merge it ASAP or do you plan an overhaul of what is fixed here ?

[bonjourmauko]

I think it is safe to merge this one.

[benjello]

@MattiSG does it LGTM for you ?

[MattiSG]

👍

[MattiSG]

I am surprised to discover that my approving review was dismissed two weeks ago. Review time is precious, and I feel frustrated that I have to review one more time and that this PR was involuntarily blocked for so long.

@maukoquiroga according to the GitHub logs, my review was dismissed through a commit of yours. Did you have any specific action towards that goal? If so, could you share what was your intention? 🙂 If not, do you have an idea why your commit dismissed my review?

[MattiSG]

Reviewed & approved again 😉

[bonjourmauko]

@MattiSG I think we just changed settings to dismiss stale reviews after #990. I've just deactivated it for now.

[MattiSG]

As seen over the phone with @maukoquiroga, the experiment of auto-dismissing stale reviews started after #990 turns out to be too much of a hassle in our process where we almost systematically rebase before merging and where reviews are very asynchronous.