Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix leaks in the test cases of CachedBatchWriterSuite #5938

Merged
merged 1 commit into from
Jul 1, 2022
Merged

Fix leaks in the test cases of CachedBatchWriterSuite #5938

merged 1 commit into from
Jul 1, 2022

Conversation

res-life
Copy link
Collaborator

@res-life res-life commented Jun 30, 2022 •
edited
Loading

contributes #5854

Fix leaks in the test cases of CachedBatchWriterSuite

Changes 1

The following spyCol.split is executed actually, ant it creates non-closed cv splits.

when(spyCol.split(any())).thenReturn(splitCols.toArray)

Should be

when(spyCol).split()

See mockito document

Changes 2

Some refactor

Verify

cd spark-rapids
mvn clean install -Dbuildver=330 -DskipTests
cd tests
mvn test -Dbuildver=330 -DwildcardSuites=com.nvidia.spark.rapids.CachedBatchWriterSuite

Signed-off-by: Chong Gao res_life@163.com

Fix leaks in the test cases of CachedBatchWriterSuite
a70de47 
Signed-off-by: Chong Gao <res_life@163.com>
@res-life res-life marked this pull request as ready for review June 30, 2022 10:22
@res-life
Copy link
Collaborator Author

build

@jlowe jlowe added this to the Jun 20 - Jul 8 milestone Jun 30, 2022
@jlowe jlowe added the test Only impacts tests label Jun 30, 2022
@res-life res-life merged commit fff7ae5 into NVIDIA:branch-22.08 Jul 1, 2022
@res-life res-life deleted the fix-CachedBatchWriterSuite branch July 1, 2022 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlowe jlowe jlowe approved these changes

Assignees
No one assigned
Labels
test Only impacts tests
Projects
None yet
Milestone
Jun 20 - Jul 8
Development

Successfully merging this pull request may close these issues.
2 participants
@res-life @jlowe