gortiz commented on code in PR #13817:
URL: https://github.com/apache/pinot/pull/13817#discussion_r1716743794
##########
pinot-core/src/test/java/org/apache/pinot/queries/FluentQueryTest.java:
##########
@@ -55,36 +57,115 @@
import org.testng.Assert;
+/**
+ * A fluent API for testing queries.
+ *
+ * Use {@link #withBaseDir(File)} to start a new test.
+ *
+ * @see
org.apache.pinot.core.query.aggregation.function.CountAggregationFunctionTest
+ */
public class FluentQueryTest {
private final FluentBaseQueriesTest _baseQueriesTest;
- private final File _baseDir;
+ final File _baseDir;
private final Map<String, String> _extraQueryOptions = new HashMap<>();
- private FluentQueryTest(FluentBaseQueriesTest baseQueriesTest, File baseDir)
{
+ FluentQueryTest(FluentBaseQueriesTest baseQueriesTest, File baseDir) {
_baseQueriesTest = baseQueriesTest;
_baseDir = baseDir;
}
+ /**
+ * Start a new test with the given base directory.
+ *
+ * Usually the base directory will be created before every test and
destroyed after that using lifecycle testing
+ * hooks like {@link org.testng.annotations.BeforeClass} and {@link
org.testng.annotations.AfterClass}.
+ *
+ * Each test will create its own subdirectory in the base directory, so
multiple tests may use the same base
+ * directory.
+ *
+ * @param baseDir the base directory for the test. It must exist, be a
directory and be writable.
+ * @return The fluent API for testing queries, where eventually {@link
#givenTable(Schema, TableConfig)} will be
+ * called.
+ */
public static FluentQueryTest withBaseDir(File baseDir) {
+ Preconditions.checkArgument(baseDir.exists(), "Base directory must exist");
+ Preconditions.checkArgument(baseDir.isDirectory(), "Base directory must be
a directory");
+ Preconditions.checkArgument(baseDir.canWrite(), "Base directory must be
writable");
return new FluentQueryTest(new FluentBaseQueriesTest(), baseDir);
}
+ /**
+ * Creates a new test with a temporary directory.
+ *
+ * @param consumer the test to run. The received FluentQueryTest will use a
temporary directory that will be removed
+ * after the consumer is executed, even if a throwable is
thrown.
+ */
+ public static void test(Consumer<FluentQueryTest> consumer) {
+ StackWalker walker =
StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE);
+ try (Closeable test = new Closeable(walker.getCallerClass().getName())) {
+ consumer.accept(test);
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ }
+
+ /**
+ * Creates a new test with a temporary directory.
+ *
+ * The returned object is intended to be used in a try-with-resources manner.
+ * Its close method will remove the temporary directory.
+ */
+ public static FluentQueryTest.Closeable open() {
Review Comment:
They are just syntactic sugar. I'm not sure which one will be better in
terms of usability/readability, so I'm adding both of them to let each test
decide.
For example, `test` may be usually shorter but sometimes it can be
problematic if the consumer implementation has to throw a cached exception (we
could introduce a version that uses a CheckedConsumer, but I would need to
create the interface and I was lazy).
Completely offtopic:
I'm more used to consume these objects from Kotlin, whose ability to use
these DSLs is super nice. For example we could use the test method as
```kotlin
fun myTest() {
FluentQueryTest.test {
givenTable(...)
.onFirstInstace(...)
...
}
}
```
(we could even remove the `.test` if we rename that method to `invoke`)
and the other as:
```kotlin
fun myTest() {
FluentQueryTest.open().use {
it.givenTable(...)
.onFirstInstace(...)
...
}
}
```
--
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...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
