Skip to content

ARROW-1331: [JAVA] Refactor unit tests#955

Closed
siddharthteotia wants to merge 2 commits intoapache:masterfrom
siddharthteotia:ARROW-1331
Closed

ARROW-1331: [JAVA] Refactor unit tests#955
siddharthteotia wants to merge 2 commits intoapache:masterfrom
siddharthteotia:ARROW-1331

Conversation

@siddharthteotia
Copy link
Copy Markdown
Contributor

[cc @jacques-n , @StevenMPhillips ]

Refactored the java-vector unit tests in TestValueVector.java

(1) Moved all the BitVector related tests to TestBitVector.java
(2) Enhanced and added the tests for different vector types and grouped them into different categories in TestValueVector.java
(3) Moved https://github.com/dremio/dremio-oss/blob/master/sabot/kernel/src/test/java/com/dremio/exec/vector/TestSplitAndTransfer.java to arrow. This currently has splitAndTransfer() test for NullableVarCharVector.
(4) No tests have been removed -- they have either been enhanced or moved around for grouping.

Recently we added splitAndTransfer tests for BitVector, UnionVector and ListVector in their respective test files. We should ideally move all the splitAndTransfer() related tests to TestSplitAndTransfer.java and should try to cover other vector types as well. A follow-up will address this (I want to keep the patch size small for easy review).

@siddharthteotia siddharthteotia changed the title ARROW-1331: Refactor unit tests ARROW-1331: [JAVA] Refactor unit tests Aug 9, 2017
@siddharthteotia
Copy link
Copy Markdown
Contributor Author

Unit tests should not check for 0 values in fixed width vectors at positions where no data has been set. No assumption can be made about the value -- it is garbage.

@siddharthteotia
Copy link
Copy Markdown
Contributor Author

Does this look good?

@siddharthteotia
Copy link
Copy Markdown
Contributor Author

Can this be merged?

@wesm
Copy link
Copy Markdown
Member

wesm commented Aug 11, 2017

@icexelloss @jacques-n @StevenMPhillips can you take a look?

I would probably wait until after the 0.6.0 release vote is completed to merge more patches

@icexelloss
Copy link
Copy Markdown
Contributor

I scan through the changes the it looks good. But this is particular hard to review line by line.

@siddharthteotia, is there anything in particular you want to be reviewed?

@jacques-n
Copy link
Copy Markdown
Contributor

LGTM +1

@asfgit asfgit closed this in 142f74e Aug 14, 2017
asfgit pushed a commit that referenced this pull request Sep 12, 2017
TestSplitAndTransfer was merged as part of #955
However, the class did not have the package statement.

This small patch fixes that.

Author: siddharth <siddharth@dremio.com>

Closes #1088 from siddharthteotia/ARROW-1331-fix and squashes the following commits:

78882b5 [siddharth] ARROW-1331: include package statement
pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
[cc @jacques-n , @StevenMPhillips ]

Refactored the java-vector unit tests in TestValueVector.java

(1) Moved all the BitVector related tests to TestBitVector.java
(2) Enhanced and added the tests for different vector types and grouped them into different categories in TestValueVector.java
(3) Moved https://github.com/dremio/dremio-oss/blob/master/sabot/kernel/src/test/java/com/dremio/exec/vector/TestSplitAndTransfer.java to arrow. This currently has splitAndTransfer() test for NullableVarCharVector.
(4) No tests have been removed -- they have either been enhanced or moved around for grouping.

Recently we added splitAndTransfer tests for BitVector, UnionVector and ListVector in their respective test files.  We should ideally move all the splitAndTransfer() related tests to TestSplitAndTransfer.java and should try to cover other vector types as well. A follow-up will address this (I want to keep the patch size small for easy review).

Author: siddharth <siddharth@dremio.com>

Closes apache#955 from siddharthteotia/ARROW-1331 and squashes the following commits:

ead5cef [siddharth] ARROW-1331: Fixed unit tests
2174e65 [siddharth] ARROW-1331: Refactor unit tests
pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
TestSplitAndTransfer was merged as part of apache#955
However, the class did not have the package statement.

This small patch fixes that.

Author: siddharth <siddharth@dremio.com>

Closes apache#1088 from siddharthteotia/ARROW-1331-fix and squashes the following commits:

78882b5 [siddharth] ARROW-1331: include package statement
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants