dev branch: cmake + catch + travis fix#145
Conversation
|
@zauguin please review. |
.travis.yml
Outdated
| - libsqlite3-dev | ||
| - libsqlcipher-dev | ||
| - libboost-all-dev | ||
| - cmake-data |
There was a problem hiding this comment.
Cmake depends on cmake-data anyway, so this is redundant. I think we should remove it to keep the list of packages short.
Maybe gcc-5 can be removed too, it's just a dependency of g++-5
.travis.yml
Outdated
| packages: | ||
| - gcc-5 | ||
| - g++-5 | ||
| - libsqlite3-dev |
There was a problem hiding this comment.
CMake ignores the preinstalled SQLite and downloads and installs it's own version, so there is no point in requiring this.
tests/exception_dont_execute.cc
Outdated
|
|
||
|
|
||
| int main() { | ||
| TEST_CASE("Prepered statement will not execute on exceptions", "[prepared_statements]") { |
.ycm_extra_conf.py
Outdated
| if compilation_database_folder: | ||
| database = ycm_core.CompilationDatabase(compilation_database_folder) | ||
| else: | ||
| database = None |
There was a problem hiding this comment.
The else block is unreachable
CMakeLists.txt
Outdated
|
|
||
| set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_SOURCE_DIR}/cmake") | ||
| include("cmake/HunterGate.cmake") | ||
| include("cmake/ParseAndAddCatchTests.cmake") |
There was a problem hiding this comment.
ParseAndAddCatchTests tries to parse the source to extract the tests.
This leads to problems with our disabled tests (especially the variant test).
These problems could probably be fixed by using this script from the Catch repo instead.
.travis.yml
Outdated
| - export CXX="g++-5" CC="gcc-5" | ||
|
|
||
| script: ./configure && make test && make clean && make LDFLAGS="-lsqlcipher -DENABLE_SQLCIPHER_TESTS" test | ||
| script: mkdir build && cd ./build && cmake .. && make && ./tests |
There was a problem hiding this comment.
Using make test instead of ./tests would be less surprising for newcomers familiar with cmake
| '.', | ||
| '-isystem', '/usr/local/include', | ||
| '-isystem', '/usr/include', | ||
| '-I.', |
There was a problem hiding this comment.
The current directory is already added on line 21 and 22.
|
Is there a reason you want to add the |
|
Thank you for reviewing :-)
I removed the flags you mentioned, feel free to update this file if you are using YCM too. |
2 participants
No description provided.