[test][python] allow to overwrite the interpreter for python test command

When using dune_python_add_test the SCRIPT is executed by default with the detected python interpreter. The optional INTERPRETER parameter allows to overwrite this setting, e.g. to pass additional command line arguments or to execute the tests using pytest.

@dominic is this sufficient for your use case of pytest?

Pytest is either invoked as python -m pytest (preferred) or as py.test. With the current situation, I would need to pass -m pytest to SCRIPT to get that, which sounds weird. I do also need this custom interpreter parameter though.

I though you call something like

  SCRIPT foo.py
  INTERPRETER pytest

but I never used pytest, so apparently have misunderstood something here. Looking at your text above, I would have though, that you now call

  SCRIPT foo.py
  INTERPRETER ${Python3_EXECUTABLE} -m pytest

Perhaps we have to collect a list of the different use cases, but the old state was broken with harder to detect errors than the explicit message we have now.

Typically, there is no foo.py. Pytest automatically collects tests from the working directory. Also, Pytest is not an interpreter, so using it in this way would most likely be confusing to some people.

I wanted to make the suggestion to add a MODULE argument to the python test. I think that is preferable since this make this a python test and not a catch all. So one would write dune_python_add_test(MODULE pytest foo.py) which would translate to Python3_Interpreter -m pytest foo.py) Could also be dune_python_add_test(MODULE pytest SCRIPT foo.py)

My main point is that I would like to get ${Python3_EXECUTABLE} out of downstream modules.

@andreas.dedner 's proposal is okay for me, as long as the INTERPRETER argument of this MR is also present, because I actually use DUNE_PYTHON_VIRTUALENV_INTERPRETER and not Python3_EXECUTABLE.

The different interpreters is an additional issue I wasn't aware of - that complicates things. So we would need a combination of Christian's and my suggestion, i.e., have 'MODULE' argument and an INTERPRETER argument?

Exactly.

I don't use the dune virtual env. Wouldn't it be more stable to switch Python3_EXECUTABLE to the interpreter in the dune environment, i.e., have DUNE_PYTHON_VIRTUALENV_INTERPRETER=Python3_EXECUTABLE. Or perhaps have a DUNE_PYTHON_EXECUTABLE cmake variable?

If @dominic really wants to use a different interpreter throughout the whole project I see three options:

1. replace the Python3_EXECUTABLE in the main CMakeLists.txt
2. set an INTERPRETER for all dune_python_add_test calls
3. add an additional cmake flag to change the behaviour and prefer DUNE_PYTHON_VIRTUALENV_INTERPRETER over Python3_EXECUTABLE

Perhaps (3) should be the default, if someone is working with a virtual env?!

The whole point of having a virtual environment is that DUNE_PYTHON_VIRTUALENV_INTERPRETER != Python3_EXECUTABLE - they are different. Also bear in mind, that I am not talking about users whose Python environment is actually a virtual env, DUNE_PYTHON_VIRTUALENV_INTERPRETER refers to the virtual env that is automatically set up by the Dune build system (upon request). Why not just provide the INTERPRETER argument for dune_python_add_test - it defaults to Python3_EXECUTABLE and your use case is untouched.

I'm fine with that. I just thought it would be better not to have to use cmake variables explicitly when calling the dune macros. Also this makes dune_python_add_test a do anything test command, because you could pass any bash command to INTERPRETER - but that is not really an issue I assume.

You have a similar "security issue" in all test suites. If you write subprocess.call("rm -rf *", shell=True) into a Python test you have the same result. Running tests will always involve a certain level of trust. The important part is that root access is protected - which is not an issue here.

OK. So @andreas.dedner would you add the module option?

Regarding security, I agree with @dominic... it is not about not being able to do such things, but we should not encourage people to use the command for arbitrary operations.

I can finish this if you want (provide INTERPRETER and MODULE parameters).

Ah, our comments have apparently overlapped. Great if you add the MODULE parameter.

added 2 commits

• ec0a7dd8 - Make INTERPRETER argument a single-valued parameter
• a53a6d41 - Introduce module parameter

Compare with previous version

Done and tested with one downstream module.

Thanks!

merged

mentioned in commit fae77c75

fine with me