Skip to content

Resolve clang-tidy warnings in test files #1

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.

Sign up for GitHub

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

Open
lennartbecker-d wants to merge 41 commits into
base: main
Choose a base branch
from
Open

Resolve clang-tidy warnings in test files #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
6ccd414
Fixed linter warnings in first test files
lennartbecker-d Apr 7, 2025
321d086
Fixed some namings
lennartbecker-d Apr 7, 2025
5ddc824
Solve linting warnings in communication
lennartbecker-d Apr 8, 2025
7945d9e
Solve more linter warnings
lennartbecker-d Apr 8, 2025
ceefdcf
Solved linting warnings. Some tests unfortunately fail
lennartbecker-d Apr 8, 2025
59c8a63
Fixed 1 test fail
lennartbecker-d Apr 8, 2025
d6a582e
Solved failing tests. Next step is to fix linter warnings on RpcServe…
lennartbecker-d Apr 8, 2025
43e2610
Finish day 2025_04_08. Continue with RpcServerTest.cpp
lennartbecker-d Apr 8, 2025
7bc68ef
Solved few warnings
lennartbecker-d Apr 9, 2025
9340df2
Solved linter warnings in test/communication/
lennartbecker-d Apr 9, 2025
e83196a
Continue fixing linter warnings in datamodel
lennartbecker-d Apr 9, 2025
be386c8
Finished UMessageValidatorTest
lennartbecker-d Apr 9, 2025
2c71e63
Solve linter warnings
lennartbecker-d Apr 10, 2025
0743fc4
Update clang-tidy.sh
lennartbecker-d Apr 10, 2025
70530b4
Solved building error
lennartbecker-d Apr 10, 2025
12a7bfd
Added NOLINT behind makros of TEST_F, EXPECT_THROW and EXPECT_NO_THROW
lennartbecker-d Apr 10, 2025
1e4715e
Add NOLINT behin ASSERT_THROW and ASSERT_NO_THROW
lennartbecker-d Apr 10, 2025
b882756
Mostly finished datamodel/
lennartbecker-d Apr 10, 2025
bad80ac
Finished transport/
lennartbecker-d Apr 10, 2025
2640935
Added NOLINT to rest of test files (TEST_F, EXPECT_THROW, EXPECT_NO_T…
lennartbecker-d Apr 10, 2025
b24249f
Continue solving warnings
lennartbecker-d Apr 10, 2025
dc2759f
Solved more linter warnings. Mostly finished.
lennartbecker-d Apr 10, 2025
918bd11
Solved more issues
lennartbecker-d Apr 11, 2025
c9f1eb0
Before updating UTransportMock.h to private member variables
lennartbecker-d Apr 11, 2025
941621d
Updated UTransportMock.h to private member. No compiling, nor test er…
lennartbecker-d Apr 11, 2025
87dabfe
12 warnings missing
lennartbecker-d Apr 11, 2025
0756b63
Before .clang-format
lennartbecker-d Apr 11, 2025
4c7b781
Adapted NOLINT Comments to formated code
lennartbecker-d Apr 11, 2025
6e33550
Adapted NOLINT Comments to formated code. Part 2
lennartbecker-d Apr 11, 2025
b564410
Adapted NOLINT Comments to formated code. Part 3
lennartbecker-d Apr 11, 2025
fc82f61
Dummy fix NOLINT for complicated warnings
lennartbecker-d Apr 11, 2025
da77a78
Fixed auto worker_handles warning in ParallelAccessSingleClientTest
lennartbecker-d Apr 11, 2025
f5c1387
Solved static issue in UMessageBuilderTest
lennartbecker-d Apr 11, 2025
9eb4686
Finished linter warnings
lennartbecker-d Apr 11, 2025
29d0d9d
Formated code
lennartbecker-d Apr 11, 2025
04c3352
Update clang-tidy.sh to check src/, inc/ and test/
lennartbecker-d Apr 11, 2025
963bed5
remove commented code for namespace. Fix typo
lennartbecker-d Apr 15, 2025
73c507d
Remove setter-Methods
lennartbecker-d Apr 15, 2025
f9d4cf2
Completed with review comments
lennartbecker-d Apr 16, 2025
fa923ec
formatted code
lennartbecker-d Apr 16, 2025
fe88182
Added linting also for src and include
lennartbecker-d Apr 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lint/clang-tidy.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ if [ -z "$target_source" ]; then
shopt -s globstar

pushd "$PROJECT_ROOT" > /dev/null
for f in include/**/*.h src/**/*.cpp; do # test/coverage/**/*.cpp test/extra/**/*.cpp test/include/**/*.h; do
for f in include/**/*.h src/**/*.cpp test/coverage/**/*.cpp test/extra/**/*.cpp test/include/**/*.h; do
if [[ ! ("$f" =~ "build/") ]]; then
echo
echo "Checking file '$f'"
Expand Down
1 change: 0 additions & 1 deletion src/communication/RpcClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,6 @@ namespace detail {

using uprotocol::v1::UCode;
using uprotocol::v1::UStatus;
// using namespace std::chrono_literals;
using ListenHandle = uprotocol::transport::UTransport::ListenHandle;

auto PendingRequest::operator>(const PendingRequest& other) const {
Expand Down
1 change: 0 additions & 1 deletion src/datamodel/validator/UMessage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
namespace uprotocol::datamodel::validator::message {

using uprotocol::v1::UPRIORITY_CS4;
// using uprotocol::datamodel::validator;

std::string_view message(Reason reason) {
switch (reason) {
Expand Down
139 changes: 80 additions & 59 deletions test/coverage/client/usubscription/v3/ConsumerTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include <up-cpp/client/usubscription/v3/Consumer.h>
#include <up-cpp/communication/NotificationSource.h>

#include <random>
#include <string>

#include "UTransportMock.h"
Expand All @@ -28,19 +27,37 @@ void someCallBack(const uprotocol::v1::UMessage& message) {
}

class ConsumerTest : public testing::Test {
protected:
// Run once per TEST_F.
// Used to set up clean environments per test.
private:
std::shared_ptr<uprotocol::test::UTransportMock> mockTransportClient_;
std::shared_ptr<uprotocol::test::UTransportMock> mockTransportServer_;
uprotocol::v1::UUri client_uuri;
uprotocol::v1::UUri server_uuri;
uprotocol::v1::UUri subcription_uuri;
uprotocol::v1::UUri subscription_uuri;

protected:
// Run once per TEST_F.
// Used to set up clean environments per test.

std::shared_ptr<uprotocol::test::UTransportMock> getMockTransportClient()
const {
return mockTransportClient_;
}
std::shared_ptr<uprotocol::test::UTransportMock> getMockTransportServer()
const {
return mockTransportServer_;
}
uprotocol::v1::UUri& getClientUUri() { return client_uuri; }
const uprotocol::v1::UUri& getServerUUri() const { return server_uuri; }
const uprotocol::v1::UUri& getSubscriptionUUri() const {
return subscription_uuri;
}

void SetUp() override {
constexpr uint32_t TEST_UE_ID = 0x18000;
constexpr uint32_t DEFAULT_RESOURCE_ID = 0x8000;
// Create a generic transport uri
client_uuri.set_authority_name("random_string");
client_uuri.set_ue_id(0x18000);
client_uuri.set_ue_id(TEST_UE_ID);
client_uuri.set_ue_version_major(3);
client_uuri.set_resource_id(0);

Expand All @@ -58,17 +75,16 @@ class ConsumerTest : public testing::Test {
std::make_shared<uprotocol::test::UTransportMock>(server_uuri);

// Create a generic subscription uri
subcription_uuri.set_authority_name("10.0.0.2");
subcription_uuri.set_ue_id(0x18000);
subcription_uuri.set_ue_version_major(3);
subcription_uuri.set_resource_id(0x8000);
subscription_uuri.set_authority_name("10.0.0.2");
subscription_uuri.set_ue_id(TEST_UE_ID);
subscription_uuri.set_ue_version_major(3);
subscription_uuri.set_resource_id(DEFAULT_RESOURCE_ID);
};
void TearDown() override {}

// Run once per execution of the test application.
// Used for setup of all tests. Has access to this instance.
ConsumerTest() = default;
~ConsumerTest() = default;

void buildDefaultSourceURI();
void buildValidNotificationURI();
Expand All @@ -78,125 +94,130 @@ class ConsumerTest : public testing::Test {
// Used only for global setup outside of tests.
static void SetUpTestSuite() {}
static void TearDownTestSuite() {}

public:
~ConsumerTest() override = default;
Comment on lines +97 to +99
Copy link

@lammjo lammjo Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why move the destructor down here? Did the linter complain about a protected destructor?

Copy link
Owner Author

@lennartbecker-d lennartbecker-d Apr 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Destructor of '...' is protected and virtual (clang-tidycppcoreguidelines-virtual-class-destructor)
@lammjo is there any other way to mitigate this problem?

Copy link

@lammjo lammjo May 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation :)

};

// Negative test case with no source filter
TEST_F(ConsumerTest, ConstructorTestSuccess) {
auto subcriptionCallback = someCallBack;
auto subscribe_request_ttl = std::chrono::milliseconds(1000);
TEST_F(ConsumerTest, ConstructorTestSuccess) { // NOLINT
constexpr int REQUEST_TTL_TIME = 0x8000;
auto subscription_callback = someCallBack;
auto subscribe_request_ttl = std::chrono::milliseconds(REQUEST_TTL_TIME);
auto priority = uprotocol::v1::UPriority::UPRIORITY_CS4;

auto options = uprotocol::client::usubscription::v3::ConsumerOptions();

auto consumerOrSatus =
auto consumer_or_status =
uprotocol::client::usubscription::v3::Consumer::create(
mockTransportClient_, subcription_uuri,
std::move(subcriptionCallback), priority,
std::move(subscribe_request_ttl), options);
getMockTransportClient(), getSubscriptionUUri(),
subscription_callback, priority, subscribe_request_ttl, options);

// Ensure that the consumer creation was successful
ASSERT_TRUE(consumerOrSatus.has_value());
ASSERT_TRUE(consumer_or_status.has_value());

// Obtain a pointer to the created consumer instance
auto& consumerPtr = consumerOrSatus.value();
const auto& consumer_ptr = consumer_or_status.value();

// Verify that the consumer pointer is not null, indicating successful
// creation
ASSERT_NE(consumerPtr, nullptr);
ASSERT_NE(consumer_ptr, nullptr);
}

TEST_F(ConsumerTest, SubscribeTestSuccess) {
auto subcriptionCallback = someCallBack;
auto subscribe_request_ttl = std::chrono::milliseconds(1000);
TEST_F(ConsumerTest, SubscribeTestSuccess) { // NOLINT
constexpr uint32_t DEFAULT_RESOURCE_ID = 0x8000;
constexpr int REQUEST_TTL_TIME = 0x8000;
auto subscription_callback = someCallBack;
auto subscribe_request_ttl = std::chrono::milliseconds(REQUEST_TTL_TIME);
auto priority = uprotocol::v1::UPriority::UPRIORITY_CS4;

auto options = uprotocol::client::usubscription::v3::ConsumerOptions();

auto consumerOrSatus =
auto consumer_or_status =
uprotocol::client::usubscription::v3::Consumer::create(
mockTransportClient_, subcription_uuri,
std::move(subcriptionCallback), priority,
std::move(subscribe_request_ttl), options);
getMockTransportClient(), getSubscriptionUUri(),
subscription_callback, priority, subscribe_request_ttl, options);

// Ensure that the consumer creation was successful
ASSERT_TRUE(consumerOrSatus.has_value());
ASSERT_TRUE(consumer_or_status.has_value());

// Obtain a pointer to the created consumer instance
auto& consumerPtr = consumerOrSatus.value();
const auto& consumer_ptr = consumer_or_status.value();

// Verify that the consumer pointer is not null, indicating successful
// creation
ASSERT_NE(consumerPtr, nullptr);
ASSERT_NE(consumer_ptr, nullptr);

// Create notification source sink uri to match resource id of sink
auto notification_uuri = server_uuri;
notification_uuri.set_resource_id(0x8000);
auto notification_uuri = getServerUUri();
notification_uuri.set_resource_id(DEFAULT_RESOURCE_ID);

// set format UPAYLOAD_FORMAT_PROTOBUF_WRAPPED_IN_ANY
auto format =
uprotocol::v1::UPayloadFormat::UPAYLOAD_FORMAT_PROTOBUF_WRAPPED_IN_ANY;

auto norificationSource = uprotocol::communication::NotificationSource(
mockTransportServer_, std::move(notification_uuri),
std::move(client_uuri), format);
auto notification_source = uprotocol::communication::NotificationSource(
getMockTransportServer(), std::move(notification_uuri),
std::move(getClientUUri()), format);
// Build payload
const std::string data = "test";
auto payload = uprotocol::datamodel::builder::Payload(data, format);

norificationSource.notify(std::move(payload));
notification_source.notify(std::move(payload));

// Check send count
EXPECT_TRUE(mockTransportServer_->send_count_ == 1);
EXPECT_TRUE(mockTransportClient_->send_count_ == 1);
EXPECT_TRUE(getMockTransportServer()->getSendCount() == 1);
EXPECT_TRUE(getMockTransportClient()->getSendCount() == 1);
}

TEST_F(ConsumerTest, UnsubscribeTestSuccess) {
auto subcriptionCallback = someCallBack;
auto subscribe_request_ttl = std::chrono::milliseconds(1000);
TEST_F(ConsumerTest, UnsubscribeTestSuccess) { // NOLINT
constexpr uint32_t DEFAULT_RESOURCE_ID = 0x8000;
constexpr int REQUEST_TTL_TIME = 0x8000;
auto subscription_callback = someCallBack;
auto subscribe_request_ttl = std::chrono::milliseconds(REQUEST_TTL_TIME);
auto priority = uprotocol::v1::UPriority::UPRIORITY_CS4;

auto options = uprotocol::client::usubscription::v3::ConsumerOptions();

auto consumerOrSatus =
auto consumer_or_status =
uprotocol::client::usubscription::v3::Consumer::create(
mockTransportClient_, subcription_uuri,
std::move(subcriptionCallback), priority,
std::move(subscribe_request_ttl), options);
getMockTransportClient(), getSubscriptionUUri(),
subscription_callback, priority, subscribe_request_ttl, options);

// Ensure that the consumer creation was successful
ASSERT_TRUE(consumerOrSatus.has_value());
ASSERT_TRUE(consumer_or_status.has_value());

// Obtain a pointer to the created consumer instance
auto& consumerPtr = consumerOrSatus.value();
const auto& consumer_ptr = consumer_or_status.value();

// Verify that the consumer pointer is not null, indicating successful
// creation
ASSERT_NE(consumerPtr, nullptr);
ASSERT_NE(consumer_ptr, nullptr);

// Create notification source sink uri to match resource id of sink
auto notification_uuri = server_uuri;
notification_uuri.set_resource_id(0x8000);
auto notification_uuri = getServerUUri();
notification_uuri.set_resource_id(DEFAULT_RESOURCE_ID);

// set format UPAYLOAD_FORMAT_PROTOBUF_WRAPPED_IN_ANY
auto format =
uprotocol::v1::UPayloadFormat::UPAYLOAD_FORMAT_PROTOBUF_WRAPPED_IN_ANY;

auto norificationSource = uprotocol::communication::NotificationSource(
mockTransportServer_, std::move(notification_uuri),
std::move(client_uuri), format);
auto notification_source = uprotocol::communication::NotificationSource(
getMockTransportServer(), std::move(notification_uuri),
std::move(getClientUUri()), format);
// Build payload
const std::string data = "test";
auto payload = uprotocol::datamodel::builder::Payload(data, format);

norificationSource.notify(std::move(payload));
notification_source.notify(std::move(payload));

// Check send count
EXPECT_TRUE(mockTransportServer_->send_count_ == 1);
EXPECT_TRUE(mockTransportClient_->send_count_ == 1);
EXPECT_TRUE(getMockTransportServer()->getSendCount() == 1);
EXPECT_TRUE(getMockTransportClient()->getSendCount() == 1);

consumerPtr->unsubscribe(priority, subscribe_request_ttl);
consumer_ptr->unsubscribe(priority, subscribe_request_ttl);

EXPECT_TRUE(mockTransportClient_->send_count_ == 2);
EXPECT_TRUE(getMockTransportClient()->getSendCount() == 2);
}

} // namespace
Loading