You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tvm.apache.org by Ramana Radhakrishnan <no...@github.com> on 2020/04/23 14:07:31 UTC
[apache/incubator-tvm] [RFC] Pytest environment improvements (#5421)
In many places having a global pytest flag is useful . For me with the
build and test of tvm , I would like to be able to globally pass in
pytest options as part of development flow or CI flows where one would
like to measure other things regularly that need measurements including
pytest coverage data that I would like to experiment with across the stack.
This has been achieved with an additional setup-pytest-env.sh file in
tests/scripts rather than putting in something in every single task test
script and something I would like to avoid.
This now means the -v option to pytest is superfluous. I did consider
having a pytest.ini file but that doesn't allow me to pass any old
environment variable in and this seems to be the best compromise.
I have also updated the pull request documentation to reflect
the presence of the -k pytest option at the same time though
technically this could be a separate pull request.
@tqchen
You can view, comment on, or merge this pull request online at:
https://github.com/apache/incubator-tvm/pull/5421
-- Commit Summary --
* [RFC] Pass pytest options globally.
* Improve other use case documentation
-- File Changes --
M docker/bash.sh (1)
M docker/build.sh (1)
M docker/with_the_same_user (2)
M docs/contribute/pull_request.rst (3)
A tests/scripts/setup-pytest-env.sh (21)
M tests/scripts/task_python_frontend.sh (18)
M tests/scripts/task_python_integration.sh (18)
M tests/scripts/task_python_nightly.sh (2)
M tests/scripts/task_python_topi.sh (2)
M tests/scripts/task_python_unittest.sh (4)
M tests/scripts/task_python_vta_fsim.sh (4)
M tests/scripts/task_python_vta_tsim.sh (4)
-- Patch Links --
https://github.com/apache/incubator-tvm/pull/5421.patch
https://github.com/apache/incubator-tvm/pull/5421.diff
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/incubator-tvm/pull/5421
Re: [apache/incubator-tvm] [RFC] Pytest environment improvements
(#5421)
Posted by "Tang, Shizhi" <no...@github.com>.
I have a question. Actually I cannot directly run `./tests/scripts/task_python_unittest.sh` without Docker after merging this PR.
```
tests/scripts/setup-pytest-env.sh: line 23: CI_PYTEST_ADD_OPTIONS: unbound variable
```
What is `CI_PYTEST_ADD_OPTIONS`? Should I set it to an empty string?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/incubator-tvm/pull/5421#issuecomment-619507703
Re: [apache/incubator-tvm] [RFC] Pytest environment improvements
(#5421)
Posted by Tianqi Chen <no...@github.com>.
Merged #5421 into master.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/incubator-tvm/pull/5421#event-3267080792
Re: [apache/incubator-tvm] [RFC] Pytest environment improvements
(#5421)
Posted by Tom Gall <no...@github.com>.
> It would be great if we can avoid the hack into the `with_same_user`. One alternative would be still pass in the `PYTEST_ADDOPTS` env variable from the docker env(for development purposes) but source the setup-pytest-env within each of the script.
>
> This also makes the intention of the CI scripts clear, since sometimes we could directly use these scripts. For example, I sometimes would use `./tests/scripts/task_python_unittest.sh` without the docker image to run the testcases
Exactly. For my modest arm dev board environments, where i want/need to run tests, docker just isn't viable.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/incubator-tvm/pull/5421#issuecomment-618499668
Re: [apache/incubator-tvm] [RFC] Pytest environment improvements
(#5421)
Posted by Ramana Radhakrishnan <no...@github.com>.
>
>
> > It would be great if we can avoid the hack into the `with_same_user`. One alternative would be still pass in the `PYTEST_ADDOPTS` env variable from the docker env(for development purposes) but source the setup-pytest-env within each of the script.
> > This also makes the intention of the CI scripts clear, since sometimes we could directly use these scripts. For example, I sometimes would use `./tests/scripts/task_python_unittest.sh` without the docker image to run the testcases
>
> Exactly. For my modest arm dev board environments, where i want/need to run tests, docker just isn't viable.
That is certainly a very interesting orthogonal question as we would need to figure out how best to run things cross as not every single python package tvm depends on is available natively on AArch32 or AArch64.
regards
Ramana
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/incubator-tvm/pull/5421#issuecomment-618504136
Re: [apache/incubator-tvm] [RFC] Pytest environment improvements
(#5421)
Posted by Tianqi Chen <no...@github.com>.
Thanks @u99127 @tom-gall
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/incubator-tvm/pull/5421#issuecomment-618695859
Re: [apache/incubator-tvm] [RFC] Pytest environment improvements
(#5421)
Posted by Ramana Radhakrishnan <no...@github.com>.
I don't like my current hack of overloading "with_same_user" for sourcing this global environment but it seemed like the simplest hack and worked in my environment. Obviously I don't have cuda testing in my CI or my regular test environment, so this isn't fully clear.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/incubator-tvm/pull/5421#issuecomment-618416038
Re: [apache/incubator-tvm] [RFC] Pytest environment improvements
(#5421)
Posted by Tianqi Chen <no...@github.com>.
It would be great if we can avoid the hack into the `with_same_user`. One alternative would be still pass in the `PYTEST_ADDOPTS` env variable from the docker env(for development purposes) but source the setup-pytest-env within each of the script.
This also makes the intention of the CI scripts clear, since sometimes we could directly use these scripts. For example, I sometimes would use `./tests/scripts/task_python_unittest.sh` without the docker image to run the testcases
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/incubator-tvm/pull/5421#issuecomment-618455854
Re: [apache/incubator-tvm] [RFC] Pytest environment improvements
(#5421)
Posted by Ramana Radhakrishnan <no...@github.com>.
>
>
Thanks for the quick review.
> It would be great if we can avoid the hack into the `with_same_user`. One alternative would be still pass in the `PYTEST_ADDOPTS` env variable from the docker env(for development purposes) but source the setup-pytest-env within each of the script.
>
Indeed , that seems a simple enough solution, here's a rework.
> This also makes the intention of the CI scripts clear, since sometimes we could directly use these scripts. For example, I sometimes would use `./tests/scripts/task_python_unittest.sh` without the docker image to run the testcases
Yeah our wrapping on top uses docker most of the time and that's why I went with the "with_same_user" hack for a prototype but I can see that others may not choose to develop in the same way as us.
R
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/incubator-tvm/pull/5421#issuecomment-618493747
Re: [apache/incubator-tvm] [RFC] Pytest environment improvements
(#5421)
Posted by Tianqi Chen <no...@github.com>.
hmm, @u99127 do you mind send a patch? we should detect if CI_PYTEST_ADD_OPTIONS is available and set it to none
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/incubator-tvm/pull/5421#issuecomment-619645809