-
Notifications
You must be signed in to change notification settings - Fork 181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[core] core c test added / core cpp test cleaned up #1423
Conversation
core cpp test cleanup
Initialize/Finalize tests removed from pubsub_test (moved to core_test)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
#include <ecal/cimpl/ecal_core_cimpl.h> | ||
#include <ecal/ecal_defs.h> | ||
|
||
#include <gtest/gtest.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'gtest/gtest.h' file not found [clang-diagnostic-error]
#include <gtest/gtest.h>
^
|
||
#include <gtest/gtest.h> | ||
|
||
TEST(core_c, Core_GetVersion) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable 'TEST' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
TEST(core_c, Core_GetVersion)
^
#include <ecal/ecal_core.h> | ||
#include <ecal/ecal_defs.h> | ||
|
||
#include <gtest/gtest.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'gtest/gtest.h' file not found [clang-diagnostic-error]
#include <gtest/gtest.h>
^
|
||
#include <gtest/gtest.h> | ||
|
||
TEST(core_cpp, Core_GetVersion) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: all parameters should be named in a function [readability-named-parameter]
TEST(core_cpp, Core_GetVersion) | |
TEST(core_cpp /*unused*/, Core_GetVersion /*unused*/) |
EXPECT_EQ(ECAL_VERSION_PATCH, patch); | ||
} | ||
|
||
TEST(core_cpp, Core_InitializeFinalize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: all parameters should be named in a function [readability-named-parameter]
TEST(core_cpp, Core_InitializeFinalize) | |
TEST(core_cpp /*unused*/, Core_InitializeFinalize /*unused*/) |
EXPECT_EQ(1, eCAL::Finalize()); | ||
} | ||
|
||
TEST(core_cpp, Core_MultipleInitializeFinalize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: all parameters should be named in a function [readability-named-parameter]
TEST(core_cpp, Core_MultipleInitializeFinalize) | |
TEST(core_cpp /*unused*/, Core_MultipleInitializeFinalize /*unused*/) |
} | ||
} | ||
|
||
TEST(core_cpp, Core_UnitName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: all parameters should be named in a function [readability-named-parameter]
TEST(core_cpp, Core_UnitName) | |
TEST(core_cpp /*unused*/, Core_UnitName /*unused*/) |
EXPECT_EQ(0, eCAL::Finalize()); | ||
} | ||
|
||
TEST(core_cpp, Core_eCAL_Ok) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: all parameters should be named in a function [readability-named-parameter]
TEST(core_cpp, Core_eCAL_Ok) | |
TEST(core_cpp /*unused*/, Core_eCAL_Ok /*unused*/) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
{ | ||
// check uninitialized eCAL, should not be okay | ||
EXPECT_EQ(0, eCAL_Ok()); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing to change atm, but it's super counter intuitive, that eCAL_Initialize()
returns 0
upon success, but on the other hand eCAL_Ok()
returns 0
upon failure, and 1
upon success (e.g. boolean values casted..).
So one behaves like a C style function while the other one doesn't....
Description
eCAL C API is currently not tested. Therefore a core C API test is added and the existing C++ API test is aligned with that.
Cherry-pick to