Introduce an option to run all YAML tests in a directory and get all failures results

[benjello]

Hi there!

I really enjoy OpenFisca, but I recently encountered an issue.

Here is what I did:

openfisca test tests

Here is what I expected to happen:

I want to grab all the failing tests ...

Here is what actually happened:

.. but It failed on the first test as expected

Here is data (or links to it) that can help you reproduce this issue:

I asked on slack and the right way to do this is

PYTEST_ADDOPTS="--maxfail=0" openfisca test tests

but I think that an explicit option maxfail directly discoverable in the help section accessiblethrough openfisca test -hwould be very useful.

Context

I identify more as a:

• Analyst (I make macroscopic computations on real populations).
• Business expert (I create tests and model legislation).
• Commenter (I make data visualisations).
• Developer (I create tools that use the existing OpenFisca code).

[MattiSG]

I personally used to edit the setup.cfg file to remove the --exitfirst option. I agree that it is important at least to document this, and potentially to add a way to more easily use that option.

I wonder if it would not be smarter to couple the two options in one single behaviour: either no test path is given and the “exit on first error” behaviour (current default) is used (expected use case: check that no breaking changes were introduced), or a test path is given and all errors are collected (expected use case: test-driven-development of an area of legislation).

I believe this would cover the vast majority of use cases, and we could avoid introducing a new option 🙂

[bonjourmauko]

Hi! The --existfirst option was removed from OpenFisca Core's setup.cfg a while ago.

Maybe it is your country's or your city's package that is actually using it? As it is for example the case of OpenFisca France.

As openfisca test is basically a test collection plugin for pytest, whatever you have on your package's setup.cfg is what is going to be taken into account, unless you override your own options as you seem to already be doing:

PYTEST_ADDOPTS="$PYTEST_ADDOPTS --maxfail=0" openfisca test tests

2 errors in 0.88s

or

PYTEST_ADDOPTS="$PYTEST_ADDOPTS --maxfail=1" openfisca test tests

2 errors in 0.88s

or its equivalent

PYTEST_ADDOPTS="$PYTEST_ADDOPTS --exitfirst" openfisca test tests

stopping after 1 failures

An explicit option maxfail directly discoverable in the help section accessiblethrough openfisca test -h would be very useful.

I agree.

I think it is a fair trade-off between (+) flexibility and clarity, and (-) over interfacing with pytest —instead of just passing the options along, as I've proposed here.

Either no test path is given

Currently openfisca test requires a path as a mandatory argument —and in my humble opinion that's good because of least astonishment.

Otherwise, we would have to force all dependent packages to use the same default path for tests —for example tests.

That would impact package's structure choices —for example the Country Package that uses openfisca_country_template/tests instead of tests, and so on.

The “exit on first error” behaviour (current default) is used

If I'm not mistaken, pytest default's behaviour is to not exit on first error (maxfail=0).

And normally openfisca test will respect that.

You can then override that default in your own package's setup.cfg.

I wonder if it would not be smarter to couple the two options in one single behaviour: either no test path is given and the “exit on first error” behaviour (current default) is used (expected use case: check that no breaking changes were introduced), or a test path is given and all errors are collected (expected use case: test-driven-development of an area of legislation).

This package already does something similar, implicitly:

1. Default behaviour is to not fail on first error —assuming TDD
2. But it forces failure on first error on CI —assuming that fail-fast is optimal for an integrity check

I agree that it is important at least to document this, and potentially to add a way to more easily use that option.

I also agree to both points.

Proposal

So I see two possible scenarios:

1. Document that package's own setup.cfg options for pytest are taken into account by openfisca test; or
2. Force maxfail=0 as a --tdd option by default, and add a --ci option to fail at first error —overriding setup.cfg

What do you think?

[benjello]

The second solution looks more discoverable to our specific user/dev base.

[benjello]

@maukoquiroga @MattiSG @sandcha how should we implement that ?

[benjello]

@MattiSG : would you agree with the second solution. It is more flexible and more useful for doing some try and error when writing some peace of legislation.

[MattiSG]

This issue thus depends on the setup.cfg value of the country packages, thanks @maukoquiroga for your detailed clarifications!

The fix for a country package such as France is thus rather trivial: just remove --exitfirst in the setup.cfg. However, this change would have the side effect of letting all tests run for very long in CI, queuing more jobs and wasting energy 🌱 I tried to edit the CI instructions in France, but they use openfisca test, which doesn't accept --exitfirst as an option. How could we forward this option with openfisca test?