Copilot commented on code in PR #19925:
URL: https://github.com/apache/druid/pull/19925#discussion_r3738016304
##########
indexing-service/src/test/java/org/apache/druid/indexing/worker/WorkerTaskManagerTest.java:
##########
@@ -91,22 +95,21 @@ public class WorkerTaskManagerTest
private final IndexMergerV9Factory indexMergerV9Factory;
private final IndexIO indexIO;
- private final boolean restoreTasksOnRestart;
+ @Parameter(0)
+ private boolean restoreTasksOnRestart;
Review Comment:
`@Parameter` fields are consistently declared `public` for
`@ParameterizedClass` tests in this codebase (for example
`processing/src/test/java/org/apache/druid/common/guava/CombiningSequenceTest.java:63-65`).
Keeping this one `private` risks the parameter injection not being applied
(and the default `false` being used), depending on how `@ParameterizedClass`
populates fields. Please align with the established pattern by making the field
`public`.
##########
indexing-service/src/test/java/org/apache/druid/indexing/worker/shuffle/ShuffleDataSegmentPusherTest.java:
##########
@@ -68,45 +67,41 @@
import java.util.List;
import java.util.Optional;
-@RunWith(Parameterized.class)
+@ParameterizedClass
+@MethodSource("data")
public class ShuffleDataSegmentPusherTest
{
private static final String LOCAL = "local";
private static final String DEEPSTORE = "deepstore";
- @Parameterized.Parameters(name = "intermediateDataManager={0}")
public static Collection<Object[]> data()
{
return ImmutableList.of(new Object[]{LOCAL}, new Object[]{DEEPSTORE});
}
- @Rule
- public final TemporaryFolder temporaryFolder = new TemporaryFolder();
+ @Parameter(0)
+ private String intermediateDataStore;
+
Review Comment:
`@Parameter` fields in existing `@ParameterizedClass` tests are declared
`public` (for example
`processing/src/test/java/org/apache/druid/common/guava/CombiningSequenceTest.java:63-65`).
Using `private` here risks the parameter injection not being applied,
depending on the `@ParameterizedClass` implementation. Please make the field
`public` to match the established pattern.
--
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]
