Skip to content
This repository was archived by the owner on May 9, 2024. It is now read-only.

Fix sanity_tests run for the top-level CMakeLists #149

Merged
merged 7 commits into from
Jan 19, 2023

Conversation

ienkovich
Copy link
Contributor

Currently, we have to use omniscidb/CMakeLists.txt directly to build and run sanity tests mostly due to relative paths used in tests to reach the test data and runtime libs.

This PR introduces a few changes to enable tests run from the top-level build dir:

  • configure data files and headers (for UdfTest) paths through macroses
  • allow alternative java classpath to properly find calcite library
  • do not force static sqlite library to avoid multiple global sqlite states linked into different shared libraries (causing segfaults on testing)
  • enforce -O0 for debug build (we had a similar patch for omnisci fork)
  • avoid unexpected dependencies for Shared on IR library through toString method
  • avoid unexpected dependencies for Util library through globals by excluding unused code from build (don't completely remove it to probably move part of it to ConfigBuilder later)

@kurapov-peter This PR finally allows us to build and run sanity_tests for HDK from any place, not necessarily from the dir created at the top of the source tree.

@ienkovich ienkovich force-pushed the ienkovich/fix-top-level-sanity-tests branch from a8e1f5a to 66137ae Compare January 12, 2023 17:49
Copy link
Contributor

@alexbaden alexbaden left a comment

Choose a reason for hiding this comment

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

Looks fine, some comments inline.

Copy link
Contributor

@kurapov-peter kurapov-peter left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@kurapov-peter kurapov-peter merged commit dca8bec into main Jan 19, 2023
@kurapov-peter kurapov-peter deleted the ienkovich/fix-top-level-sanity-tests branch January 19, 2023 14:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants