Resolve clang-tidy warnings in test files #1
Conversation
|
Code coverage report is ready! 📈
|
|
Code coverage report is ready! 📈
|
|
Code coverage report is ready! 📈
|
|
Code coverage report is ready! 📈
|
|
Code coverage report is ready! 📈
|
|
Code coverage report is ready! 📈
|
|
Code coverage report is ready! 📈
|
|
Code coverage report is ready! 📈
|
|
Code coverage report is ready! 📈
|
|
Code coverage report is ready! 📈
|
|
Code coverage report is ready! 📈
|
|
Code coverage report is ready! 📈
|
| void setMockTransportClient( | ||
| const std::shared_ptr<uprotocol::test::UTransportMock>& client) { | ||
| mockTransportClient_ = client; | ||
| } | ||
| void setMockTransportServer( | ||
| const std::shared_ptr<uprotocol::test::UTransportMock>& server) { | ||
| mockTransportClient_ = server; | ||
| } | ||
| void setClientUUri(const uprotocol::v1::UUri& uuri) { client_uuri = uuri; } | ||
| void setServerUUri(const uprotocol::v1::UUri& uuri) { server_uuri = uuri; } | ||
| void setSubcriptionUUri(const uprotocol::v1::UUri& uuri) { | ||
| subcription_uuri = uuri; | ||
| } |
There was a problem hiding this comment.
That´s a point. I was no quiet sure if the setter might be needed later. Just a matter if habit...
@lammjo shall I remove all setters from this PR?
There was a problem hiding this comment.
Yes, please :) I would favor YAGNI here.
|
|
||
| public: | ||
| ~ConsumerTest() override = default; |
There was a problem hiding this comment.
Why move the destructor down here? Did the linter complain about a protected destructor?
There was a problem hiding this comment.
Yes! Destructor of '...' is protected and virtual (clang-tidycppcoreguidelines-virtual-class-destructor)
@lammjo is there any other way to mitigate this problem?
| auto subscribe_request_ttl = std::chrono::milliseconds(1000); | ||
| TEST_F(ConsumerTest, ConstructorTestSuccess) { // NOLINT | ||
| constexpr int REQUEST_TTL_TIME = 0x8000; | ||
| auto subcription_callback = someCallBack; |
| // namespace { | ||
| // using namespace uprotocol::datamodel::builder; |
| uprotocol::v1::UUri uri; | ||
| static v1::UUri methodUri(const std::string& auth = "TestAuth", | ||
| uint16_t ue_id = 0x8000, | ||
| uint16_t ue_instance = 1, // NOLINT |
|
Code coverage report is ready! 📈
|
lammjo
left a comment
lammjo
left a comment
There was a problem hiding this comment.
Thanks, Lennart! Great work! After you've addressed my comments (if applicable) feel free to open a PR into up-cpp/main.
|
|
||
| EXPECT_TRUE(invoke_future.valid()); | ||
| auto is_ready = invoke_future.wait_for(0ms); | ||
| auto is_ready = invoke_future.wait_for(std::chrono::milliseconds(0)); |
There was a problem hiding this comment.
I suggest turning 0ms also into a constant just to be consistent with 10ms and 150ms.
| TEST(UuidBuilderTest, CustomTimeAndRandomSource) { | ||
| TEST(UuidBuilderTest, CustomTimeAndRandomSource) { // NOLINT | ||
| constexpr std::time_t FIXED_TIME_T = 1623456789; | ||
| constexpr uint64_t FIXED_RANDOM_T = 0x1234567890ABCDEF; |
There was a problem hiding this comment.
| constexpr uint64_t FIXED_RANDOM_T = 0x1234567890ABCDEF; | |
| constexpr uint64_t FIXED_RANDOM_UINT = 0x1234567890ABCDEF; |
| // TODO(unknown) | ||
| TEST_F(TestFixture, SomeTestName) {} // NOLINT |
There was a problem hiding this comment.
Oh noez... 🙈 This is not your job right now, but we should keep in mind to implement all missing tests at some point.
| } // namespace | ||
| } // namespace uprotocol::v1 |
|
Code coverage report is ready! 📈
|
|
Code coverage report is ready! 📈
|
|
Code coverage report is ready! 📈
|
No description provided.