You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2021/04/14 16:23:40 UTC

[GitHub] [drill] cgivre opened a new pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

cgivre opened a new pull request #2200:
URL: https://github.com/apache/drill/pull/2200


   # [DRILL-7901](https://issues.apache.org/jira/browse/DRILL-7901): Bump junit from 4.12 to 4.13.1
   
   ## Description
   <details>
   <summary>Release notes</summary>
   <p><em>Sourced from <a href="https://github.com/junit-team/junit4/releases">junit's releases</a>.</em></p>
   <blockquote>
   <h2>JUnit 4.13.1</h2>
   <p>Please refer to the <a href="https://github.com/junit-team/junit/blob/HEAD/doc/ReleaseNotes4.13.1.md">release notes</a> for details.</p>
   <h2>JUnit 4.13</h2>
   <p>Please refer to the <a href="https://github.com/junit-team/junit/blob/HEAD/doc/ReleaseNotes4.13.md">release notes</a> for details.</p>
   <h2>JUnit 4.13 RC 2</h2>
   <p>Please refer to the <a href="https://github.com/junit-team/junit4/wiki/4.13-Release-Notes">release notes</a> for details.</p>
   <h2>JUnit 4.13 RC 1</h2>
   <p>Please refer to the <a href="https://github.com/junit-team/junit4/wiki/4.13-Release-Notes">release notes</a> for details.</p>
   <h2>JUnit 4.13 Beta 3</h2>
   <p>Please refer to the <a href="https://github.com/junit-team/junit4/wiki/4.13-Release-Notes">release notes</a> for details.</p>
   <h2>JUnit 4.13 Beta 2</h2>
   <p>Please refer to the <a href="https://github.com/junit-team/junit4/wiki/4.13-Release-Notes">release notes</a> for details.</p>
   <h2>JUnit 4.13 Beta 1</h2>
   <p>Please refer to the <a href="https://github.com/junit-team/junit4/wiki/4.13-Release-Notes">release notes</a> for details.</p>
   </blockquote>
   </details>
   
   ## Documentation
   N/A
   
   ## Testing
   N/A


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] luocooong commented on pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
luocooong commented on pull request #2200:
URL: https://github.com/apache/drill/pull/2200#issuecomment-831693653


   @cgivre Would you please rebase on the master? and then re-run the CI on the basis of the #2203.
   Let us merge this PR once the CI successfully. Create a new PR for the `Bump the version to JUnit 5`?


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] martin-g commented on pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #2200:
URL: https://github.com/apache/drill/pull/2200#issuecomment-840351108


   > Would you recommend using Junit 5 rather than 4.13?
   
   I'd recommend JUnit 5 too!
   The initial upgrade would be very easy by using JUnit Vintage Engine. It is fully backwards compatible with JUnit 4.
   Later you could migrate the tests one by one to JUnit 5 Jupiter Engine.
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] vvysotskyi commented on pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on pull request #2200:
URL: https://github.com/apache/drill/pull/2200#issuecomment-819741252


   I recall some tests were hanging when updating the JUnit version to 4.13.1. Have you tried running all unit tests locally since CI is not working?


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre commented on a change in pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2200:
URL: https://github.com/apache/drill/pull/2200#discussion_r635226065



##########
File path: contrib/storage-kafka/pom.xml
##########
@@ -86,6 +86,25 @@
       <version>${project.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>com.101tec</groupId>
+      <artifactId>zkclient</artifactId>
+      <version>0.11</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.kafka</groupId>
+      <artifactId>kafka_2.12</artifactId>
+      <version>${kafka.version}</version>
+      <classifier>test</classifier>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.servicemix.bundles</groupId>
+      <artifactId>org.apache.servicemix.bundles.kafka_2.12</artifactId>
+      <version>2.3.1_1</version>
+      <scope>test</scope>
+    </dependency>

Review comment:
       Done!




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] vvysotskyi commented on a change in pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2200:
URL: https://github.com/apache/drill/pull/2200#discussion_r633233309



##########
File path: contrib/storage-kafka/src/test/java/org/apache/drill/exec/store/kafka/KafkaFilterPushdownTest.java
##########
@@ -419,13 +419,13 @@ public void testNoPushdownOfOffsetWithNonMetadataField() throws Exception {
     final int expectedRowCount = 30;
 
     final String queryString = String.format(TestQueryConstants.QUERY_TEMPLATE_AND_OR_PATTERN_2,
-        TestQueryConstants.JSON_PUSHDOWN_TOPIC, predicate1, predicate2, predicate3);
+      TestQueryConstants.JSON_PUSHDOWN_TOPIC, predicate1, predicate2, predicate3);
 
     runKafkaSQLVerifyCount(queryString,expectedRowCount);
     queryBuilder()
-        .sql(queryString)
-        .jsonPlanMatcher()
-        .include(String.format(EXPECTED_PATTERN, expectedRowCountInPlan))
-        .match();
+      .sql(queryString)
+      .jsonPlanMatcher()
+      .include(String.format(EXPECTED_PATTERN, expectedRowCountInPlan))
+      .match();
   }
-}
+}

Review comment:
       Please revert formatting changes here and in other places. It is fine to have 4 spaces for the case when moving builder methods or arguments on the new line and having an empty line at the end of the class.

##########
File path: contrib/storage-kafka/pom.xml
##########
@@ -86,6 +86,25 @@
       <version>${project.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>com.101tec</groupId>
+      <artifactId>zkclient</artifactId>
+      <version>0.11</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.kafka</groupId>
+      <artifactId>kafka_2.12</artifactId>
+      <version>${kafka.version}</version>
+      <classifier>test</classifier>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.servicemix.bundles</groupId>
+      <artifactId>org.apache.servicemix.bundles.kafka_2.12</artifactId>
+      <version>2.3.1_1</version>
+      <scope>test</scope>
+    </dependency>

Review comment:
       Could you please revert these changes, since they are not necessary?




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre commented on pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2200:
URL: https://github.com/apache/drill/pull/2200#issuecomment-841826190


   I'm a little stuck on this PR.  If I run the unit tests in `KafkaQueriesTest` individually on my local machine or from the CLI they all work.  However, if I run the entire test suite in my IDE, either the `testPartitionMaxOff` or `testPartitiionMinOff` fails with an `IllegalThreadException`. 
   
   Is this my environment, or does anyone have any suggestions as to how I can get this to work?  I'm not sure if this is a blocking issue or not. 
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] martin-g commented on pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #2200:
URL: https://github.com/apache/drill/pull/2200#issuecomment-840349595


   > I don't think this is related, but I'm getting out of memory errors on my local machine. Is there anything I can do to my environment to fix this?
   
   @cgivre You can overcome this by increasing the value of `directMemoryMb` system property. This is what I did with https://github.com/apache/drill/pull/2219


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre commented on pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2200:
URL: https://github.com/apache/drill/pull/2200#issuecomment-841826190






-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] laurentgo commented on pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
laurentgo commented on pull request #2200:
URL: https://github.com/apache/drill/pull/2200#issuecomment-831675862


   If the change is ready, I would recommend merging it asap. Moving to JUnit5 is likely to involve much more code change, and why beneficial, I'm not sure it warrants delaying the dependency update.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre commented on pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2200:
URL: https://github.com/apache/drill/pull/2200#issuecomment-828488597


   > I had Junit 5 migration code for Drill in my `shelf`. Let me check it how tests will pass on it.
   
   Hi @vdiravka 
   Would you recommend using Junit 5 rather than 4.13?


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre commented on pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2200:
URL: https://github.com/apache/drill/pull/2200#issuecomment-819778357


   > I recall some tests were hanging when updating the JUnit version to 4.13.1. Have you tried running all unit tests locally since CI is not working?
   
   I don't think this is related, but I'm getting out of memory errors on my local machine.  Is there anything I can do to my environment to fix this?
   
   ```bash
   Caused by: org.apache.drill.exec.exception.OutOfMemoryException: Failure allocating buffer.
   Caused by: java.lang.Error: failed to allocate 16777216 byte(s) of direct memory (used: 2617245696, max: 2621440000)
   
   [INFO] Running org.apache.drill.TestDirScanToValuesConversion
   [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.417 s - in org.apache.drill.TestDirScanToValuesConversion
   [WARNING] Tests run: 66, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 349.714 s - in org.apache.drill.exec.sql.TestMetastoreCommands
   [INFO]
   [INFO] Results:
   [INFO]
   [ERROR] Errors:
   [ERROR]   TestBugFixes.testDRILL1337_LocalRightFilterLeftOutJoin » UserRemote RESOURCE E...
   [ERROR]   TestBugFixes.testDRILL4884:231->BaseTestQuery.testRunAndReturn:346 » Rpc org.a...
   [ERROR]   TestExampleQueries.testParquetComplex » UserRemote RESOURCE ERROR: One or more...
   [ERROR]   TestExampleQueries.testWindowFunAndStarCol:1109->BaseTestQuery.testRunAndReturn:346 » Rpc
   [ERROR]   TestTpchDistributed.tpch01 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch03 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch04 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch05 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch07 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch08 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch09 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch10 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch11 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch12 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch13 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch15 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch16 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch17 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch18 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch19_1 » UserRemote RESOURCE ERROR: One or more nodes ra...
   [ERROR]   TestTpchDistributed.tpch20 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestTpchDistributed.tpch21 » UserRemote RESOURCE ERROR: One or more nodes ran ...
   [ERROR]   TestUnionAll.testDrill4147_2:1075->BaseTestQuery.testRunAndReturn:346 » Rpc or...
   [ERROR]   TestUnionDistinct.testUnionWithManyColumns » UserRemote RESOURCE ERROR: One or...
   [ERROR] org.apache.drill.exec.compile.TestLargeFileCompilation.testEXTERNAL_SORT
   [ERROR]   Run 1: TestLargeFileCompilation.setJDK:174 » IllegalState org.apache.drill.common.exc...
   [ERROR]   Run 2: TestLargeFileCompilation.resetJDK:179 » IllegalState org.apache.drill.common.e...
   [INFO]
   [ERROR] org.apache.drill.exec.compile.TestLargeFileCompilation.testGROUP_BY
   [ERROR]   Run 1: TestLargeFileCompilation.setJDK:174 » IllegalState org.apache.drill.common.exc...
   [ERROR]   Run 2: TestLargeFileCompilation.resetJDK:179 » IllegalState org.apache.drill.common.e...
   [INFO]
   [ERROR] org.apache.drill.exec.compile.TestLargeFileCompilation.testHashJoin
   [ERROR]   Run 1: TestLargeFileCompilation.setJDK:174 » IllegalState org.apache.drill.common.exc...
   [ERROR]   Run 2: TestLargeFileCompilation.resetJDK:179 » IllegalState org.apache.drill.common.e...
   [INFO]
   [ERROR] org.apache.drill.exec.compile.TestLargeFileCompilation.testJDKHugeStringConstantCompilation
   [ERROR]   Run 1: TestLargeFileCompilation.setJDK:174 » IllegalState org.apache.drill.common.exc...
   [ERROR]   Run 2: TestLargeFileCompilation.resetJDK:179 » IllegalState org.apache.drill.common.e...
   [INFO]
   [ERROR]   TestLargeFileCompilation.testMergeJoin » UserRemote SYSTEM ERROR: OutOfMemoryE...
   [ERROR] org.apache.drill.exec.compile.TestLargeFileCompilation.testPARQUET_WRITER
   [ERROR]   Run 1: TestLargeFileCompilation.setJDK:174 » IllegalState org.apache.drill.common.exc...
   [ERROR]   Run 2: TestLargeFileCompilation.resetJDK:179 » IllegalState org.apache.drill.common.e...
   [INFO]
   [ERROR] org.apache.drill.exec.compile.TestLargeFileCompilation.testProject
   [ERROR]   Run 1: TestLargeFileCompilation.testProject » User CONNECTION ERROR: Connection /127....
   [ERROR]   Run 2: TestLargeFileCompilation.resetJDK:179 » IllegalState org.apache.drill.common.e...
   [INFO]
   [ERROR] org.apache.drill.exec.compile.TestLargeFileCompilation.testTEXT_WRITER
   [ERROR]   Run 1: TestLargeFileCompilation.setJDK:174 » IllegalState org.apache.drill.common.exc...
   [ERROR]   Run 2: TestLargeFileCompilation.resetJDK:179 » IllegalState org.apache.drill.common.e...
   [INFO]
   [ERROR] org.apache.drill.exec.compile.TestLargeFileCompilation.testTOP_N_SORT
   [ERROR]   Run 1: TestLargeFileCompilation.setJDK:174 » IllegalState org.apache.drill.common.exc...
   [ERROR]   Run 2: TestLargeFileCompilation.resetJDK:179 » IllegalState org.apache.drill.common.e...
   [INFO]
   [ERROR]   TestAggregateFunctions.testDRILLNestedGBWithSubsetKeys:223 »  org.apache.drill...
   [ERROR]   TestAggregateFunctions.testDrill2170:177 »  org.apache.drill.common.exceptions...
   [ERROR]   TestNewSimpleRepeatedFunctions.testRepeatedCountVarBinary:259 »  org.apache.dr...
   [ERROR]   TestNewSimpleRepeatedFunctions.testRepeatedCountVarChar:150 »  org.apache.dril...
   [ERROR]   TestNewSimpleRepeatedFunctions.testRepeatedCountVarDecimal:249 »  org.apache.d...
   [ERROR]   TestDummyWriter>SubOperatorTest.classSetup:34 » OutOfMemory Failure allocating...
   [ERROR]   TestFillEmpties>SubOperatorTest.classSetup:34 » OutOfMemory Failure allocating...
   [ERROR] org.apache.drill.exec.physical.rowSet.TestOffsetVectorWriter.null
   [ERROR]   Run 1: TestOffsetVectorWriter.setup:62 » OutOfMemory Failure allocating buffer.
   [ERROR]   Run 2: TestOffsetVectorWriter>SubOperatorTest.classTeardown:39 » IllegalState Allocat...
   [INFO]
   [ERROR]   TestScalarAccessors>SubOperatorTest.classSetup:34 » OutOfMemory Failure alloca...
   [ERROR]   TestAnalyze.join » UserRemote RESOURCE ERROR: One or more nodes ran out of mem...
   [ERROR]   TestAnalyze.testAnalyzeSupportedFormats » UserRemote RESOURCE ERROR: One or mo...
   [ERROR]   TestAnalyze.testUseStatistics » UserRemote RESOURCE ERROR: One or more nodes r...
   [ERROR]   TestMockPlugin.testSizeLimit » UserRemote EXECUTION_ERROR ERROR: Failure alloc...
   [ERROR] org.apache.drill.exec.work.filter.BloomFilterTest.null
   [ERROR]   Run 1: BloomFilterTest>SubOperatorTest.classSetup:34 » OutOfMemory Failure allocating...
   [ERROR]   Run 2: BloomFilterTest>SubOperatorTest.classTeardown:39 » IllegalState Allocator[ROOT...
   [INFO]
   [ERROR]   TestFillEmpties>SubOperatorTest.classSetup:34 » OutOfMemory Failure allocating...
   [ERROR] org.apache.drill.vector.TestToNullable.null
   [ERROR]   Run 1: TestToNullable>SubOperatorTest.classSetup:34 » OutOfMemory Failure allocating ...
   [ERROR]   Run 2: TestToNullable>SubOperatorTest.classTeardown:39 » IllegalState Allocator[ROOT]...
   [INFO]
   [INFO]
   [ERROR] Tests run: 4921, Failures: 0, Errors: 49, Skipped: 149
   ```


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre commented on pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2200:
URL: https://github.com/apache/drill/pull/2200#issuecomment-841880835


   @vdiravka  Could you do a quick review of this?
   Thx!


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] martin-g commented on pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #2200:
URL: https://github.com/apache/drill/pull/2200#issuecomment-840352034


   But if you decide to stay with 4.x for some more then maybe you should upgrade to 4.13.**2** (https://search.maven.org/artifact/junit/junit/4.13.2/jar). It has been released 3 months ago.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre commented on a change in pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2200:
URL: https://github.com/apache/drill/pull/2200#discussion_r635231706



##########
File path: contrib/storage-kafka/src/test/java/org/apache/drill/exec/store/kafka/KafkaFilterPushdownTest.java
##########
@@ -419,13 +419,13 @@ public void testNoPushdownOfOffsetWithNonMetadataField() throws Exception {
     final int expectedRowCount = 30;
 
     final String queryString = String.format(TestQueryConstants.QUERY_TEMPLATE_AND_OR_PATTERN_2,
-        TestQueryConstants.JSON_PUSHDOWN_TOPIC, predicate1, predicate2, predicate3);
+      TestQueryConstants.JSON_PUSHDOWN_TOPIC, predicate1, predicate2, predicate3);
 
     runKafkaSQLVerifyCount(queryString,expectedRowCount);
     queryBuilder()
-        .sql(queryString)
-        .jsonPlanMatcher()
-        .include(String.format(EXPECTED_PATTERN, expectedRowCountInPlan))
-        .match();
+      .sql(queryString)
+      .jsonPlanMatcher()
+      .include(String.format(EXPECTED_PATTERN, expectedRowCountInPlan))
+      .match();
   }
-}
+}

Review comment:
       @vvysotskyi 
   I put the version of this file from `master` back in this PR, so this **should** be what was originally there.  I didn't modify this file at all, so I'm not sure why the spacing got all messed up in the first place.  I hope that's ok. 




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre merged pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.2

Posted by GitBox <gi...@apache.org>.
cgivre merged pull request #2200:
URL: https://github.com/apache/drill/pull/2200


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] vdiravka commented on pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
vdiravka commented on pull request #2200:
URL: https://github.com/apache/drill/pull/2200#issuecomment-821194726


   I had Junit 5 migration code for Drill in my `shelf`. Let me check it how tests will pass on 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] vvysotskyi commented on a change in pull request #2200: DRILL-7901: Bump junit from 4.12 to 4.13.1

Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2200:
URL: https://github.com/apache/drill/pull/2200#discussion_r633233309



##########
File path: contrib/storage-kafka/src/test/java/org/apache/drill/exec/store/kafka/KafkaFilterPushdownTest.java
##########
@@ -419,13 +419,13 @@ public void testNoPushdownOfOffsetWithNonMetadataField() throws Exception {
     final int expectedRowCount = 30;
 
     final String queryString = String.format(TestQueryConstants.QUERY_TEMPLATE_AND_OR_PATTERN_2,
-        TestQueryConstants.JSON_PUSHDOWN_TOPIC, predicate1, predicate2, predicate3);
+      TestQueryConstants.JSON_PUSHDOWN_TOPIC, predicate1, predicate2, predicate3);
 
     runKafkaSQLVerifyCount(queryString,expectedRowCount);
     queryBuilder()
-        .sql(queryString)
-        .jsonPlanMatcher()
-        .include(String.format(EXPECTED_PATTERN, expectedRowCountInPlan))
-        .match();
+      .sql(queryString)
+      .jsonPlanMatcher()
+      .include(String.format(EXPECTED_PATTERN, expectedRowCountInPlan))
+      .match();
   }
-}
+}

Review comment:
       Please revert formatting changes here and in other places. It is fine to have 4 spaces for the case when moving builder methods or arguments on the new line and having an empty line at the end of the class.

##########
File path: contrib/storage-kafka/pom.xml
##########
@@ -86,6 +86,25 @@
       <version>${project.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>com.101tec</groupId>
+      <artifactId>zkclient</artifactId>
+      <version>0.11</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.kafka</groupId>
+      <artifactId>kafka_2.12</artifactId>
+      <version>${kafka.version}</version>
+      <classifier>test</classifier>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.servicemix.bundles</groupId>
+      <artifactId>org.apache.servicemix.bundles.kafka_2.12</artifactId>
+      <version>2.3.1_1</version>
+      <scope>test</scope>
+    </dependency>

Review comment:
       Could you please revert these changes, since they are not necessary?




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org