aihuaxu commented on code in PR #158:
URL: https://github.com/apache/polaris/pull/158#discussion_r1733631818
##########
polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisOverlappingCatalogTest.java:
##########
@@ -110,101 +119,72 @@ private Response createCatalog(
}
}
- @Test
- public void testBasicOverlappingCatalogs() {
- Arrays.asList(false, true)
- .forEach(
- initiallyExternal -> {
- Arrays.asList(false, true)
- .forEach(
- laterExternal -> {
- String prefix = UUID.randomUUID().toString();
-
- assertThat(createCatalog(prefix, "root",
initiallyExternal))
- .returns(Response.Status.CREATED.getStatusCode(),
Response::getStatus);
-
- // OK, non-overlapping
- assertThat(createCatalog(prefix, "boot",
laterExternal))
- .returns(Response.Status.CREATED.getStatusCode(),
Response::getStatus);
-
- // OK, non-overlapping due to no `/`
- assertThat(createCatalog(prefix, "roo", laterExternal))
- .returns(Response.Status.CREATED.getStatusCode(),
Response::getStatus);
-
- // Also OK due to no `/`
- assertThat(createCatalog(prefix, "root.child",
laterExternal))
- .returns(Response.Status.CREATED.getStatusCode(),
Response::getStatus);
-
- // inside `root`
- assertThat(createCatalog(prefix, "root/child",
laterExternal))
- .returns(
- Response.Status.BAD_REQUEST.getStatusCode(),
Response::getStatus);
-
- // `root` is inside this
- assertThat(createCatalog(prefix, "", laterExternal))
- .returns(
- Response.Status.BAD_REQUEST.getStatusCode(),
Response::getStatus);
- });
- });
+ @ParameterizedTest
Review Comment:
It's not. Let me revert.
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java:
##########
@@ -46,7 +46,6 @@
*/
public abstract class LocalPolarisMetaStoreManagerFactory<StoreType>
implements MetaStoreManagerFactory {
-
Review Comment:
Yeah. Let me remove this unrelated change from this PR.
##########
polaris-service/src/test/java/org/apache/polaris/service/test/PolarisConnectionExtension.java:
##########
@@ -109,10 +118,40 @@ public void beforeAll(ExtensionContext extensionContext)
throws Exception {
}
}
+ @Override
+ public void afterAll(ExtensionContext context) {
+ if (metaStoreManagerFactory instanceof
LocalPolarisMetaStoreManagerFactory) {
+ metaStoreManagerFactory.purgeRealms(List.of(realm));
+ }
+ }
+
public static String getTestRealm(Class testClassName) {
return testClassName.getName().replace('.', '_');
}
+ public static void createTestDir(String realm) throws IOException {
+ // Set up the database location
+ Path testDir = Path.of("build/test_data/iceberg/" + realm);
Review Comment:
Yeah. That is like this currently. Let me change to polaris.
##########
polaris-service/build.gradle.kts:
##########
@@ -108,6 +108,9 @@ dependencies {
testImplementation(libs.assertj.core)
testImplementation(libs.mockito.core)
testRuntimeOnly("org.junit.platform:junit-platform-launcher")
+
+ testRuntimeOnly(project(":polaris-eclipselink"))
Review Comment:
If we create a separate project to include the dependencies, we still need
to include such project here , right? This is for test only though.
--
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]
