Separate build and test for python runner

[themperek]

This allows to run the parallel example with pytest-xdist:

pytest -m compile
pytest -m "not compile" -n auto

Not sure if we should put all those arguments in test() like with build_dir , library_name or parameters ? @ktbarrett

Need to add some optional arguments to test()

• build_dir
• library_name -> needed for Icarus executable file name could use some generic name and not touch this.
• parameters -> needed for GHDL/Questa/Riviera

[themperek]

Maybe should rise exception if not provided or default?

[themperek]

Maybe can add something like "self.did_build" and based on this require some extra settings or return warnings?

[ktbarrett]

Not sure if we should put all those arguments in test() like with build_dir , library_name or parameters ?

This is exactly why I didn't like the current approach where all the options were heaped into the Simulator object. build should instead produce a Library/Build object with the pertinent fields saved. This is also necessary for describing pre-built libraries like IP cores. test would take a Library/Build object instead of having library_name and build_dir passed in. Perhaps even build would take Library/Build dependencies for doing arbitrarily-nested separate compilation.

[themperek]

Not sure if we should put all those arguments in test() like with build_dir , library_name or parameters ?

This is exactly why I didn't like the current approach where all the options were heaped into the Simulator object. build should instead produce a Library/Build object with the pertinent fields saved. This is also necessary for describing pre-built libraries like IP cores. test would take a Library/Build object instead of having library_name and build_dir passed in. Perhaps even build would take Library/Build dependencies for doing arbitrarily-nested separate compilation.

I guess everything will have its pros and cons. I removed library_name from test()

[codecov]

Codecov Report

Merging #2853 (97d8f7f) into master (de5c25d) will increase coverage by 0.16%.
The diff coverage is 60.00%.

❗ Current head 97d8f7f differs from pull request most recent head c649535. Consider uploading reports for the commit c649535 to get more accurate results

@@            Coverage Diff             @@
##           master    #2853      +/-   ##
==========================================
+ Coverage   71.96%   72.13%   +0.16%     
==========================================
  Files          47       47              
  Lines        8437     8440       +3     
  Branches     1455     1453       -2     
==========================================
+ Hits         6072     6088      +16     
+ Misses       1953     1940      -13     
  Partials      412      412              

Impacted Files	Coverage Δ
cocotb/runner.py	56.90% <60.00%> (+3.42%)	⬆️
cocotb/share/lib/vpi/VpiImpl.cpp	78.33% <0.00%> (+0.27%)	⬆️
cocotb/share/lib/gpi/GpiCommon.cpp	76.55% <0.00%> (+0.34%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de5c25d...c649535. Read the comment docs.

[ktbarrett]

If we moved to build returning a build object, we could use pytest fixtures and not have a two step process.

@pytest.fixture(scope="session")
def simulator():
    return get_runner(sim)()


@pytest.fixture(scope="session")
def build(simulator):
    return runner.build(
        always=True,
        verilog_sources=verilog_sources,
        vhdl_sources=vhdl_sources,
        toplevel=toplevel,
        build_dir=sim_build,
        extra_args=compile_args,
    )


@pytest.mark.parametrize("seed", list(range(4)))
def test_cocotb_parallel(seed, simulator, build):
    simulator.test(
        build=build,   # pass build object here
        toplevel=toplevel,
        python_search=python_search,
        py_module=module_name,
        seed=seed,
    )

[themperek]

If we moved to build returning a build object, we could use pytest fixtures and not have a two step process.

@pytest.fixture(scope="session")
def simulator():
    return get_runner(sim)()


@pytest.fixture(scope="session")
def build(simulator):
    return runner.build(
        always=True,
        verilog_sources=verilog_sources,
        vhdl_sources=vhdl_sources,
        toplevel=toplevel,
        build_dir=sim_build,
        extra_args=compile_args,
    )


@pytest.mark.parametrize("seed", list(range(4)))
def test_cocotb_parallel(seed, simulator, build):
    simulator.test(
        build=build,   # pass build object here
        toplevel=toplevel,
        python_search=python_search,
        py_module=module_name,
        seed=seed,
    )

If you use pytest-xdist it will compile in every thread (if you span 4 runners fixture will run 4 times). This is also a use case but ...

[ktbarrett]

It looks like many people are interested in such a feature, but they have put no effort into implementing it, even though they have a potential viable design. pytest-dev/pytest-xdist#271.

The current approach only works if you are running xdist tests locally. Builds should be done per node, but per worker may be fine, it consumes N disk space vs 1, but should take the same amount of wall time for the test to complete since it's all happening in parallel.

[themperek]

It looks like many people are interested in such a feature, but they have put no effort into implementing it, even though they have a potential viable design. pytest-dev/pytest-xdist#271.

The current approach only works if you are running xdist tests locally. Builds should be done per node, but per worker may be fine, it consumes N disk space vs 1, but should take the same amount of wall time for the test to complete since it's all happening in parallel.

I have lost hope we will have the perfect solution. Just some solution.

As it is in this PR would give full flexibility if needed.
One can use hooks from pytest-xdist and do all sorts of "magic" but probably only for advanced users.

[ktbarrett]

We need to make sure we track the fact we need documentation for this once it's all complete and we are happy with it.

[ktbarrett]

I probably should add a mypy check for the files that are checkable. We need to first decide if that is a goal, or if that should all be handled with cocotb-stubs.