feature/unittests #134

761417898 · 2024-06-21T08:55:18Z

The unit test framework chosen is GTest(#111 (comment)), as recommended by Chris.
The unit tests have been implemented for the common utils file encoding section.

ColinLeeo · 2024-06-25T07:03:50Z

Excellent job. Google Test is a good choice for verification testing in C/C++. However, it seems there is still an issue with the POM configuration.

ColinLeeo

After run UTs, there is core dump:

(lldb) bt
* thread #1, name = 'TsFile_Test', stop reason = signal SIGABRT
  * frame #0: 0x00007fe14801c9fc libc.so.6`__GI___pthread_kill at pthread_kill.c:44:76
    frame #1: 0x00007fe14801c9b0 libc.so.6`__GI___pthread_kill [inlined] __pthread_kill_internal(signo=6, threadid=140605551810368) at pthread_kill.c:78:10
    frame #2: 0x00007fe14801c9b0 libc.so.6`__GI___pthread_kill(threadid=140605551810368, signo=6) at pthread_kill.c:89:10
    frame #3: 0x00007fe147fc8476 libc.so.6`__GI_raise(sig=6) at raise.c:26:13
    frame #4: 0x00007fe147fae7f3 libc.so.6`__GI_abort at abort.c:79:7
    frame #5: 0x00007fe14800f676 libc.so.6`__libc_message(action=do_abort, fmt="\xd8\U00000003") at libc_fatal.c:155:5
    frame #6: 0x00007fe148026cfc libc.so.6`malloc_printerr(str=<unavailable>) at malloc.c:5664:3
    frame #7: 0x00007fe148028a44 libc.so.6`_int_free(av=<unavailable>, p=<unavailable>, have_lock=0) at malloc.c:4439:5
    frame #8: 0x00007fe14802b453 libc.so.6`__GI___libc_free(mem=<unavailable>) at malloc.c:3391:7
    frame #9: 0x000055a4b1e76f69 TsFile_Test`common::Value::~Value() + 45
    frame #10: 0x000055a4b1e7387f TsFile_Test`common::ValueTest_IsTypeAndIsLiteral_Test::TestBody() + 2941
    frame #11: 0x000055a4b1f468c3 TsFile_Test`void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) + 105
    frame #12: 0x000055a4b1f3f17d TsFile_Test`void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) + 94
    frame #13: 0x000055a4b1f1aec6 TsFile_Test`testing::Test::Run() + 260
    frame #14: 0x000055a4b1f1b9e1 TsFile_Test`testing::TestInfo::Run() + 395
    frame #15: 0x000055a4b1f1c31f TsFile_Test`testing::TestSuite::Run() + 373
    frame #16: 0x000055a4b1f2c6cd TsFile_Test`testing::internal::UnitTestImpl::RunAllTests() + 1051
    frame #17: 0x000055a4b1f47cf9 TsFile_Test`bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) + 105
    frame #18: 0x000055a4b1f40267 TsFile_Test`bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) + 94
    frame #19: 0x000055a4b1f2acef TsFile_Test`testing::UnitTest::Run() + 199
    frame #20: 0x000055a4b1f07b4b TsFile_Test`RUN_ALL_TESTS() + 21
    frame #21: 0x000055a4b1f07ac4 TsFile_Test`main + 73
    frame #22: 0x00007fe147fafd90 libc.so.6`__libc_start_call_main(main=(TsFile_Test`main), argc=1, argv=0x00007ffef1a874d8) at libc_start_call_main.h:58:16
    frame #23: 0x00007fe147fafe40 libc.so.6`__libc_start_main_impl(main=(TsFile_Test`main), argc=1, argv=0x00007ffef1a874d8, init=(_rtld_global), fini=<unavailable>, rtld_fini=<unavailable>, stack_end=0x00007ffef1a874c8) at libc-start.c:392:3
    frame #24: 0x000055a4b1e35235 TsFile_Test`_start + 37
(lldb) exit

ColinLeeo · 2024-06-25T07:24:20Z

cpp/test/CMakeLists.txt

+# https://github.com/google/leveldb/blob/main/CMakeLists.txt#L353
+set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
+
+include(FetchContent)
+FetchContent_Declare(
+  googletest
+  URL https://github.com/google/googletest/archive/03597a01ee50ed33e9dfd640b249b4be3799d395.zip
+)
+set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
+FetchContent_MakeAvailable(googletest)
+


Is the requirement for the C++14 standard a bit high? The code does not use very advanced language features.
Can we consider introducing the test binary files or code as third-party dependencies?

ColinLeeo · 2024-06-25T07:30:37Z

cpp/test/CMakeLists.txt

+FetchContent_Declare(
+  googletest
+  URL https://github.com/google/googletest/archive/03597a01ee50ed33e9dfd640b249b4be3799d395.zip
+)


change it to google test repo and release tag.

ColinLeeo · 2024-06-25T07:31:27Z

cpp/test/CMakeLists.txt

+set(SDK_INCLUDE_DIR ${PROJECT_SOURCE_DIR}/../src)
+message("SDK_INCLUDE_DIR: ${SDK_INCLUDE_DIR}")
+set(SDK_LIB_DIR_RELEASE ${PROJECT_SOURCE_DIR}/../build/Release/lib)
+message("SDK_LIB_DIR_RELEASE: ${SDK_LIB_DIR_RELEASE}")
+


With Maven, libtsfile is stored in target/build/lib.

ColinLeeo · 2024-06-25T07:57:09Z

cpp/test/utils/db_utils_test.cc

+#include <string.h>
+#include <sys/time.h>


ColinLeeo · 2024-06-25T08:02:33Z

cpp/test/encoding/bitpack_codec_test.cc

+    std::vector<int64_t> values = {1, 2, 3, 4, 5, 255};
+    int bit_width = encoder.get_int_max_bit_width(values);


std::vector/std::vector<int32_t>

ColinLeeo · 2024-06-25T08:05:51Z

cpp/test/encoding/dictionary_codec_test.cc

+ */
+#include <gtest/gtest.h>
+
+#include <map>


ColinLeeo · 2024-06-25T08:44:27Z

cpp/test/encoding/inpacker_test.cc

+
+TEST(IntPackerTest, Pack8Values) {
+    IntPacker int_packer(10);
+    int64_t values[NUM_OF_INTS] = {100, 200, 300, 400, 500, 600, 700, 800};


int64_t -> int/int32_t
same as below

cpp/test/common/container/byte_buffer_test.cc

cpp/test/common/record_test.cc

cpp/test/writer/tsfile_writer_test.cc

ColinLeeo · 2024-07-02T07:23:09Z

LGTM

ColinLeeo · 2024-07-02T13:36:20Z

The C++ 14 version is actually a bit high; we should at least support up to C++ 11. Google Test might only support C++ 14 after a certain version, so perhaps we should adjust the version of Google Test.

The test is compiled using C++ 14, but the code is written in C++ 11, which may lead to some errors.

761417898 · 2024-07-03T07:53:48Z

The C++ 14 version is actually a bit high; we should at least support up to C++ 11. Google Test might only support C++ 14 after a certain version, so perhaps we should adjust the version of Google Test.

The test is compiled using C++ 14, but the code is written in C++ 11, which may lead to some errors.

I have updated the version of GTest. V0.12 is the latest version with support for C++11

761417898 · 2024-07-03T10:28:02Z

The issues pointed out by the review have been fixed, are we ready to prepare for the merge? @jt2594838

* bugfix: bitmap clear method * feature/unittests * add license * support cmake-maven * support cmake-maven * unittest support cmake-maven * unittest support cmake-maven * fix for macos compilation * fix for macos compilation * fix for macos compilation * fix for macos compilation * fix unittest dependency * fix macos compilation * fix win compilation * fix win compilation * fix win compilation * test * test * test * fix windows mingw compilation * fix mingw compilation * test * fix MinGW compilation * fix MinGW compilation * fix MinGW compilation * fix MinGW compilation * test compress * test/encoding * test/write * testcompress * testencoding * testcommon * testutils * testfile * fix MinGW compilation * support cross platform compilation * support mingw macos ubuntu compilation * fix code formatting issues * fix newly introduced encoding issue(bitpack) --------- Co-authored-by: hongzhigao <hongzhigao@foxmail.com>

hongzhigao and others added 5 commits June 14, 2024 20:58

bugfix: bitmap clear method

29a9a06

Merge branch 'apache:develop' into develop

ea0818c

Merge branch 'apache:develop' into develop

5312b8c

Merge branch 'apache:develop' into develop

a556cd1

feature/unittests

2962c70

761417898 mentioned this pull request Jun 21, 2024

[CPP] Add unit test #111

Closed

add license

a5510be

ColinLeeo reviewed Jun 25, 2024

View reviewed changes

761417898 added 6 commits June 26, 2024 15:13

Merge branch 'apache:develop' into feature/unittests

3ec364f

support cmake-maven

6b0f936

support cmake-maven

472c933

Merge branch 'apache:develop' into feature/unittests

11b6c28

unittest support cmake-maven

a4c7f9b

unittest support cmake-maven

51a3748

761417898 requested a review from ColinLeeo June 28, 2024 11:21

761417898 added 5 commits June 28, 2024 21:30

fix for macos compilation

73baaca

fix for macos compilation

b10fafe

fix for macos compilation

3eb753c

fix for macos compilation

38c80fd

fix unittest dependency

37cf01a

761417898 force-pushed the feature/unittests branch 2 times, most recently from 0f40985 to 37cf01a Compare June 29, 2024 05:38

761417898 added 7 commits June 29, 2024 14:14

fix macos compilation

62208df

fix win compilation

2a6c306

fix win compilation

711315b

fix win compilation

b31da4b

test

d630230

test

c6d96bf

test

e10ee82

761417898 added 17 commits June 30, 2024 16:32

test

285f396

fix MinGW compilation

56f7205

fix MinGW compilation

ac58c7d

fix MinGW compilation

279203b

fix MinGW compilation

82e8fba

test compress

9783c39

test/encoding

e5cf880

test/write

82b1786

testcompress

b934603

testencoding

7d231f4

testcommon

e021e5f

testutils

6569c7a

testfile

313b32e

fix MinGW compilation

51fb5b8

Merge branch 'apache:develop' into feature/unittests

ac0b18f

support cross platform compilation

c0105ea

support mingw macos ubuntu compilation

0ade727

jt2594838 approved these changes Jul 2, 2024

View reviewed changes

cpp/test/common/container/byte_buffer_test.cc Outdated Show resolved Hide resolved

cpp/test/common/record_test.cc Outdated Show resolved Hide resolved

cpp/test/writer/tsfile_writer_test.cc Show resolved Hide resolved

ColinLeeo approved these changes Jul 2, 2024

View reviewed changes

fix code formatting issues

8f48e51

761417898 and others added 2 commits July 4, 2024 09:44

Merge branch 'apache:develop' into feature/unittests

df3e9a3

fix newly introduced encoding issue(bitpack)

ec50e7e

jt2594838 merged commit 0e337f9 into apache:develop Jul 4, 2024
13 checks passed

HTHou linked an issue Jul 5, 2024 that may be closed by this pull request

[CPP] Add unit test #111

Closed

761417898 deleted the feature/unittests branch July 6, 2024 06:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature/unittests #134

feature/unittests #134

761417898 commented Jun 21, 2024

ColinLeeo commented Jun 25, 2024

ColinLeeo left a comment •

edited

Loading

ColinLeeo Jun 25, 2024

ColinLeeo Jun 25, 2024

ColinLeeo Jun 25, 2024 •

edited

Loading

ColinLeeo Jun 25, 2024

ColinLeeo Jun 25, 2024

ColinLeeo Jun 25, 2024

ColinLeeo Jun 25, 2024

ColinLeeo commented Jul 2, 2024

ColinLeeo commented Jul 2, 2024 •

edited

Loading

761417898 commented Jul 3, 2024

761417898 commented Jul 3, 2024

		std::vector<int64_t> values = {1, 2, 3, 4, 5, 255};
		int bit_width = encoder.get_int_max_bit_width(values);

		#include <string.h>
		#include <sys/time.h>

feature/unittests #134

feature/unittests #134

Conversation

761417898 commented Jun 21, 2024

ColinLeeo commented Jun 25, 2024

ColinLeeo left a comment • edited Loading

Choose a reason for hiding this comment

ColinLeeo Jun 25, 2024

Choose a reason for hiding this comment

ColinLeeo Jun 25, 2024

Choose a reason for hiding this comment

ColinLeeo Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

ColinLeeo Jun 25, 2024

Choose a reason for hiding this comment

ColinLeeo Jun 25, 2024

Choose a reason for hiding this comment

ColinLeeo Jun 25, 2024

Choose a reason for hiding this comment

ColinLeeo Jun 25, 2024

Choose a reason for hiding this comment

ColinLeeo commented Jul 2, 2024

ColinLeeo commented Jul 2, 2024 • edited Loading

761417898 commented Jul 3, 2024

761417898 commented Jul 3, 2024

ColinLeeo left a comment •

edited

Loading

ColinLeeo Jun 25, 2024 •

edited

Loading

ColinLeeo commented Jul 2, 2024 •

edited

Loading