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

Fix/move icp tests to separate folder #509

Conversation

YoshuaNava
Copy link
Contributor

Description

This PR separates the main file for libpointmatcher's unit tests (utest.cpp), from the existing ICP tests. The motivation being that, I have more ICP tests in implementation stage, and I see the potential to cover more facets of the algorithm.

Tests

All succeeding locally.

Instructions

I tested this locally with a ROS1 catkin build, by

  1. Cloning libpointmatcher into a catkin workspace, along with the latest version of libnabo
  2. Building with catkin build -DBUILD_TESTS=Enabled
  3. Executing the tests from the catkin workspace folder with $ ./build/libpointmatcher/utest/utest --path <path_to_workspace>/src/libpointmatcher/examples/data/

@pomerlef
Copy link
Collaborator

pomerlef commented Mar 7, 2023

Add to whitelist

@YoshuaNava
Copy link
Contributor Author

Hi @pomerlef,
There is a message saying that all checks have failed, but no tests were run:

image

What could be the issue? I also see it in #508

@YoshuaNava
Copy link
Contributor Author

Hi @pomerlef,
Sorry to bother. I wanted to catch up on the status of this MR, and your thoughts on it.

Thank you in advance 🙏

Copy link
Collaborator

@pomerlef pomerlef left a comment

Choose a reason for hiding this comment

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

Looks good on my side

@YoshuaNava
Copy link
Contributor Author

Thank you!

About the pipeline, it doesn't seem to be running atm:

image

Is there any way I could trigger it?

@pomerlef
Copy link
Collaborator

The CI tool setup by ASL seems out of order. I need to set up a local machine to validate the modification. Once it is done, I'll start processing the PRs.

@YoshuaNava
Copy link
Contributor Author

YoshuaNava commented Mar 16, 2023

Oki 👍

@pomerlef
Copy link
Collaborator

This was manually merged into master.

@pomerlef pomerlef closed this Mar 17, 2023
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.

2 participants