github-actions[bot] commented on code in PR #64571:
URL: https://github.com/apache/doris/pull/64571#discussion_r3421222178
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/AdminCreateClusterSnapshotCommandTest.java:
##########
@@ -97,13 +97,10 @@ public void testValidateNormal() throws Exception {
properties.add(Pair.of(ImmutableMap.of("ttl", "a", "label", "a"),
"Invalid value"));
properties.add(Pair.of(ImmutableMap.of("ttl", "0", "label", "a"),
"Property 'ttl' must be positive"));
properties.add(Pair.of(ImmutableMap.of("ttl", "3600", "label", ""),
"Property 'label' cannot be empty"));
- properties.add(Pair.of(ImmutableMap.of("ttl", "3600", "label", "a",
"vault_name", ""),
- "Property 'vault_name' cannot be empty"));
// unknown property
properties.add(Pair.of(ImmutableMap.of("ttl", "0", "a", "b"), "Unknown
property"));
Review Comment:
This no longer proves that unknown properties, especially the removed
`vault_name`, are rejected. `ImmutableMap.of("ttl", "0", "a", "b")` can fail on
the `ttl <= 0` validation before reaching the unknown-property branch, and the
third `assertThrows` argument is only the assertion failure message, not a
check on the thrown exception text. Since this PR changes the user-facing
behavior from accepting `vault_name` to rejecting it, please make the negative
case otherwise valid and add an explicit `vault_name` case.
```suggestion
properties.add(Pair.of(ImmutableMap.of("ttl", "3600", "label", "a",
"a", "b"), "Unknown property"));
properties.add(Pair.of(ImmutableMap.of("ttl", "3600", "label", "a",
"vault_name", "vault_1"),
"Unknown property"));
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
