Skip to content

Commit a0b0b9a

Browse files
committed
fix: address PR #18069 review comments
- Remove deprecated PropertyNamingStrategies.UPPER_CAMEL_CASE usage - Remove empty javadoc line - Fix infinite loop in API tests with 120s timeout - Reduce polling interval from 5s to 2s - Add explicit workflow instance final state assertions
1 parent 88d8640 commit a0b0b9a

2 files changed

Lines changed: 22 additions & 14 deletions

File tree

  • dolphinscheduler-api-test/dolphinscheduler-api-test-case/src/test/java/org/apache/dolphinscheduler/api/test/cases/tasks
  • dolphinscheduler-task-plugin/dolphinscheduler-task-emr-serverless/src/main/java/org/apache/dolphinscheduler/plugin/task/emrserverless

dolphinscheduler-api-test/dolphinscheduler-api-test-case/src/test/java/org/apache/dolphinscheduler/api/test/cases/tasks/EmrServerlessTaskAPITest.java

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -142,23 +142,27 @@ public void testEmrServerlessSuccessWorkflowInstance() {
142142
workflowInstanceIds = (List<Integer>) startWorkflowInstanceResponse.getBody().getData();
143143
Assertions.assertFalse(workflowInstanceIds.isEmpty(), "No workflow instances were created");
144144

145-
// Wait for workflow instance to finish and assert SUCCESS
145+
// Wait for workflow instance to finish with timeout
146146
int workflowInstanceId = workflowInstanceIds.get(0);
147147
log.info("Waiting for EMR Serverless success workflow instance: {}", workflowInstanceId);
148-
while (true) {
149-
Thread.sleep(5000);
148+
long timeout = 120_000; // 120 seconds
149+
long startTime = System.currentTimeMillis();
150+
String finalState = null;
151+
while (System.currentTimeMillis() - startTime < timeout) {
152+
Thread.sleep(2000);
150153
HttpResponse queryResponse = workflowInstancePage.queryWorkflowInstanceById(
151154
loginUser, projectCode, workflowInstanceId);
152155
LinkedHashMap<String, Object> instanceData =
153156
(LinkedHashMap<String, Object>) queryResponse.getBody().getData();
154157
String state = (String) instanceData.get("state");
155158
log.info("EMR Serverless success workflow instance state: {}", state);
156-
if ("SUCCESS".equals(state)) {
159+
if ("SUCCESS".equals(state) || "FAILURE".equals(state) || "STOP".equals(state)) {
160+
finalState = state;
157161
break;
158-
} else if ("FAILURE".equals(state) || "STOP".equals(state)) {
159-
Assertions.fail("EMR Serverless workflow instance expected SUCCESS but got: " + state);
160162
}
161163
}
164+
Assertions.assertNotNull(finalState, "Workflow instance did not reach a final state within timeout");
165+
Assertions.assertEquals("SUCCESS", finalState, "Expected workflow instance to succeed");
162166
} catch (Exception e) {
163167
log.error("failed", e);
164168
Assertions.fail();
@@ -204,23 +208,28 @@ public void testEmrServerlessFailedWorkflowInstance() {
204208
(List<Integer>) startWorkflowInstanceResponse.getBody().getData();
205209
Assertions.assertFalse(failedWorkflowInstanceIds.isEmpty(), "No workflow instances were created");
206210

207-
// Wait for workflow instance to finish and assert FAILURE
211+
// Wait for workflow instance to finish with timeout
208212
int failedWorkflowInstanceId = failedWorkflowInstanceIds.get(0);
209213
log.info("Waiting for EMR Serverless failed workflow instance: {}", failedWorkflowInstanceId);
210-
while (true) {
211-
Thread.sleep(5000);
214+
long timeout = 120_000; // 120 seconds
215+
long startTime = System.currentTimeMillis();
216+
String finalState = null;
217+
while (System.currentTimeMillis() - startTime < timeout) {
218+
Thread.sleep(2000);
212219
HttpResponse queryResponse = workflowInstancePage.queryWorkflowInstanceById(
213220
loginUser, projectCode, failedWorkflowInstanceId);
214221
LinkedHashMap<String, Object> instanceData =
215222
(LinkedHashMap<String, Object>) queryResponse.getBody().getData();
216223
String state = (String) instanceData.get("state");
217224
log.info("EMR Serverless failed workflow instance state: {}", state);
218-
if ("FAILURE".equals(state) || "STOP".equals(state)) {
225+
if ("SUCCESS".equals(state) || "FAILURE".equals(state) || "STOP".equals(state)) {
226+
finalState = state;
219227
break;
220-
} else if ("SUCCESS".equals(state)) {
221-
Assertions.fail("EMR Serverless workflow instance expected FAILURE but got SUCCESS");
222228
}
223229
}
230+
Assertions.assertNotNull(finalState, "Workflow instance did not reach a final state within timeout");
231+
Assertions.assertTrue("FAILURE".equals(finalState) || "STOP".equals(finalState),
232+
"Expected workflow instance to fail, but got: " + finalState);
224233
} catch (Exception e) {
225234
log.error("failed", e);
226235
Assertions.fail();

dolphinscheduler-task-plugin/dolphinscheduler-task-emr-serverless/src/main/java/org/apache/dolphinscheduler/plugin/task/emrserverless/EmrServerlessTask.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@
7070
* Submits a job run to an EMR Serverless application and tracks it until completion.
7171
* Supports Spark and Hive job types.
7272
* </p>
73-
*
7473
*/
7574
@Slf4j
7675
public class EmrServerlessTask extends AbstractRemoteTask {
@@ -93,7 +92,7 @@ public class EmrServerlessTask extends AbstractRemoteTask {
9392
.configure(READ_UNKNOWN_ENUM_VALUES_AS_NULL, true)
9493
.configure(REQUIRE_SETTERS_FOR_GETTERS, true)
9594
.defaultTimeZone(SystemConstants.DEFAULT_TIME_ZONE)
96-
.propertyNamingStrategy(PropertyNamingStrategies.UPPER_CAMEL_CASE)
95+
.propertyNamingStrategy(new PropertyNamingStrategies.UpperCamelCaseStrategy())
9796
.build();
9897

9998
private final TaskExecutionContext taskExecutionContext;

0 commit comments

Comments
 (0)