eric-maynard commented on code in PR #1557:
URL: https://github.com/apache/polaris/pull/1557#discussion_r2086058797
##########
polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisConfigurationStoreTest.java:
##########
@@ -75,7 +75,7 @@ public void testConfigsCanBeCastedFromString() {
public void testInvalidCastThrowsException() {
// Bool not included because Boolean.valueOf turns non-boolean strings to
false
List<PolarisConfiguration<?>> configs =
- List.of(buildConfig("int", 12), buildConfig("long", 34L),
buildConfig("double", 5.6D));
+ List.of(buildConfig("int2", 12), buildConfig("long2", 34L),
buildConfig("double2", 5.6D));
Review Comment:
Good eye, it's because of this check:
https://github.com/apache/polaris/pull/1557/files#diff-47b5a081da8c7bbc4f361419fdb9f746bbdfbcc1dc4e5544c96123abbe4ab1adR61
These keys are re-used across tests. Maybe there's some elegant way we can
avoid having to make the names unique in tests (a `deregister` method?) but I
judged the issue small enough to defer a fix for now. Let me know what you
think though, I admit it's a bit awkward. It could result in something that's
annoying to debug.
edit: I toyed with a `deregisterConfiguration` method but it does make the
tests not safe to parallelize
--
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]
