This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch branch-1.1
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.1 by this push:
new 241a7f06a1 [#9554] fix(test): Adjust `TestJobManager` to fix flaky
test in CI pipeline (#9577)
241a7f06a1 is described below
commit 241a7f06a1bf1f2a915d4f48137b805a0be64a76
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Mon Dec 29 19:26:51 2025 +0800
[#9554] fix(test): Adjust `TestJobManager` to fix flaky test in CI pipeline
(#9577)
### What changes were proposed in this pull request?
This pull request refactors the `TestJobManager` test class to improve
test isolation and reliability by converting static fields and lifecycle
methods to instance-level, ensuring a fresh test environment for each
test case. Additionally, it addresses potential interference from
background threads and updates utility methods for consistency.
Test lifecycle and isolation improvements:
* Converted all static fields (e.g., `jobManager`, `entityStore`,
`config`, etc.) to instance fields, and changed `@BeforeAll`/`@AfterAll`
methods to `@BeforeEach`/`@AfterEach` to ensure each test runs with a
clean setup and teardown.
* Updated the test staging directory to use a unique `UUID` for each
test run, preventing collisions between tests.
Background thread management:
* Explicitly shuts down background scheduler executors
(`cleanUpExecutor` and `statusPullExecutor`) in the `setUp` method to
prevent interference with test execution and resource leaks.
Utility method updates:
* Changed helper methods (`newShellJobTemplateEntity`,
`newSparkJobTemplateEntity`, and `newJobEntity`) from static to instance
methods for consistency with the new instance-based test structure.
[[1]](diffhunk://#diff-de6e5a136a3bf0d4be93b1b267b421b38e404c4f70bfa798858251c5f24b9687L813-R818)
[[2]](diffhunk://#diff-de6e5a136a3bf0d4be93b1b267b421b38e404c4f70bfa798858251c5f24b9687L832-R837)
[[3]](diffhunk://#diff-de6e5a136a3bf0d4be93b1b267b421b38e404c4f70bfa798858251c5f24b9687L852-R857)
Cleanup and mocking:
* Removed the redundant `@AfterEach reset()` method and ensured proper
cleanup of resources and mocks in the new `tearDown` method.
### Why are the changes needed?
Fix: #9554
### Does this PR introduce _any_ user-facing change?
N/A.
### How was this patch tested?
Test locally.
Co-authored-by: Mini Yu <[email protected]>
---
.../java/org/apache/gravitino/job/JobManager.java | 4 +-
.../org/apache/gravitino/job/TestJobManager.java | 84 ++++++++++++++--------
2 files changed, 57 insertions(+), 31 deletions(-)
diff --git a/core/src/main/java/org/apache/gravitino/job/JobManager.java
b/core/src/main/java/org/apache/gravitino/job/JobManager.java
index b6f255ae67..49d3a900b9 100644
--- a/core/src/main/java/org/apache/gravitino/job/JobManager.java
+++ b/core/src/main/java/org/apache/gravitino/job/JobManager.java
@@ -99,9 +99,9 @@ public class JobManager implements JobOperationDispatcher {
private final long jobStagingDirKeepTimeInMs;
- private final ScheduledExecutorService cleanUpExecutor;
+ @VisibleForTesting final ScheduledExecutorService cleanUpExecutor;
- private final ScheduledExecutorService statusPullExecutor;
+ @VisibleForTesting final ScheduledExecutorService statusPullExecutor;
public JobManager(Config config, EntityStore entityStore, IdGenerator
idGenerator) {
this(config, entityStore, idGenerator, JobExecutorFactory.create(config));
diff --git a/core/src/test/java/org/apache/gravitino/job/TestJobManager.java
b/core/src/test/java/org/apache/gravitino/job/TestJobManager.java
index 674e68cf9a..4e669bec79 100644
--- a/core/src/test/java/org/apache/gravitino/job/TestJobManager.java
+++ b/core/src/test/java/org/apache/gravitino/job/TestJobManager.java
@@ -38,6 +38,8 @@ import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.Random;
+import java.util.UUID;
+import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.ArrayUtils;
@@ -70,39 +72,37 @@ import org.apache.gravitino.storage.RandomIdGenerator;
import org.apache.gravitino.utils.NameIdentifierUtil;
import org.apache.gravitino.utils.NamespaceUtil;
import org.awaitility.Awaitility;
-import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
public class TestJobManager {
- private static JobManager jobManager;
+ private JobManager jobManager;
- private static EntityStore entityStore;
+ private EntityStore entityStore;
- private static Config config;
+ private Config config;
- private static String testStagingDir;
+ private String testStagingDir;
- private static String metalake = "test_metalake";
+ private String metalake = "test_metalake";
- private static NameIdentifier metalakeIdent = NameIdentifier.of(metalake);
+ private NameIdentifier metalakeIdent = NameIdentifier.of(metalake);
- private static MockedStatic<MetalakeManager> mockedMetalake;
+ private MockedStatic<MetalakeManager> mockedMetalake;
- private static JobExecutor jobExecutor;
+ private JobExecutor jobExecutor;
- private static IdGenerator idGenerator;
+ private IdGenerator idGenerator;
- @BeforeAll
- public static void setUp() throws IllegalAccessException {
+ @BeforeEach
+ public void setUp() throws IllegalAccessException {
config = new Config(false) {};
- Random rand = new Random();
- testStagingDir = "test_staging_dir_" + rand.nextInt(100);
+ testStagingDir = "test_staging_dir_" + UUID.randomUUID().toString();
config.set(Configs.JOB_STAGING_DIR, testStagingDir);
config.set(Configs.JOB_STAGING_DIR_KEEP_TIME_IN_MS, 1000L);
@@ -114,23 +114,49 @@ public class TestJobManager {
JobManager jm = new JobManager(config, entityStore, idGenerator,
jobExecutor);
jobManager = Mockito.spy(jm);
+ // Stop the background schedulers to prevent interference with tests
+ ScheduledExecutorService cleanUpExecutor = jobManager.cleanUpExecutor;
+ if (cleanUpExecutor != null) {
+ cleanUpExecutor.shutdownNow();
+ try {
+ if (!cleanUpExecutor.awaitTermination(100, TimeUnit.MILLISECONDS)) {
+ cleanUpExecutor.shutdownNow();
+ }
+ } catch (InterruptedException e) {
+ cleanUpExecutor.shutdownNow();
+ Thread.currentThread().interrupt();
+ }
+ }
+
+ ScheduledExecutorService statusPullExecutor =
jobManager.statusPullExecutor;
+ if (statusPullExecutor != null) {
+ statusPullExecutor.shutdown();
+ try {
+ if (!statusPullExecutor.awaitTermination(100, TimeUnit.MILLISECONDS)) {
+ statusPullExecutor.shutdownNow();
+ }
+ } catch (InterruptedException e) {
+ statusPullExecutor.shutdownNow();
+ Thread.currentThread().interrupt();
+ }
+ }
+
mockedMetalake = mockStatic(MetalakeManager.class);
}
- @AfterAll
- public static void tearDown() throws Exception {
+ @AfterEach
+ public void tearDown() throws Exception {
+ // Reset mocks to ensure test isolation
+ if (mockedMetalake != null) {
+ mockedMetalake.reset();
+ }
+ Mockito.reset(entityStore, jobManager);
// Clean up resources if necessary
jobManager.close();
FileUtils.deleteDirectory(new File(testStagingDir));
- mockedMetalake.close();
- }
-
- @AfterEach
- public void reset() {
- // Reset the mocked static methods after each test
- mockedMetalake.reset();
- Mockito.reset(entityStore);
- Mockito.reset(jobManager);
+ if (mockedMetalake != null) {
+ mockedMetalake.close();
+ }
}
@Test
@@ -810,7 +836,7 @@ public class TestJobManager {
oldJobTemplateEntity.nameIdentifier(), oldJobTemplateEntity,
invalidChange));
}
- private static JobTemplateEntity newShellJobTemplateEntity(String name,
String comment) {
+ private JobTemplateEntity newShellJobTemplateEntity(String name, String
comment) {
ShellJobTemplate shellJobTemplate =
ShellJobTemplate.builder()
.withName(name)
@@ -829,7 +855,7 @@ public class TestJobManager {
.build();
}
- private static JobTemplateEntity newSparkJobTemplateEntity(String name,
String comment) {
+ private JobTemplateEntity newSparkJobTemplateEntity(String name, String
comment) {
SparkJobTemplate sparkJobTemplate =
SparkJobTemplate.builder()
.withName(name)
@@ -849,7 +875,7 @@ public class TestJobManager {
.build();
}
- private static JobEntity newJobEntity(String templateName, JobHandle.Status
status) {
+ private JobEntity newJobEntity(String templateName, JobHandle.Status status)
{
Random rand = new Random();
return JobEntity.builder()
.withId(rand.nextLong())
