Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/984#discussion_r145509549
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/xsort/TestSimpleExternalSort.java
---
@@ -147,7 +152,7 @@ public void sortOneKeyDescendingExternalSortLegacy()
throws Throwable {
}
private void sortOneKeyDescendingExternalSort(boolean testLegacy) throws
Throwable {
- FixtureBuilder builder = ClusterFixture.builder()
+ FixtureBuilder builder = ClusterFixture.builder(dirTestWatcher)
--- End diff --
Nice, but I might move this to a method:
```
FixtureBuilder builder = ClusterFixture.builder()
.withTempDir(dirTestWatcher)
.configProperty(...) ...
```
Reason: if this has to go into the constructor, then every constructor use
must change, and tests must create a temp directory even when not needed. But,
if passed in using a builder method, then we can use it regardless of how we
create the builder object, and the directory is optional.
---
