echonesis commented on code in PR #9960:
URL: https://github.com/apache/gravitino/pull/9960#discussion_r2910667139
##########
core/src/test/java/org/apache/gravitino/job/TestJobManager.java:
##########
@@ -899,4 +901,125 @@ private JobEntity newJobEntity(String templateName,
JobHandle.Status status) {
AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build())
.build();
}
+
+ @Test
+ public void testBuildArgumentsWithOptional() {
+ // Test Case 1: No optional arguments provided
+ List<String> templateArgs1 =
+ Lists.newArrayList(
+ "--catalog", "{{catalog}}", "--table", "{{table}}", "?--strategy",
"?{{strategy}}");
+ Map<String, String> jobConf1 = ImmutableMap.of("catalog", "hive", "table",
"db.table");
+
+ List<String> result1 =
JobManager.buildArgumentsWithOptional(templateArgs1, jobConf1);
+ Assertions.assertEquals(
+ Lists.newArrayList("--catalog", "hive", "--table", "db.table"),
result1);
+
+ // Test Case 2: One optional argument provided
+ Map<String, String> jobConf2 =
+ ImmutableMap.of("catalog", "hive", "table", "db.table", "strategy",
"binpack");
+
+ List<String> result2 =
JobManager.buildArgumentsWithOptional(templateArgs1, jobConf2);
+ Assertions.assertEquals(
+ Lists.newArrayList("--catalog", "hive", "--table", "db.table",
"--strategy", "binpack"),
+ result2);
+
+ // Test Case 3: Empty string should be filtered
+ Map<String, String> jobConf3 =
+ ImmutableMap.of("catalog", "hive", "table", "db.table", "strategy",
"");
+
+ List<String> result3 =
JobManager.buildArgumentsWithOptional(templateArgs1, jobConf3);
+ Assertions.assertEquals(
+ Lists.newArrayList("--catalog", "hive", "--table", "db.table"),
result3);
+
+ // Test Case 4: Whitespace should be filtered
+ Map<String, String> jobConf4 =
+ ImmutableMap.of("catalog", "hive", "table", "db.table", "strategy", "
");
+
+ List<String> result4 =
JobManager.buildArgumentsWithOptional(templateArgs1, jobConf4);
+ Assertions.assertEquals(
+ Lists.newArrayList("--catalog", "hive", "--table", "db.table"),
result4);
Review Comment:
Thanks for reminding.
All three edge cases in **testBuildArgumentsWithOptional**:
1. Standalone `?--flag`: Added Test Case 10 to cover `?--flag` at the end of
the list, verifying it is always emitted.
2. `["?--flag", "{{val}}"]`: Added Test Case 8 to verify that the entire
pair is correctly skipped when the value is absent, fixing the previous bug
where the flag was leaked.
3. Compound `?{{prefix}}/{{suffix}}`: Added Test Case 11 to document the
known limitation where non-whitespace separators (like `/`) cause the raw
placeholder string to leak when keys are absent.
--
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]
