You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by GitBox <gi...@apache.org> on 2021/08/09 05:43:29 UTC
[GitHub] [orc] williamhyun opened a new pull request #838: ORC-925: Simplify Assertions
williamhyun opened a new pull request #838:
URL: https://github.com/apache/orc/pull/838
### What changes were proposed in this pull request?
This PR aims to simplify assertion statements in test code.
### Why are the changes needed?
This will improve overall code quality.
### How was this patch tested?
Pass the Cis.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #838: ORC-925: Simplify Assertions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #838:
URL: https://github.com/apache/orc/pull/838#discussion_r685000544
##########
File path: java/core/src/test/org/apache/orc/util/TestOrcUtils.java
##########
@@ -42,8 +44,7 @@ public void testBloomFilterIncludeColumns() {
includeColumns[1] = true;
includeColumns[3] = true;
- assertTrue(Arrays.equals(includeColumns,
- OrcUtils.includeColumns("msisdn, imei", schema)));
+ assertArrayEquals(includeColumns, OrcUtils.includeColumns("msisdn, imei", schema));
Review comment:
Yep, +1 for this change, too.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun merged pull request #838: ORC-925: Simplify Assertions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun merged pull request #838:
URL: https://github.com/apache/orc/pull/838
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #838: ORC-925: Simplify Assertions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #838:
URL: https://github.com/apache/orc/pull/838#issuecomment-895043817
For this one, I'll test and cherry-pick to branch-1.7 for Apache ORC 1.7.0.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #838: ORC-925: Simplify Assertions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #838:
URL: https://github.com/apache/orc/pull/838#discussion_r684996896
##########
File path: java/core/src/test/org/apache/orc/TestRowFilteringSkip.java
##########
@@ -172,10 +172,10 @@ public void testDecimalRepeatingFilterCallback() throws Exception {
int noNullCnt = 0;
while (rows.nextBatch(batch)) {
assertTrue(batch.selectedInUse);
- assertTrue(batch.selected != null);
+ assertNotNull(batch.selected);
Review comment:
This one looks much better.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #838: ORC-925: Simplify Assertions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #838:
URL: https://github.com/apache/orc/pull/838#discussion_r684996028
##########
File path: java/core/src/findbugs/exclude.xml
##########
@@ -56,4 +56,44 @@
<Class name="org.apache.orc.TypeDescription"/>
<Method name="equals" />
</Match>
+ <Match>
+ <Bug pattern="NP_NULL_ON_SOME_PATH"/>
+ <Class name="org.apache.orc.impl.StreamName"/>
+ <Method name="compareTo" />
+ </Match>
+ <Match>
+ <Bug pattern="NP_NULL_ON_SOME_PATH"/>
+ <Class name="org.apache.orc.impl.reader.ReaderEncryptionKey"/>
+ <Method name="compareTo" />
+ </Match>
+ <Match>
+ <Bug pattern="NP_NULL_ON_SOME_PATH"/>
+ <Class name="org.apache.orc.impl.reader.ReaderEncryptionVariant"/>
+ <Method name="compareTo" />
+ </Match>
+ <Match>
+ <Bug pattern="NP_NULL_ON_SOME_PATH"/>
+ <Class name="org.apache.orc.impl.MaskDescriptionImpl"/>
+ <Method name="compareTo" />
+ </Match>
+ <Match>
+ <Bug pattern="NP_NULL_ON_SOME_PATH"/>
+ <Class name="org.apache.orc.impl.TreeReaderFactory$TreeReader"/>
+ <Method name="<init>" />
+ </Match>
+ <Match>
+ <Bug pattern="NP_NULL_ON_SOME_PATH"/>
+ <Class name="org.apache.orc.impl.writer.WriterEncryptionKey"/>
+ <Method name="compareTo" />
+ </Match>
+ <Match>
+ <Bug pattern="NP_NULL_ON_SOME_PATH"/>
+ <Class name="org.apache.orc.impl.writer.WriterEncryptionVariant"/>
+ <Method name="compareTo" />
+ </Match>
+ <Match>
+ <Bug pattern="NP_NULL_PARAM_DEREF"/>
+ <Class name="org.apache.orc.impl.reader.StripePlanner"/>
+ <Method name="planPartialDataReading" />
+ </Match>
Review comment:
Got it. I remember that `findbugs` was confused before in the similar way. For this one, it looks like due to `JetBrain` NotNull annotation.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #838: ORC-925: Simplify Assertions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #838:
URL: https://github.com/apache/orc/pull/838#discussion_r684998075
##########
File path: java/core/src/test/org/apache/orc/TestTypeDescription.java
##########
@@ -269,31 +269,31 @@ public void testBadFindSubtype() {
"g:uniontype<string,int>>");
try {
type.findSubtype("13");
- assertTrue(false);
+ fail();
Review comment:
I agree that `fail` is much clearer than `assertTrue(false)`.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #838: ORC-925: Simplify Assertions
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #838:
URL: https://github.com/apache/orc/pull/838#discussion_r684997268
##########
File path: java/core/src/test/org/apache/orc/TestRowFilteringSkip.java
##########
@@ -614,10 +614,10 @@ public void testDoubleRoundRobbinRowFilterCallback() throws Exception {
assertEquals(NUM_BATCHES * 512, noNullCnt);
assertEquals(0, batch.selected[0]);
assertEquals(2, batch.selected[1]);
- assertTrue(col2.vector[0] == 100.0);
- assertTrue(col2.vector[511] == 0.0);
- assertTrue(col2.vector[1020] == 100);
- assertTrue(col2.vector[1021] == 0);
+ assertEquals(100.0, col2.vector[0]);
+ assertEquals(0.0, col2.vector[511]);
+ assertEquals(100, col2.vector[1020]);
+ assertEquals(0, col2.vector[1021]);
Review comment:
+1 for these changes.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org