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.

Sign up for GitHub

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

Jump to bottom

Fix ompl tests builds #178

Merged
merged 4 commits into from
Apr 18, 2017
Merged

Fix ompl tests builds #178

merged 4 commits into from
Apr 18, 2017

Conversation

jslee02
Copy link
Member

@jslee02 jslee02 commented Apr 17, 2017 •
edited
Loading

OMPL_FOUND is only defined in the scope of <aikido_dir>/src/planner/ompl so tests/planner/CMakeListx.txt couldn't check if ompl is found. This PR instead check whether aikido_planner_ompl target is built to decide whether Aikido builds the ompl tests or not.

This problem was discovered by @aditya-vk .

@jslee02 jslee02 added this to the Aikido 0.1.0 milestone Apr 17, 2017
@jslee02 jslee02 requested review from mkoval and aditya-vk April 17, 2017 23:51
@aditya-vk
Copy link
Contributor

aditya-vk commented Apr 18, 2017 •
edited
Loading

I don't know the right fix yet but the edit does not still generate the executables for tests defined under tests/planner/ompl like it does for tests/planner/parabolic.

@aditya-vk aditya-vk mentioned this pull request Apr 18, 2017
Merged
@jslee02
Copy link
Member Author

jslee02 commented Apr 18, 2017 •
edited
Loading

So far, ompl tests were not being tested. Many of errors are fixed by 372517b.

mkoval
mkoval approved these changes Apr 18, 2017
View reviewed changes
Copy link
Member

@mkoval mkoval left a comment

Choose a reason for hiding this comment

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

Good catch! LGTM. 👏

@coveralls
Copy link

Coverage Status

Coverage increased (+3.1%) to 80.374% when pulling 6c4b5ec on test/fix_planner_ompl into 38ce8e6 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+3.1%) to 80.35% when pulling 6c4b5ec on test/fix_planner_ompl into 38ce8e6 on master.

@jslee02 jslee02 merged commit 2af5c55 into master Apr 18, 2017
@jslee02 jslee02 deleted the test/fix_planner_ompl branch April 18, 2017 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkoval mkoval mkoval approved these changes

@aditya-vk aditya-vk Awaiting requested review from aditya-vk

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Aikido 0.1.0
Development

Successfully merging this pull request may close these issues.
4 participants
@jslee02 @aditya-vk @coveralls @mkoval