jason810496 commented on code in PR #73595:
URL: https://github.com/apache/airflow/pull/73595#discussion_r4140346151
##########
java-sdk/sdk/src/test/kotlin/org/apache/airflow/sdk/DagDefTest.kt:
##########
@@ -161,6 +163,126 @@ internal class DagDefTest {
Assertions.assertEquals(setOf("left", "right"), upstreamsOf(dag, "join"))
}
+ @Test
+ @DisplayName("Should store validated dag config values keyed by schema name")
+ fun shouldStoreDagConfigValues() {
+ val dag =
+ DagDef("dag")
+ .config("schedule", "@daily")
+ .config("description", "demo")
+ .config("catchup", true)
+ .config("max_active_runs", 3)
+ .config("dagrun_timeout", Duration.ofMinutes(5))
+ .config("start_date", OffsetDateTime.parse("2026-01-01T00:00:00Z"))
+ .config("tags", listOf("a", "b"))
+
+ Assertions.assertEquals(
+ mapOf(
+ "schedule" to "@daily",
+ "description" to "demo",
+ "catchup" to true,
+ "max_active_runs" to 3,
+ "dagrun_timeout" to Duration.ofMinutes(5),
+ "start_date" to OffsetDateTime.parse("2026-01-01T00:00:00Z"),
+ "tags" to listOf("a", "b"),
+ ),
+ dag.dagConfig,
+ )
+ }
+
+ @Test
+ @DisplayName("Should reject unknown dag config keys")
+ fun shouldRejectUnknownDagConfigKey() {
+ val error =
+ Assertions.assertThrows(IllegalArgumentException::class.java) {
+ DagDef("dag").config("scheduel", "@daily")
+ }
+
+ Assertions.assertEquals("Unknown Dag config key: 'scheduel'",
error.message)
+ }
+
+ @Test
+ @DisplayName("Should reject dag config values of the wrong type")
+ fun shouldRejectMismatchedDagConfigValue() {
+ val error =
+ Assertions.assertThrows(IllegalArgumentException::class.java) {
+ DagDef("dag").config("catchup", "yes")
+ }
+
+ Assertions.assertEquals(
+ "Value for Dag config key 'catchup' must be a Boolean, got:
java.lang.String",
+ error.message,
+ )
+ }
+
+ @Test
+ @DisplayName("Should reject null dag config values")
+ fun shouldRejectNullDagConfigValue() {
+ val error =
+ Assertions.assertThrows(IllegalArgumentException::class.java) {
+ DagDef("dag").config("description", null)
+ }
+
+ Assertions.assertEquals("Value for Dag config key 'description' must not
be null", error.message)
+ }
+
+ @Test
+ @DisplayName("Should reject non-integral values for integer dag config keys")
+ fun shouldRejectFractionalIntegerValue() {
+ val error =
+ Assertions.assertThrows(IllegalArgumentException::class.java) {
+ DagDef("dag").config("max_active_runs", 1.5)
+ }
+
+ Assertions.assertEquals(
+ "Value for Dag config key 'max_active_runs' must be an integral Number,
got: java.lang.Double",
+ error.message,
+ )
+ }
Review Comment:
Added, one per remaining value type, in `8b8bccd75e`:
- `config("dagrun_timeout", "PT5M")` and `config("retry_delay", "PT5M")` on
a `TaskDef` — the second one also covers the `task` scope in the message, since
the first four rejection cases only exercised `Dag`.
- `config("start_date", "2026-01-01")`
- `config("tags", ...)` with non-string elements
That covers every `FieldType` branch in the validator now. One deviation
from your example: I built the bad list as `arrayListOf(1, 2)` rather than
`listOf(1, 2)`, because the message ends in `got: <runtime class>` and `listOf`
returns `java.util.Arrays$ArrayList`, which is an implementation detail of the
Kotlin stdlib rather than anything the test is about. `arrayListOf` makes the
asserted `java.util.ArrayList` obvious from the test source.
Worth noting separately: for the array case that `got: java.util.ArrayList`
is not very useful, since the value *is* an `Iterable` and it is the element
type that is wrong. Happy to make the message name the offending element
instead if you think it is worth it.
---
Drafted-by: Claude Code (Opus 5) (no human review before posting)
--
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]
