rishabhdaim commented on code in PR #2947:
URL: https://github.com/apache/jackrabbit-oak/pull/2947#discussion_r3405405258
##########
oak-core-spi/src/test/java/org/apache/jackrabbit/oak/spi/toggle/FeatureToggleTest.java:
##########
@@ -50,6 +51,28 @@ public void disabledByDefault() {
}
}
+ @Test
Review Comment:
Optional (for local consideration only — not pushing code):
`defaultOverriddenAsTrue` / `defaultOverriddenAsFalse` already cover explicit
boolean overrides. For other `oak-feature.*` values, `SystemPropertySupplier`
uses `Boolean.valueOf(String)` semantics: only `"true"` (case-insensitive)
enables the toggle; strings like `"yes"`, `"garbage"`, `"1"`, etc. are treated
as `false`, not as a parse error with log-and-fallback. If you want to document
that contract in tests, something like:
```java
/**
* SystemPropertySupplier uses {@link Boolean#valueOf(String)} for
boolean props: only
* {@code "true"} (case-insensitive) enables the toggle; values like
{@code "yes"} or
* {@code "garbage"} are treated as {@code false}, not as a parse error.
*/
@Test
public void defaultOverriddenWithNonTrueValueStaysDisabled() {
String sysPropName = "oak-feature.my.toggle";
for (String value : new String[] {"garbage", "yes", "1", "on", ""}) {
System.setProperty(sysPropName, value);
try (Feature feature =
newFeatureWithSystemPropertyDefault("my.toggle", whiteboard)) {
assertFalse("property value '" + value + "' must not enable
the toggle", feature.isEnabled());
} finally {
System.clearProperty(sysPropName);
}
}
}
```
--
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]
