kgyrtkirk commented on code in PR #16382:
URL: https://github.com/apache/druid/pull/16382#discussion_r1591916011
##########
sql/src/test/java/org/apache/druid/sql/calcite/SqlTestFrameworkConfig.java:
##########
@@ -36,77 +39,200 @@
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.Set;
import java.util.function.Function;
/**
- * Annotation to specify desired framework settings.
+ * Specifies current framework settings.
*
- * This class provides junit rule facilities to build the framework accordingly
- * to the annotation. These rules also cache the previously created frameworks.
+ * Intended usage from tests is via the annotations:
+ * @SqlTestFrameworkConfig.MinTopNThreshold(33)
+ *
+ * In case of annotations used; it picks up all annotations from:
+ * * the method
+ * * its enclosing class and its parents
+ * if none contains a specific setting the default is being taken.
+ *
+ * All configurable setting should have:
+ * * an annotation with `value` with the desired type
+ * * the annotation itself should be annotated with itslef to set the
default value
+ * * a field should be added to the main config class
*/
-@Retention(RetentionPolicy.RUNTIME)
-@Target({ElementType.METHOD})
-public @interface SqlTestFrameworkConfig
+public class SqlTestFrameworkConfig
{
- int numMergeBuffers() default 0;
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target({ElementType.METHOD, ElementType.TYPE})
+ @NumMergeBuffers(0)
+ public @interface NumMergeBuffers
+ {
+ int value();
+ }
- int minTopNThreshold() default TopNQueryConfig.DEFAULT_MIN_TOPN_THRESHOLD;
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target({ElementType.METHOD, ElementType.TYPE})
+ @MinTopNThreshold(TopNQueryConfig.DEFAULT_MIN_TOPN_THRESHOLD)
+ public @interface MinTopNThreshold
Review Comment:
I've not planned anything for queryContext - because that can be changed at
a query-by-query basis in the testBuilder; these configs should only include
things which make the backing test environment different.
I was always seen `minTopNThreashold` which might have been mis-categorized
to be specified as test environment config instead of just overriding it in the
querycontext.
I'll see if it can be removed in a followup - unnecessary configuration axes
may just increase the configuration space and possibly cause slow down if
different values are provided for them without much benefits
I you have in mind something like:
```java
@QueryContext( enableWindowing = true )
class SomeTest {
@Test
@QueryContext( minTopNThreshold = 11 )
{
}
}
```
that's also possible ; but haven't thinked about it
--
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: commits-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
