Skip to content

Commit

Permalink
chore: Enable ASAN/LSAN/UBSAN checks for code quality (#34)
Browse files Browse the repository at this point in the history
  • Loading branch information
elefeint authored Mar 26, 2024
1 parent 5e40db6 commit fa638da
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 16 deletions.
13 changes: 11 additions & 2 deletions .github/workflows/integration_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ on:
branches: [main]

jobs:

build_and_test:
name: Build and test the connector
runs-on: ubuntu-latest
Expand All @@ -20,16 +21,24 @@ jobs:

- name: build the dependencies
if: ${{ steps.cache-deps.outputs.cache-hit != 'true' }}
env:
CC: clang-15
CXX: clang++-15
CPP: clang-cpp-15
run: |
make build_openssl_native build_grpc build_arrow build_test_dependencies
- name: build the connector
env:
CC: clang-15
CXX: clang++-15
CPP: clang-cpp-15
run: |
make get_fivetran_protos get_duckdb
make build_connector
make build_connector_debug
- name: run tests
env:
motherduck_token: ${{ secrets.CI_TESTING_TOKEN }}
run: |
./build/Release/integration_tests
ASAN_OPTIONS=alloc_dealloc_mismatch=0:fast_unwind_on_malloc=0 LSAN_OPTIONS=suppressions=lsan-suppressions.supp ./build/Debug/integration_tests
23 changes: 23 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,29 @@
cmake_minimum_required(VERSION 3.8)
project(MotherDuckFivetranDestination C CXX)


# Debug Asan/Ubsan
if(CMAKE_BUILD_TYPE STREQUAL "Debug" AND NOT DISABLE_SANITIZER)
if("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang$")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize-blacklist=${CMAKE_CURRENT_LIST_DIR}/sanitizer-disallowed-entries.txt")
set(ENABLE_UBSAN TRUE)
else()
set(ENABLE_UBSAN FALSE)
endif()

add_compile_options("-fsanitize=address,undefined")
add_link_options("-fsanitize=address,undefined")
set(ENABLE_SANITIZER TRUE)
else()
set(ENABLE_SANITIZER FALSE)
set(ENABLE_UBSAN FALSE)
endif()

message("-- ENABLE_SANITIZER=${ENABLE_SANITIZER}")
message("-- ENABLE_UBSAN=${ENABLE_UBSAN}")

# Proto generation

set(TARGET_ROOT ${PROJECT_SOURCE_DIR}/gen)
include(./proto_helper.cmake)

Expand Down
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,3 @@ To upgrade, change `DUCKDB_VERSION` in [Makefile](Makefile) and re-run `make get

## TODO
- support GZIP compression
- implement truncate-before (utc_delete_before) and synced_column = 4;
- implement soft delete?
3 changes: 3 additions & 0 deletions lsan-suppressions.supp
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# https://github.com/MotherDuck-Open-Source/motherduck-fivetran-connector/issues/33
leak:arrow::internal::StaticVectorImpl
leak:duckdb::GZipFileSystem
4 changes: 4 additions & 0 deletions sanitizer-disallowed-entries.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# cf. https://clang.llvm.org/docs/SanitizerSpecialCaseList.html
# nothing excluded for now
#src:*
#fun:*
23 changes: 11 additions & 12 deletions test/integration/test_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,18 @@ bool IS_FAIL(const grpc::Status &status, const std::string &expected_error) {

TEST_CASE("ConfigurationForm", "[integration]") {
DestinationSdkImpl service;
::fivetran_sdk::ConfigurationFormRequest request;
::fivetran_sdk::ConfigurationFormResponse response;

auto request = ::fivetran_sdk::ConfigurationFormRequest().New();
auto response = ::fivetran_sdk::ConfigurationFormResponse().New();

auto status = service.ConfigurationForm(nullptr, request, response);
auto status = service.ConfigurationForm(nullptr, &request, &response);
REQUIRE_NO_FAIL(status);

REQUIRE(response->fields_size() == 2);
REQUIRE(response->fields(0).name() == "motherduck_token");
REQUIRE(response->fields(1).name() == "motherduck_database");
REQUIRE(response->tests_size() == 1);
REQUIRE(response->tests(0).name() == CONFIG_TEST_NAME_AUTHENTICATE);
REQUIRE(response->tests(0).label() == "Test Authentication");
REQUIRE(response.fields_size() == 2);
REQUIRE(response.fields(0).name() == "motherduck_token");
REQUIRE(response.fields(1).name() == "motherduck_database");
REQUIRE(response.tests_size() == 1);
REQUIRE(response.tests(0).name() == CONFIG_TEST_NAME_AUTHENTICATE);
REQUIRE(response.tests(0).label() == "Test Authentication");
}

TEST_CASE("DescribeTable fails when database missing", "[integration]") {
Expand Down Expand Up @@ -301,7 +300,7 @@ std::unique_ptr<duckdb::Connection> get_test_connection(char *token) {
return std::make_unique<duckdb::Connection>(db);
}

TEST_CASE("WriteBatch", "[integration][current]") {
TEST_CASE("WriteBatch", "[integration][write-batch]") {
DestinationSdkImpl service;

// Schema will be main
Expand Down Expand Up @@ -581,7 +580,7 @@ TEST_CASE("WriteBatch", "[integration][current]") {
}
}

TEST_CASE("Table with multiple primary keys", "[integration]") {
TEST_CASE("Table with multiple primary keys", "[integration][write-batch]") {
DestinationSdkImpl service;

const std::string table_name =
Expand Down

0 comments on commit fa638da

Please sign in to comment.