openfisca-core icon indicating copy to clipboard operation
openfisca-core copied to clipboard

Introduce a way to run test in parallel

Open benoit-cty opened this issue 4 years ago • 12 comments

Hi there!

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

Here is what I did:

openfisca test --country-package openfisca_france tests

Here is what I expected to happen:

Use of all the core of my CPU to run test so that it took less than 3 minutes.

Here is what actually happened:

Only one core is used, so test took 17 minutes.

Context

I identify more as a:

  • Developer (I create tools that use the existing OpenFisca code at https://LexImpact.an.fr).

Experimentation I have made

Use GNU parallel

find tests/**/*.{yaml,yml} | parallel -j 32 --progress openfisca test --country-package openfisca_france | grep FAILED Pros:

  • it is really fast, less than 3 minutes. Cons:
  • All the test output is printed to the console, so you need to process them to know error, with grep FAILED for example.
  • Need an install at OS level sudo apt-get install parallel.

Use of Xdist

pytest-xdist is a Pytest plugins that run test in parallel. To use it: pip install pytest-xdist[psutil] Add in setup.cfg:

[tool:pytest]
addopts      = -n auto` 

Run the test openfisca test --country-package openfisca_france tests

openfisca test  --country-package openfisca_france tests
=================================================================================================== test session starts ====================================================================================================
platform linux -- Python 3.8.6, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /LEXIMPACT/openfisca-france, configfile: setup.cfg
plugins: forked-1.3.0, xdist-2.3.0
gw0 [42] / gw1 [42] / gw2 [42] / gw3 [42] / gw4 [42] / gw5 [42] / gw6 [42] / gw7 [42] / gw8 [42] / gw9 [42] / gw10 [42] / gw11 [42] / gw12 [42] / gw13 [42] / gw14 [42] / gw15 [42]
..........................................
============================================================================================ 42 passed, 1522 warnings in 16.44s ============================================================================================

Pros:

  • Use all CPU core
  • Nice pytest output Cons:
  • Need to upgrade pytest version dependencies
  • Don't work as expected : it run the test of the project like a normal pytest, not the OpenFisca YAML testfiles.

It seems that the OpenFisca pytest plugin https://github.com/openfisca/openfisca-core/blob/master/openfisca_core/tools/test_runner.py need to override some xdist method to give it the right test.

Use pytest-parallel

https://github.com/browsertron/pytest-parallel Not tested, it seems to work like Xdist, so may have the same problem.

benoit-cty avatar Jun 25 '21 19:06 benoit-cty

Hi @benoit-cty and thanks for this issue.

I see there is an option overlap regarding -n https://github.com/openfisca/openfisca-core/blob/master/openfisca_core/scripts/openfisca_command.py#L34-L43.

So I've tried this:

PYTEST_ADDOPTS="--numprocesses=auto" openfisca test --country-package openfisca_france tests

And got:

platform darwin -- Python 3.7.0, pytest-6.2.4, py-1.10.0, pluggy-0.13.1 rootdir: /Users/hyperion/Sites/openfisca/openfisca-france, configfile: setup.cfg plugins: xdist-2.3.0, forked-1.3.0 gw0 [42] / gw1 [42] / gw2 [42] / gw3 [42] .......................................... ==================================================================================== 42 passed, 1510 warnings in 34.30s ====================================================================================

~~We should further investigate why all those warnings.~~

The same result you had, so I confirm that it actually overrides OpenFisca's test collector.

bonjourmauko avatar Jun 27 '21 09:06 bonjourmauko

Tried directly adding the option to openfisca test.

# openfisca_command.py

    def build_test_parser(parser):
        parser.add_argument('path', help = "paths (files or directories) of tests to execute", nargs = '+')
        parser = add_tax_benefit_system_arguments(parser)
        parser.add_argument('-n', '--name_filter', default = None, help = "partial name of tests to execute. Only tests with the given name_filter in their name, file name, or keywords will be run.")
        parser.add_argument('-p', '--pdb', action = 'store_true', default = False, help = "drop into debugger on failures or errors")
        parser.add_argument('--performance-graph', '--performance', action = 'store_true', default = False, help = "output a performance graph in a 'performance_graph.html' file")
        parser.add_argument('--performance-tables', action = 'store_true', default = False, help = "output performance CSV tables")
        parser.add_argument('-v', '--verbose', action = 'store_true', default = False, help = "increase output verbosity")
        parser.add_argument('-o', '--only-variables', nargs = '*', default = None, help = "variables to test. If specified, only test the given variables.")
        parser.add_argument('-i', '--ignore-variables', nargs = '*', default = None, help = "variables to ignore. If specified, do not test the given variables.")
        parser.add_argument('-d', '--distributed', nargs = 1, default = None, help = "run tests distributed. Use `auto` to detect the number of cores.")
# run_test.py

    options = {
        'pdb': args.pdb,
        'performance_graph': args.performance_graph,
        'performance_tables': args.performance_tables,
        'verbose': args.verbose,
        'name_filter': args.name_filter,
        'only_variables': args.only_variables,
        'ignore_variables': args.ignore_variables,
        'distributed': args.distributed,
        }
# test_runner.py

    argv = ["--capture", "no"]

    if options.get('pdb'):
        argv.append('--pdb')

    if options.get("distributed"):
        argv.append(f"--numprocesses={options.get('distributed')[0]}")

Results where not better:

COUNTRY_TEMPLATE_PATH=`python -c "import openfisca_country_template; print(openfisca_country_template.CountryTaxBenefitSystem().get_package_metadata()['location'])"`
openfisca test -d auto $COUNTRY_TEMPLATE_PATH/openfisca_country_template/tests/

=========================================================================================== test session starts ============================================================================================
platform darwin -- Python 3.7.0, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /Users/hyperion
plugins: xdist-2.3.0, cov-2.11.1, forked-1.3.0
gw0 [0] / gw1 [0] / gw2 [0] / gw3 [0]

========================================================================================== no tests ran in 0.50s ===========================================================================================

bonjourmauko avatar Jun 27 '21 09:06 bonjourmauko

Thanks for investigating, I think I will open an issue in Xdist to get some help.

benoit-cty avatar Jun 27 '21 10:06 benoit-cty

Finally from https://github.com/pytest-dev/pytest-xdist/blob/1dc019709c15678faa622834d3bc851abddb426c/OVERVIEW.md#L70

xdist works by spawning one or more workers, which are controlled by the master. Each worker is responsible for performing a full test collection and afterwards running tests as dictated by the master.

The master receives the result of the collection from all nodes. At this point the master performs some sanity check to ensure that all workers collected the same tests (including order), bailing out otherwise. If all is well, it converts the list of test-ids into a list of simple indexes, where each index corresponds to the position of that test in the original collection list. This works because all nodes have the same collection list, and saves bandwidth because the master can now tell one of the workers to just execute test index 3 index of passing the full test id.

Why does each worker do its own collection, as opposed to having the master collect once and distribute from that collection to the workers?

If collection was performed by master then it would have to serialize collected items to send them through the wire, as workers live in another process. The problem is that test items are not easily (impossible?) to serialize, as they contain references to the test functions, fixture managers, config objects, etc. Even if one manages to serialize it, it seems it would be very hard to get it right and easy to break by any small change in pytest.

bonjourmauko avatar Jun 27 '21 10:06 bonjourmauko

So it seems pytest-xdist creates a master worker, that interrupts normal test collection —so pytest_collect_item is not called, instantiates a node for each processor, then lets each node do the collection work.

It seems to me that the solution is to hook up OpenFiscaPlugin to each node, however I wasn't able to do so.

Hope it helps!

bonjourmauko avatar Jun 27 '21 13:06 bonjourmauko

Thanks for all the details, I've openned an XDist Issue: https://github.com/pytest-dev/pytest-xdist/issues/677

benoit-cty avatar Jun 28 '21 09:06 benoit-cty

https://github.com/pytest-dev/pytest-xdist/issues/681#issuecomment-878250080

unfortunately manually passed pytest plugins do not pass over to xdist

if your plugin is a straightforward single file thing anyway i strongly recomment to use the -p option to pytest to pass it over as then the workers in xdist will also try to use them

bonjourmauko avatar Jul 31 '21 09:07 bonjourmauko

If I understand that correctly, xdist is a dead-end.

Why doesn't nose suffice, as was the intention in #516?

MattiSG avatar Aug 19 '21 14:08 MattiSG

Hello @MattiSG,

If I understand that correctly, xdist is a dead-end.

As I currently understand it yes, because of the way we use pystest.

Why doesn't nose suffice, as was the intention in #516?

At the time of that discussion, nose was being shelved for deprecation. However, work on nose2 seems quite active today.

bonjourmauko avatar Aug 19 '21 16:08 bonjourmauko

By the way, I've got parallel tests probably working here, to be tested with France for example.

bonjourmauko avatar Aug 19 '21 17:08 bonjourmauko

@HAEKADI since you have created a GitHub Actions syntax to try out parallel tests in #1027, could you please investigate if @benoit-cty’s suggestion to use https://github.com/nektos/act would also work for parallelising locally? 😃

MattiSG avatar Aug 24 '21 10:08 MattiSG

I investigated the use of act in OpenFisca-France here :)

HAEKADI avatar Sep 23 '21 14:09 HAEKADI