pivotal-jbarrett commented on a change in pull request #765:
URL: https://github.com/apache/geode-native/pull/765#discussion_r593473980
##########
File path: cppcache/test/CMakeLists.txt
##########
@@ -46,7 +50,7 @@ add_executable(apache-geode_unittests
LRUQueueTest.cpp
PdxInstanceImplTest.cpp
PdxTypeTest.cpp
- QueueConnectionRequestTest.cpp
+ ${ENABLE_QUEUECONNECTIONREQUESTTEST}
Review comment:
Why conditionally compile it here when the test itself conditionally
adds the disabled flag?
##########
File path: ci/base/base.yml
##########
@@ -96,4 +96,11 @@ builds:
image_family: build-ubuntu-20-04
source_image_family: ubuntu-2004-lts
+ - _: #@ template.replace(new_build("ubuntu-20.04-ipv6"))
+ image_family: build-ubuntu-20-04
+ source_image_family: ubuntu-2004-lts
+ #@yaml/map-key-override
+ params:
+ CMAKE_CONFIGURE_FLAGS: "-DWITH_IPV6=ON"
Review comment:
Why not just enable it for the base. IPv6 shouldn't exclude IPv4
support. Heck for that matter why not make it default to on?
##########
File path: cppcache/integration/test/BasicIPv6Test.cpp
##########
@@ -53,7 +55,11 @@ std::shared_ptr<Region> setupRegion(Cache& cache) {
* Example test using 2 servers and waiting for async tasks to synchronize
using
* furtures.
*/
+#ifdef WITH_IPV6
+TEST(BasicIPv6Test, queryResultForRange) {
+#else
TEST(BasicIPv6Test, DISABLED_queryResultForRange) {
Review comment:
If I recall this test was disable because it relies on very specific
address existing on the host or some specific address formatting. Something
about it was not portable. We should just fix that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
