You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "isapego (via GitHub)" <gi...@apache.org> on 2025/06/17 09:05:20 UTC
[PR] IGNITE-25688 Fix names of C++ parametrized tests [ignite-3]
isapego opened a new pull request, #6058:
URL: https://github.com/apache/ignite-3/pull/6058
https://issues.apache.org/jira/browse/IGNITE-25688
Some of our tests parameters have malformed unicode in them, and these parameters used by default as the part of the test name, which may result in malformed test name, which can not be parsed by the TC. This PR fix this by using simple indexing instead in such parametrized tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25688 Fix names of C++ parametrized tests [ignite-3]
Posted by "isapego (via GitHub)" <gi...@apache.org>.
isapego merged PR #6058:
URL: https://github.com/apache/ignite-3/pull/6058
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25688 Fix names of C++ parametrized tests [ignite-3]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #6058:
URL: https://github.com/apache/ignite-3/pull/6058#discussion_r2151748100
##########
modules/platforms/cpp/ignite/client/table/qualified_name_test.cpp:
##########
@@ -24,6 +24,14 @@
using namespace ignite;
using namespace detail;
+namespace {
Review Comment:
The PrintTestIndex helper function is duplicated across test files. Consider extracting it to a shared utility header to improve maintainability.
##########
modules/platforms/cpp/ignite/client/detail/table/name_utils_test.cpp:
##########
@@ -24,6 +24,15 @@
using namespace ignite;
using namespace detail;
+namespace {
Review Comment:
The PrintTestIndex function is repeated here as well. Extracting it into a common header may help reduce duplication and ease future maintenance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25688 Fix names of C++ parametrized tests [ignite-3]
Posted by "isapego (via GitHub)" <gi...@apache.org>.
isapego commented on code in PR #6058:
URL: https://github.com/apache/ignite-3/pull/6058#discussion_r2151755177
##########
modules/platforms/cpp/ignite/client/table/qualified_name_test.cpp:
##########
@@ -24,6 +24,14 @@
using namespace ignite;
using namespace detail;
+namespace {
Review Comment:
WIth current approach all unit tests are independent.
##########
modules/platforms/cpp/ignite/client/detail/table/name_utils_test.cpp:
##########
@@ -24,6 +24,15 @@
using namespace ignite;
using namespace detail;
+namespace {
Review Comment:
WIth current approach all unit tests are independent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25688 Fix names of C++ parametrized tests [ignite-3]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #6058:
URL: https://github.com/apache/ignite-3/pull/6058#discussion_r2151740350
##########
modules/platforms/cpp/ignite/client/table/qualified_name_test.cpp:
##########
@@ -24,6 +24,10 @@
using namespace ignite;
using namespace detail;
+template<typename T>
+std::string PrintTestIndex(const testing::TestParamInfo<typename T::ParamType>& info) {
+ return "_" + std::to_string(info.index);
+}
Review Comment:
[nitpick] Consider placing `PrintTestIndex` in an anonymous namespace or marking it `static` to prevent potential ODR/linkage conflicts.
```suggestion
namespace {
template<typename T>
std::string PrintTestIndex(const testing::TestParamInfo<typename T::ParamType>& info) {
return "_" + std::to_string(info.index);
}
} // namespace
```
##########
modules/platforms/cpp/ignite/client/detail/table/name_utils_test.cpp:
##########
@@ -24,6 +24,11 @@
using namespace ignite;
using namespace detail;
+template<typename T>
+std::string PrintTestIndex(const testing::TestParamInfo<typename T::ParamType>& info) {
+ return "_" + std::to_string(info.index);
+}
Review Comment:
[nitpick] Consider placing `PrintTestIndex` in an anonymous namespace or marking it `static` to prevent potential ODR/linkage conflicts.
```suggestion
namespace {
template<typename T>
std::string PrintTestIndex(const testing::TestParamInfo<typename T::ParamType>& info) {
return "_" + std::to_string(info.index);
}
} // namespace
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org