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

Test other generators: run several tests in test_*/conanfile.py #8389

Merged
merged 24 commits into from
Feb 16, 2022

Conversation

jgsogo
Copy link
Contributor

@jgsogo jgsogo commented Dec 10, 2021

We are introducing several changes with Conan v2 in mind, the biggest effort and challenge will be to modify recipes to work with new features backported from Conan v2 while we ensure they keep working with current Conan version.

One of the things we need to be really sure that works is the logic behind the generators and cpp_info. The best way to do it is to actually run a test_package for them. For sure we can run everything in the same test_package/conanfile.py, but IMHO it would be much clear to have each generator being tested in its own test_* folder. We don't need to go crazy with it, neither add it to every single recipe, but it might be really useful for those that define some convoluted cpp_info.name/filename and/or set_property together with components.

This is just a test and a proposal. The main point is that project files should be the same: same CMakeLists.txt, same .h/.cpp files. Only conanfile.py is different. Here I propose to keep this files in the test_package folder as it is the official one, while the other/s folder/s will just use sources from it.

Let's hear the CI and the people. Wdyt?

Note.- This feature was already there, but not documented.

@ghost
Copy link

ghost commented Dec 10, 2021

I detected other pull requests that are modifying fmt/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@jgsogo
Copy link
Contributor Author

jgsogo commented Dec 10, 2021

As you can see in the table outputs, in the test logs, both test-package are executed:

********************************************************************************
conan test conan-center-index/recipes/fmt/all/test_cmakedeps/conanfile.py fmt/5.3.0@ --profile=/home/conan/w/BuildSingleReference/60145/3345f072-8cfa-48fc-ae4f-9fc0a3346760/profile.txt
********************************************************************************

...

********************************************************************************
conan test conan-center-index/recipes/fmt/all/test_package/conanfile.py fmt/5.3.0@ --profile=/home/conan/w/BuildSingleReference/60145/3345f072-8cfa-48fc-ae4f-9fc0a3346760/profile.txt
********************************************************************************

cc/ @czoido @SSE4 @franramirez688 any suggestions about the project structure?

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@ericLemanissier
Copy link
Contributor

what is the witchcraft which makes C3I run test_cmakedeps ? most of the contributors most likely just run conan create ... lib/1.0@ which only runs test_package right ?

@jgsogo
Copy link
Contributor Author

jgsogo commented Dec 13, 2021

what is the witchcraft which makes C3I run test_cmakedeps ? most of the contributors most likely just run conan create ... lib/1.0@ which only runs test_package right ?

@ericLemanissier , in C3i we run conan install --build=XXX and then conan test in order to provide better information in the output table (we can say if the error comes from the build or from the testing). The command conan test takes an argument which is the path to the conanfile.py from the test package... and we are just iterating all the folders that match test_*** pattern.

Of course, we will:

  • document this behavior in this repository
  • write a hook to ensure that there is always a test_package (users cloning the repo and running conan create will expect some test)

In this PR I'm interested in knowing if this is the best approach to test different generators, or we can figure out something better.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@prince-chrismc
Copy link
Contributor

what is the witchcraft which makes C3I run test_cmakedeps ?

We'll need to write some docs for this :)

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
@conan-center-bot

This comment has been minimized.

project(test_package CXX)

include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup(TARGETS)
if(EXISTS ${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is it needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed because the same CMakeLists.txt file is used from the test_cmakedeps/conanfile.py and there we are not adding the cmake generator.

IMO, it is important to reuse as much as possible the same project files.

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

All green in build 29 (18da51d56a5a625be943d4e867b949683ffa79bc):

  • fmt/6.2.1@:
    All packages built successfully! (All logs)

  • fmt/8.0.1@:
    All packages built successfully! (All logs)

  • fmt/5.3.0@:
    All packages built successfully! (All logs)

  • fmt/8.1.1@:
    All packages built successfully! (All logs)

  • fmt/7.1.3@:
    All packages built successfully! (All logs)

Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing, a new step towards Conan 2.0

@jgsogo
Copy link
Contributor Author

jgsogo commented Feb 15, 2022

@prince-chrismc , can you review here again? Thanks!

@conan-center-bot conan-center-bot merged commit e9f0b66 into conan-io:master Feb 16, 2022
@jgsogo jgsogo deleted the multiple-testing branch February 16, 2022 09:57
SSE4 pushed a commit to madebr/conan-center-index that referenced this pull request Feb 21, 2022
…nanfile.py

* [poc] Test other generators: run several tests in test_*/conanfile.py

* use new pattern to access info from dependencies

* use rel path

* fix import (1.43.1)

* Update recipes/fmt/all/test_package/CMakeLists.txt

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>

* VS output path for CMakeDeps uses configuration type

* make it a string

* force output directory

* force it for the configuration name \o/

* following team feedback, this should be the way to go

* use cmake_layout explicitly

* we can delegate the path to the source to the layout

* Update recipes/fmt/all/test_cmakedeps/conanfile.py

Co-authored-by: Uilian Ries <uilianries@gmail.com>

* touch

* touch

* revert change

* revert back, explained in PR review

* Update recipes/fmt/all/test_cmakedeps/conanfile.py

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>

* touch

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants