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

geographiclib: Add unit testing #24173

Closed
wants to merge 1 commit into from

Conversation

FannoFlow1
Copy link
Contributor

@FannoFlow1 FannoFlow1 commented May 30, 2024

add unit testing to geographiclib builds

Specify library name and version: geographiclib/2.3

resolves #24172

try adding cmake.test()
@FannoFlow1 FannoFlow1 changed the title Update conanfile.py Add unit testing to Geographiclib May 30, 2024
@FannoFlow1 FannoFlow1 changed the title Add unit testing to Geographiclib geographiclib: Add unit testing May 30, 2024
@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 1 (ed06d3d72672c9ecb366ffe5f603f60caa9ecd4a):

  • geographiclib/1.51:
    All packages built successfully! (All logs)

  • geographiclib/2.3:
    All packages built successfully! (All logs)

  • geographiclib/1.50.2:
    All packages built successfully! (All logs)

  • geographiclib/1.52:
    All packages built successfully! (All logs)


Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 1 (ed06d3d72672c9ecb366ffe5f603f60caa9ecd4a):

  • geographiclib/2.3:
    All packages built successfully! (All logs)

  • geographiclib/1.50.2:
    All packages built successfully! (All logs)

  • geographiclib/1.51:
    All packages built successfully! (All logs)

  • geographiclib/1.52:
    All packages built successfully! (All logs)

@jcar87
Copy link
Contributor

jcar87 commented May 31, 2024

Hi @FannoFlow1, thank you for your contribution.

We don't currently build or run tests during the package builds, in particular in our CI as the validity of the packages is a responsibility we place on the users who wish to test that. This is also in line with other package managers.

However let me check with the team, as it is still a useful feature that should be optional in the recipes.
We introduced the tools.build:skip_test configuration for this very purpose - this way the recipes could conditionally build and run tests.

@FannoFlow1
Copy link
Contributor Author

Hi @FannoFlow1, thank you for your contribution.

We don't currently build or run tests during the package builds, in particular in our CI as the validity of the packages is a responsibility we place on the users who wish to test that. This is also in line with other package managers.

However let me check with the team, as it is still a useful feature that should be optional in the recipes. We introduced the tools.build:skip_test configuration for this very purpose - this way the recipes could conditionally build and run tests.

Ah interesting! For some reason I thought I was seeing unit tests in other packages being built, with this being an outlier. I must have been seeing something else in the logs since you're right the recipes I'm looking at don't run tests.

I think it would be very desirable to have cmake.test() in the conan center recipes which leverages the tools.build:skip_test property internally.

@FannoFlow1
Copy link
Contributor Author

FannoFlow1 commented May 31, 2024

I would also argue it would be very important to run the unit tests with the conancenter index packages because there are so many compiler variables. The compiler settings the package author tested with might be (or likely are) different than the conan center index builds. And I'm not sure its a safe assumption to argue that those differences dont matter.
Now most probably dont have an affect on most packages, but I'm not sure you can guarantee that for all packages in all builds without running the unit tests. Especially if downstream users of those binaries are considering them pre-tested, but I'm not sure we can assume they've been pre-tested since the unit tests aren't running during the build process.

And same goes for the downstream user building from source. At best you'll get compiler / linker errors, at worst you have a silent runtime issue giving you the wrong answer on some math.

@perseoGI
Copy link
Contributor

perseoGI commented Sep 3, 2024

Hi @FannoFlow1!

Thank you for your interest in Conan Center Index.
Running unit tests is not a CCI policy so we are closing this PR because we can't find the value in deviating from it.

Nevertheless, thanks a lot for taking the time to open the PR, we appreciate it 🐸

@perseoGI perseoGI closed this Sep 3, 2024
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.

[package] geographiclib/2.3: Run unitests on GeographicLib Builds
5 participants