[GitHub] [arrow] lidavidm commented on a diff in pull request #35090: GH-35089: [CI][C++][Flight] Test failures in macos release verification nightlies

["lidavidm (via GitHub)" <gi...@apache.org>]

lidavidm commented on code in PR #35090:
URL: https://github.com/apache/arrow/pull/35090#discussion_r1164761075


##########
cpp/src/arrow/flight/flight_test.cc:
##########
@@ -1023,7 +1023,11 @@ TEST_F(TestFlightClient, TimeoutFires) {
   Status status = client->GetFlightInfo(options, FlightDescriptor{}).status();
   auto end = std::chrono::system_clock::now();
 #ifdef ARROW_WITH_TIMING_TESTS
+#ifdef __APPLE__
+  EXPECT_LE(end - start, std::chrono::milliseconds{1000});
+#else
   EXPECT_LE(end - start, std::chrono::milliseconds{400});
+#endif

Review Comment:
   Maybe just raise the timeout universally?



##########
cpp/src/arrow/flight/test_definitions.cc:
##########
@@ -108,7 +108,15 @@ void ConnectivityTest::TestBrokenConnection() {
   ASSERT_OK(server->Shutdown());
   ASSERT_OK(server->Wait());
 
+#if defined(__APPLE__) && defined(__x86_64__)
+  // for some reason, on macos-x86-64 the broken connection here returns
+  // a different error and we get UnknownError instead of IOError.
+  // Since we're *testing* the broken connection, we can just check for
+  // the other error in this case.
+  ASSERT_RAISES(UnknownError, client->GetFlightInfo(FlightDescriptor::Command("")));

Review Comment:
   It seems inconsistent - maybe just get the status, assert that it fails, and then `ASSERT_THAT(status.code(), ::testing::AnyOf())`?



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org