Add pytest - step 2 #554
Add pytest - step 2 #554
Conversation
|
|
…ocks into lekcyjna/add-pytest-2
|
|
…ocks into lekcyjna/add-pytest-2
lekcyjna123
requested review from
tilk,
piotro888 and
xThaid
and removed request for
xThaid
| @@ -0,0 +1,127 @@ | |||
| from .memory import * | |||
There was a problem hiding this comment.
To be found during discovery procedure. Every file with tests should have either test_ or _test by default. test.py is not correct name in default configuration.
https://doc.pytest.org/en/latest/explanation/goodpractices.html#conventions-for-python-test-discovery
There was a problem hiding this comment.
I think that we should separate 'regression' as [external tests that may be run with verilator], apart from [the riscv-tests suite].
In this file MMIO and rv32uc-rvc are both very riscv-test specific, but it also contains regression_body_with_cocotb, verilate_model that seem like they are generic for all verilator regeression test concept.
It was also a problem with previous solution, so maybe it is a good time to separate and generalize it now? Or am I missing something and those would not be useful in more generic context?
Also I think that in test/regression/conftest.py, test collecting functions should be named more to reflect that they are configuring riscv-tests suite and not regression tests in general.
There was a problem hiding this comment.
It is a good point, but I would prefer to do that in separate PR (the one in which multithreading will be added to riscof tests), because regression_body_with_cocotb have some hardcoded values specific for riscv-tests.
There was a problem hiding this comment.
Seems fine to me, but please don't forget to also clarify the usage of 'regression' term in all places in that PR :)
Co-authored-by: piotro888 <[email protected]>
Next step of adding pytest.