conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.13k stars 969 forks source link

[question] How to run python unit tests #16780

Closed pasbi closed 1 week ago

pasbi commented 1 month ago

My C++-library has a Python interface. Consequently, its unit tests are both C++ (googletest) and Python (pytest).

When doing conan create mylibrary ..., it builds the library and tests and then attempts to run the tests. That works well for the C++ tests, but the Python tests fail. Most likely because they require import mylib, which is not available because mylib has only been built but not yet installed.

 class MyLibRecipe(ConanFile):
     [...]
     def build(self):
         cmake = CMake(self)
         cmake.configure()
         cmake.build()
         cmake.test()

     def package(self):
         cmake = CMake(self)
         cmake.install()
     [...]

I'm a bit lost, please head me into the right direction.

  1. Are my build() and package() plausible?
  2. In the CI (before introducing conan), there were separate jobs for building and unit testing. I would like to keep it separated (though it would be possible to merge them into one job, if required). Is there a way to separate building and testing?
  3. Currently only installing mylib creates an importable python package, hence this step is required before cmake.test(). Do I need to change that (i.e., create a proper python package in the build step) or is there a way to call cmake.test() after cmake.install()?

Thanks!

Have you read the CONTRIBUTING guide?

memsharded commented 1 month ago

Hi @pasbi

Thanks for your questions.

Are my build() and package() plausible?

yes, those are standard Conan recipe methods, it should be possible to do what you want.

In the CI (before introducing conan), there were separate jobs for building and unit testing. I would like to keep it separated (though it would be possible to merge them into one job, if required). Is there a way to separate building and testing?

It is possible, indeed, you can separate the creation of the package, containing only the build, and then move the testing (functional or e2e, that is, external testing, without access to private implementation detail), from a "consumer" job that basically conan install the package that you are creating and then uses the python and c++ code from the package, as a regular user of the package would do (just you do it for extensive testing)

Currently only installing mylib creates an importable python package, hence this step is required before cmake.test(). Do I need to change that (i.e., create a proper python package in the build step) or is there a way to call cmake.test() after cmake.install()?

It is not really necessary to install python things to make them available. You can make them available by adding their path to PYTHONPATH, then import xxxx will work. You can define dynamically the PYTHONPATH in the recipes, via different conan mechanisms, like defining it in the self.buildenv_info in the package_info() method of the package.

pasbi commented 1 month ago

Thanks for your reply.

It is possible, indeed, you can separate the creation of the package, containing only the build, and then move the testing (functional or e2e, that is, external testing, without access to private implementation detail), from a "consumer" job that basically conan install the package that you are creating and then uses the python and c++ code from the package, as a regular user of the package would do (just you do it for extensive testing)

But that means we'd need to add the unit tests to the package or to create two packages. Surely possible, but I'm afraid it gets too complicated for me to continue this path.

It is not really necessary to install python things to make them available. You can make them available by adding their path to PYTHONPATH, then import xxxx will work. You can define dynamically the PYTHONPATH in the recipes, via different conan mechanisms, like defining it in the self.buildenv_info in the package_info() method of the package.

But to my understanding, package_info() runs after build(), i.e., adding PYTHONPATH there won't make the bindings available in the unit tests, which are run in the build() method?

memsharded commented 1 month ago

But that means we'd need to add the unit tests to the package or to create two packages. Surely possible, but I'm afraid it gets too complicated for me to continue this path.

I understood that you already had completely separate CI jobs for this, so I thought you had highly decoupled tasks, so having the packaging in the "build" task and the package-consuming in the "test" task wouldn't really be more work than building and testing in the same place.

But to my understanding, package_info() runs after build(), i.e., adding PYTHONPATH there won't make the bindings available in the unit tests, which are run in the build() method?

That was intended for the separate testing in another job. For running building and testing in the same job and recipe, you can do something like:

def build(self):
     cmake = CMake(self)
     cmake.configure()
     cmake.build()
     self.run("export PYTHONPATH=<mypath> && ctest ...")

Just the idea, not something tested or complete. There are other possible approaches, like you could use an Environment() object to create a myenv script (typically in the scope="run", not in the build scope) file and have that automatically used by the self.run("ctest ...) or equivalent cmake.ctest(), which will run in the "run" scope

pasbi commented 1 month ago

I understood that you already had completely separate CI jobs for this, so I thought you had highly decoupled tasks, so having the packaging in the "build" task and the package-consuming in the "test" task wouldn't really be more work than building and testing in the same place.

Yes I do have separate jobs. But they share build artifacts (GitLab CI). We currently don't include tests in any of our packages.

Just the idea, not something tested or complete. There are other possible approaches, like you could use an Environment() object to create a myenv script (typically in the scope="run", not in the build scope) file and have that automatically used by the self.run("ctest ...) or equivalent cmake.ctest(), which will run in the "run" scope

Thanks for clarification, that really helps a lot. I was quite successful with generating a VirtualBuildEnv that sets up PATH and TEST_DATA_DIR in generate() before running the tests in build(). I guess putting the PYTHONPATH there will work (can't test it currently because of other issues).

I think I understood the general approach. There are still a lot of details to fix, but I'll close this thread. Thanks again, very helpful!

memsharded commented 1 month ago

Sounds good, thanks very much for the feedback!

Don't hesitate to create new tickets for any further question you might have.

pasbi commented 3 weeks ago

I think it's better to reopen than creating a new issue because it's very related.

@memsharded You said

It is not really necessary to install python things to make them available. You can make them available by adding their path to PYTHONPATH, then import xxxx will work. You can define dynamically the PYTHONPATH in the recipes, via different conan mechanisms, like defining it in the self.buildenv_info in the package_info() method of the package.

Yes, I agree, it's possible (and this technique works in my pipeline in practice). However, there are many ways to build python modules and they are meant to be usable after installation. They may be usable after build (before install) already, but I'd consider that a lucky coincidence.

In my particular case, the pyd file did not match the module name (import mylib but having a file mylib-python.cp312-win_amd64.pyd), and this seems to be a common issue.

Of course, there are ways to work around this (use set_target_properties, rename the pyd manually, create a __init__.py file manually, etc.), but my point is: These steps are handled by the installation step already. The real solution is not to use the python module before it is installed, unfortunately, conan forces me to.

I appreciate your feedback. Do you think it's worth searching a way to test after install, e.g., by moving the test call into the package() method or move cmake.install() into the build() method? Or do you think that ...

separate the creation of the package, containing only the build, and then move the testing (functional or e2e, that is, external testing, without access to private implementation detail), from a "consumer" job that basically conan install the package that you are creating and then uses the python and c++ code from the package, as a regular user of the package would do

... would be the cleaner approach (implying we need to ship tests to consumers or build two separate packages in the CI).

memsharded commented 3 weeks ago

However, there are many ways to build python modules and they are meant to be usable after installation.

The problem with Python installation is that it is something outside of Conan control, and it is quite problematic in practice. Installing things in Python requires a virtualenv to not pollute too much the environment, but this is something not that easy. Conan is already running in a Python interpreter, but it would be very discouraged to install things in that interpreter environments. There is already some work in https://github.com/conan-io/conan/pull/11601, to be able to create Python virtualenvs on the fly for Conan packages installing Python things, but it is a bit too hacky, so we couldn't prioritize it yet.

So beyond the actual place where the import of Python code happens, the install part is the problematic one. How do you plan to deal with it? Please let me know if this concern is something that makes sense to you, or maybe I am missing details about the "install" part.

pasbi commented 3 weeks ago

With "install", I mean what cmake --install ... or cmake --build ... --target install usually does (without conan) or cmake.install() in a conanfile.py. There is no need for a virtual environment, and it doesn't pollute anything (given that the install prefix is outside of any package manager's realm); I usually install somewhere in the build directory while developing, which reads (without conan): cmake --build build-dir; cmake --install build-dir --prefix build-dir/install.

That is, if I want to use the python module, I still need to set the PYTHONPATH (and mostly PATH or LD_LIBRARY_PATH on Linux). In this example something like $env:PYTHONPATH = "build-dir/install/lib/python" would be necessary (if the python module is build-dir/install/lib/python/mylib/mylib.cp312-win_amd64.pyd).

The term "install" is, unfortunately, quite ambiguous. I mean it in context of cmake, not concerning package managers like conan, pip, conda, etc.

You could argue that this approach in itself is hacky. It'd be better to build a proper package (e.g., pip package) and install it (best into a virtual environment). I'd agree, but I haven't looked into building pip packages (requiring external DLLs) and the current approach with local installations and manually set PYTHONPATH works for us.

I'd like to emphasize that my actual problem is solved (I simply import the package before installing it), this discussion is more or less academic because I'd like to understand the paradigms of conan better.

memsharded commented 3 weeks ago

I see, thanks for the clarification. It makes sense to keep things in the realm of cmake-install and keep away python virtualenvs and python packages installation.

If the process happens in cmake.install() time, then I see these alternatives:

I know it might be possible to do the testing in the package() method after the cmake.install(), but it sounds a bit dirty

memsharded commented 1 week ago

Hi @pasbi

Did you manage to make it work, did you try to implement the above with a local cmake.install() then a package() with copy()?

pasbi commented 1 week ago

The problem was solved using workarounds (i.a., applying https://github.com/pybind/pybind11/issues/415#issuecomment-247856419, copying some stuff around manually and joining building and testing into one CI-job).

That is, cmake.install() is in package() and the build() method ends with cmake.ctest().

I guess our main problem is that we haven't fully committed to conan yet but use it as a mere build and test tool (frankly speaking, mostly because we became tired of getting all the dependencies of OpenCV manually on Windows). I suppose conan is not really targeting this use case (understandably) and since we don't have time to tackle the problem on a higher level we're happy for now.

We have, however, plans to use conan for distributing and testing our proprietary software and we really want to split building and testing again soon™.

memsharded commented 1 week ago

The problem was solved using workarounds (i.a., applying https://github.com/pybind/pybind11/issues/415#issuecomment-247856419, copying some stuff around manually and joining building and testing into one CI-job).

Ok, sounds reasonable at the moment.

I guess our main problem is that we haven't fully committed to conan yet but use it as a mere build and test tool (frankly speaking, mostly because we became tired of getting all the dependencies of OpenCV manually on Windows). I suppose conan is not really targeting this use case (understandably) and since we don't have time to tackle the problem on a higher level we're happy for now.

yes, that is true, one thing is fetching and using some dependencies, but creating packages, building and testing driven by Conan recipes, etc, requires taking into consideration packaging/install issues a bit earlier on in the process.

We have, however, plans to use conan for distributing and testing our proprietary software and we really want to split building and testing again soon™.

Sounds great. Please do not hesitate to create new tickets for any further question or issue you might have. If you also want to connect more directly and talk with us, feel free to reach out (info@conan.io, social, etc)

I think this ticket can be closed by now, thanks very much for the feedback!