Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clean argument for runner #3351

Merged
merged 1 commit into from Jul 2, 2023
Merged

clean argument for runner #3351

merged 1 commit into from Jul 2, 2023

Conversation

vborchsh
Copy link
Contributor

The argument might be used for 100% pure rebuild the testbench and delete any previous results such as build artifacts, simulator libraries and so on.

Actually, simple copy-paste of cocotb-test with minor editing, because cocotb runner knows exact path to the building directory thanks to build_dir argument.

Not sure about tests, it could be something like:

  1. create build_dir before runner.build() call
  2. put some file into folder
  3. run runner.build()
  4. assert file in the folder. Must be false for clean build, true for non-clean build.

Looks a bit poor, but still... Feedback is welcome :)

@themperek
Copy link
Contributor

I am OK with this. Maybe should also take the COCOTB_CLEAN environmental variable if set?

Could we just "clean" when always is set?

Maybe add runner.clean() instead?

The question would be should we have a command like cocotb-clean command? Since build_dir can be set it will only work on the default.

@vborchsh
Copy link
Contributor Author

I am OK with this. Maybe should also take the COCOTB_CLEAN environmental variable if set?

If I get it right, you're going to leave Makefiles flow for back compatibility only. So, this case I'd leave it as is.

Could we just "clean" when always is set?
Maybe add runner.clean() instead?

This is the idea I thought about. But I think, clean + always is more flexible. You can keep any libs and artifacts in the folder in case always. And purge everything in case clean.

The question would be should we have a command like cocotb-clean command? Since build_dir can be set it will only work on the default.

I used to use the command with cocotb-test before. And we can do the same, plus couple keys: --recursive, --buildir. In the other PR, I guess.

@vborchsh vborchsh marked this pull request as ready for review June 26, 2023 04:30
@ktbarrett
Copy link
Member

I think this is fine as is. Just needs a newsfragement. See instructions here.

@vborchsh
Copy link
Contributor Author

Are there any ideas about failed MacOS icarus test? There is a timeout in the tests but I didn't touch them... And test_runner is passed:

tests/pytest/test_runner.py::test_runner[False-parameters0] PASSED       [ 66%]
tests/pytest/test_runner.py::test_runner[False-parameters1] PASSED       [ 75%]
tests/pytest/test_runner.py::test_runner[True-parameters0] PASSED        [ 83%]
tests/pytest/test_runner.py::test_runner[True-parameters1] PASSED        [ 91%]

@vborchsh
Copy link
Contributor Author

Everything is merged into single commit. Thanks @imphil for advice.

@ktbarrett ktbarrett merged commit 08ef2f6 into cocotb:master Jul 2, 2023
22 checks passed
@vborchsh vborchsh deleted the runner_clean branch July 3, 2023 03:25
@imphil imphil added the backport? Backport requested/proposed label Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport? Backport requested/proposed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants