clintropolis commented on code in PR #18207: URL: https://github.com/apache/druid/pull/18207#discussion_r2237828303
########## embedded-tests/src/test/java/org/apache/druid/testing/embedded/compact/CompactionTestBase.java: ########## @@ -0,0 +1,134 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.testing.embedded.compact; + +import org.apache.druid.indexing.common.task.TaskBuilder; +import org.apache.druid.indexing.overlord.Segments; +import org.apache.druid.java.util.common.guava.Comparators; +import org.apache.druid.testing.embedded.EmbeddedBroker; +import org.apache.druid.testing.embedded.EmbeddedClusterApis; +import org.apache.druid.testing.embedded.EmbeddedCoordinator; +import org.apache.druid.testing.embedded.EmbeddedDruidCluster; +import org.apache.druid.testing.embedded.EmbeddedHistorical; +import org.apache.druid.testing.embedded.EmbeddedIndexer; +import org.apache.druid.testing.embedded.EmbeddedOverlord; +import org.apache.druid.testing.embedded.EmbeddedRouter; +import org.apache.druid.testing.embedded.junit5.EmbeddedClusterTestBase; +import org.apache.druid.timeline.DataSegment; +import org.joda.time.Interval; +import org.junit.jupiter.api.Assertions; + +import java.io.Closeable; +import java.util.Comparator; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.stream.Collectors; + +public abstract class CompactionTestBase extends EmbeddedClusterTestBase Review Comment: other than maybe the cluster config, most of the utility methods on this class don't seem too related to compaction, should they live somewhere more common? Is fine to change this later, just want to avoid copy and paste of these methods down the line or like weird extending of compaction test base for things that aren't doing any compaction ########## embedded-tests/src/test/java/org/apache/druid/testing/embedded/compact/AutoCompactionTest.java: ########## @@ -1989,6 +1884,7 @@ private void forceTriggerAutoCompaction( ); // Wait for scheduler to pick up the compaction job + // TODO: make this latch-based Review Comment: should either do this or transition from a TODO to a comment about how someone in the future can improve this ########## embedded-tests/src/test/java/org/apache/druid/testing/embedded/indexing/IndexTaskTest.java: ########## @@ -50,7 +50,7 @@ /** * Simulation tests for batch {@link IndexTask} using inline datasources. Review Comment: nit: unrelated to this PR but did a search and see 2 usages of this terminology in embedded-tests, here and `KafkaClusterMetricsTest` which if i remember correctly was the old name before we settled on embedded (there are another 2 that show up but are related to coordinator simulation stuff which seem legit to call them that) -- 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]
