Savonitar commented on code in PR #28857:
URL: https://github.com/apache/flink/pull/28857#discussion_r3719496493


##########
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)
+    void testFetcherThreadCarriesJobIdInMdcAndThreadName() throws Exception {
+        final JobID jobId = new JobID();
+        final String jobName = "my-test-job";
+
+        final FetcherThreadInfo fetcherThread =
+                captureFetcherThread(new JobInfoImpl(jobId, jobName));
+
+        assertThat(fetcherThread.mdcJobId).isEqualTo(jobId.toHexString());
+        assertThat(fetcherThread.threadName)
+                .startsWith(SplitFetcherManager.THREAD_NAME_PREFIX)
+                .endsWith(" (job: " + jobName + " / " + jobId.toHexString() + 
")");
+    }
+
+    @Test
+    @Timeout(value = 30000, unit = TimeUnit.MILLISECONDS)
+    void testFetcherThreadWithoutJobInfoKeepsHistoricalNameAndNoJobIdInMdc() 
throws Exception {
+        final FetcherThreadInfo fetcherThread = captureFetcherThread(null);
+
+        // Fetcher threads are named after the thread creating the manager, 
i.e. this test thread.
+        assertThat(fetcherThread.threadName)
+                .isEqualTo(
+                        SplitFetcherManager.THREAD_NAME_PREFIX + 
Thread.currentThread().getName());
+        assertThat(fetcherThread.mdcJobId).isNull();
+    }
+
+    @Test
+    @Timeout(value = 30000, unit = TimeUnit.MILLISECONDS)
+    void testFetcherThreadNameTruncatesLongJobName() throws Exception {
+        final JobID jobId = new JobID();
+        final String longJobName = "abcdefghijklmnopqrstuvwxyz0123456789-xyz";
+        
assertThat(longJobName.length()).isGreaterThan(MdcUtils.MAX_JOB_NAME_IN_THREAD_NAME);
+
+        final FetcherThreadInfo fetcherThread =
+                captureFetcherThread(new JobInfoImpl(jobId, longJobName));
+
+        final String expectedSuffix =
+                " (job: "
+                        + longJobName.substring(0, 
MdcUtils.MAX_JOB_NAME_IN_THREAD_NAME)
+                        + "... / "
+                        + jobId.toHexString()
+                        + ")";
+        assertThat(fetcherThread.threadName).endsWith(expectedSuffix);
+    }
+
+    @Test
+    @Timeout(value = 30000, unit = TimeUnit.MILLISECONDS)
+    void testFetcherThreadNameOmitsEmptyOrNullJobName() throws Exception {
+        final JobID jobId = new JobID();
+        final String suffixWithoutJobName = " (job: " + jobId.toHexString() + 
")";
+
+        final FetcherThreadInfo emptyNameThread = captureFetcherThread(new 
JobInfoImpl(jobId, ""));
+        assertThat(emptyNameThread.threadName).endsWith(suffixWithoutJobName);
+        assertThat(emptyNameThread.mdcJobId).isEqualTo(jobId.toHexString());

Review Comment:
   Blank and null job names are now covered in the MdcUtilsTest.
   I parameterised tests, but some tests left out side of parametersied case 
intentionally, because they test different things.



-- 
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]

Reply via email to