Savonitar commented on code in PR #28857:
URL: https://github.com/apache/flink/pull/28857#discussion_r3713261848
##########
flink-connectors/flink-connector-base/src/test/java/org/apache/flink/connector/base/source/reader/fetcher/SplitFetcherManagerTest.java:
##########
@@ -75,6 +83,80 @@ void testCloseFetcherWithException() throws Exception {
.hasRootCauseMessage("Artificial exception on closing the
split reader.");
}
+ @Test
+ @Timeout(value = 30000, unit = TimeUnit.MILLISECONDS)
Review Comment:
I know different committers prefer different options here , e.g. some start
with a minimal timeout and adjust when it flakes, and that's what I did
initially (also this class already uses Timeout with 30 in another test). But
on the "common guidance" question: we actually have one "Avoid timeouts in
JUnit tests" recommends no local timeouts at all, relying on the CI watchdog.
And this class already went through the adjustment once in FLINK-39919 it
was raised from 30s to 60s after flakyness on CI. So I went with that proven
pattern: dropped `@Timeout` from both new tests and raised the in-body wait to
60s , the same value FLINK-39919 validated for this class.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
