diff --git a/src/log/apitest/tet_saLogDispatch.c b/src/log/apitest/tet_saLogDispatch.c index 30597d6..4495c05 100644 --- a/src/log/apitest/tet_saLogDispatch.c +++ b/src/log/apitest/tet_saLogDispatch.c @@ -17,6 +17,7 @@ #include "logtest.h" +// [Lennart] Why not comment this as well? void saLogDispatch_01(void) { rc = logInitialize(); @@ -33,6 +34,9 @@ void saLogDispatch_01(void) * step3:Now call the saLogDispatch() with logHandle * Result: Shall fail with return code SA_AIS_ERR_BAD_HANDLE */ +// [Lennart] See comments for log stream close +// For all functions here the comment about descriptive +// name is valid void saLogDispatch_02(void) { rc = logInitialize(); diff --git a/src/log/apitest/tet_saLogFinalize.c b/src/log/apitest/tet_saLogFinalize.c index 74faea3..e01c58a 100644 --- a/src/log/apitest/tet_saLogFinalize.c +++ b/src/log/apitest/tet_saLogFinalize.c @@ -38,6 +38,7 @@ void saLogFinalize_02(void) * step3:Now call the saLogFinalize() with logHandle * Result: Shall fail with return code SA_AIS_ERR_BAD_HANDLE */ +// [Lennart] See comment for log stream close void saLogFinalize_03(void) { SaVersionT log_version = kLogVersion; diff --git a/src/log/apitest/tet_saLogInitialize.c b/src/log/apitest/tet_saLogInitialize.c index dd3e474..2c9b198 100644 --- a/src/log/apitest/tet_saLogInitialize.c +++ b/src/log/apitest/tet_saLogInitialize.c @@ -16,6 +16,15 @@ * */ +// [Lennart] The name of this file does not match its content. +// The file name suggests that this file is for testing saLogInitialize() +// but other tests are added like saLogDispatch() and tests for saLogFinalize() +// There is a file named tet_saLogFinalize, why not add tests of saLogFinalize() +// in that file instead? Is there a file for testing saLogDispatch()? In any case +// Tests for testing saLogDispatch() should be place in an appropriate file +// NOTE: This comment is applicable for several of the new test cases (and maybe +// some older as well?) + #include "logtest.h" void saLogInitialize_01(void) diff --git a/src/log/apitest/tet_saLogSelectionObjectGet.c b/src/log/apitest/tet_saLogSelectionObjectGet.c index 465f1e7..29316af 100644 --- a/src/log/apitest/tet_saLogSelectionObjectGet.c +++ b/src/log/apitest/tet_saLogSelectionObjectGet.c @@ -39,6 +39,7 @@ void saLogSelectionObjectGet_02(void) * step3:Now call the saLogSelectionObjectGet() with logHandle * Result: Shall fail with return code SA_AIS_ERR_BAD_HANDLE */ +// [Lennart] Same comments as for the other test cases... void saLogSelectionObjectGet_03(void) { SaVersionT log_version = kLogVersion; diff --git a/src/log/apitest/tet_saLogStreamClose.c b/src/log/apitest/tet_saLogStreamClose.c index 6812873..17ed0a5 100644 --- a/src/log/apitest/tet_saLogStreamClose.c +++ b/src/log/apitest/tet_saLogStreamClose.c @@ -17,6 +17,8 @@ #include "logtest.h" +// [Lennart] Why not add description here also +// Also change name of test cases to descriptive names void saLogStreamClose_01(void) { rc = logInitialize(); @@ -38,12 +40,31 @@ done: /* Object to test: logStreamClose() API: * Test: Test the logStreamClose() with finalized handle + [Lennart] You are testing saLogStreamClose() + What you really want to test I assume is calling saLogStreamClose() + with an invalid handle. This is not the same thing as + testing with a finalized handle. When you call logFinalize() not only + the handle is finalized but before the handle is finalized the stream + is closed meaning that any ongoing request will be serviced, the + log file is closed and renamed (with closeing time) etc. + The method you are using is to finalize the handle before + calling saLogStreamClose(). You could have used another + method for testing, for example use a value no handle may have + and such a value is 0. + Also a good enough description of prerequisites could be: + "Test closing an open stream using an invalid handle" unless + it is closing a stream after the handle is finalized you + actually intend to test + * step1:Call logInitialize() * step2:Call logStreamOpen() * step3:Call logFinalize() * step4:Now call the saLogStreamClose() with logStreamHandle * Result: Shall fail with return code SA_AIS_ERR_BAD_HANDLE */ +// [Lennart] A descriptive name is a name describing WHAT the +// function is doing. An example for this function could be: +// saLogStreamClose_with_invalid_handle() void saLogStreamClose_02(void) { rc = logInitialize(); diff --git a/src/log/apitest/tet_saLogStreamOpen_2.c b/src/log/apitest/tet_saLogStreamOpen_2.c index 1c35260..99f068c 100644 --- a/src/log/apitest/tet_saLogStreamOpen_2.c +++ b/src/log/apitest/tet_saLogStreamOpen_2.c @@ -498,6 +498,11 @@ void saLogStreamOpen_2_23(void) * logFileCreateAttributes to NULL pointer and logStreamOpenFlags to ZERO(0) * Result: Shall fail with return code SA_AIS_ERR_INVALID_PARAM */ +// [Lennart] Use descriptive name. See comment for log stream close +// Also, if 2_53 means that this is supposed to be test case 53 in suite 2 +// it it may not be so for very long. I easily happen that someone will add +// new test case in this suite before this test case an then this test is 54. +// Beside that, this information does not have any purpose or signigicance here void saLogStreamOpen_2_53(void) { rc = saLogStreamOpen_2(12345, NULL, NULL, 0, SA_TIME_ONE_SECOND, @@ -1265,6 +1270,7 @@ __attribute__((constructor)) static void saLibraryLifeCycle_constructor(void) test_case_add( 2, saLogWriteLogAsync_20, "saLogWriteLogAsync() after LogFianlize"); + // [Lennart] Spelling error "LogFianlize" test_case_add(2, saLogWriteLogCallbackT_01, "saLogWriteLogCallbackT() SA_DISPATCH_ONE"); test_case_add(2, saLogWriteLogCallbackT_02, diff --git a/src/log/apitest/tet_saLogWriteLogAsync.c b/src/log/apitest/tet_saLogWriteLogAsync.c index 8dfb3cc..c2468c6 100644 --- a/src/log/apitest/tet_saLogWriteLogAsync.c +++ b/src/log/apitest/tet_saLogWriteLogAsync.c @@ -463,6 +463,7 @@ done: * step4:Now call the saLogWriteLogAsync() with logStreamHandle * Result: Shall fail with return code SA_AIS_ERR_BAD_HANDLE */ +// [Lennart] See comment for log stream close void saLogWriteLogAsync_20(void) { SaInvocationT invocation = 0;