You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "llama90 (via GitHub)" <gi...@apache.org> on 2024/06/07 15:17:32 UTC

[PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

llama90 opened a new pull request, #42029:
URL: https://github.com/apache/arrow/pull/42029

   <!--
   Thanks for opening a pull request!
   If this is your first pull request you can find detailed information on how 
   to contribute here:
     * [New Contributor's Guide](https://arrow.apache.org/docs/dev/developers/guide/step_by_step/pr_lifecycle.html#reviews-and-merge-of-the-pull-request)
     * [Contributing Overview](https://arrow.apache.org/docs/dev/developers/overview.html)
   
   
   If this is not a [minor PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose
   
   Opening GitHub issues ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
   
   Then could you also rename the pull request title in the following format?
   
       GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
   
   or
   
       MINOR: [${COMPONENT}] ${SUMMARY}
   
   In the case of PARQUET issues on JIRA the title also supports:
   
       PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
   
   -->
   
   ### Rationale for this change
   
   Update package from JUnit 4(`org.junit`) to JUnit 5(`org.junit.jupiter`).
   
   <!--
    Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
    Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.  
   -->
   
   ### What changes are included in this PR?
   
   - [x] Replacing `org.junit` and `junit.framework.TestCase` with `org.junit.jupiter.api`.
   - [x] Updating `Assertions.assertXXX` to `assertXXX` using static imports.
   - [x] Updating annotations such as `@Before`, `@After`.
     - `@Before` -> `@BeforeEach`
     - `@After` -> `@AfterEach`
     - `@Test` -> `@Test` with `org.junit.jupiter`
   - [x] Updating `Parameterized` test
   - [x] Doing self review
   
   <!--
   There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
   -->
   
   ### Are these changes tested?
   
   Yes, existing tests have passed.
   
   <!--
   We typically require tests for all PRs in order to:
   1. Prevent the code from being accidentally broken by subsequent changes
   2. Serve as another way to document the expected behavior of the code
   
   If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
   -->
   
   ### Are there any user-facing changes?
   
   No.
   
   <!--
   If there are user-facing changes then we may require documentation to be updated before approving the PR.
   -->
   
   <!--
   If there are any breaking changes to public APIs, please uncomment the line below and explain which changes are breaking.
   -->
   <!-- **This PR includes breaking changes to public APIs.** -->
   
   <!--
   Please uncomment the line below (and provide explanation) if the changes fix either (a) a security vulnerability, (b) a bug that caused incorrect or invalid data to be produced, or (c) a bug that causes a crash (even when the API contract is upheld). We use this to highlight fixes to issues that may affect users without their knowledge. For this reason, fixing bugs that cause errors don't count, since those are usually obvious.
   -->
   <!-- **This PR contains a "Critical Fix".** -->


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #42029:
URL: https://github.com/apache/arrow/pull/42029#discussion_r1635828456


##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/search/TestParallelSearcher.java:
##########
@@ -48,45 +46,39 @@ private enum ComparatorType {
 
   private static final int VECTOR_LENGTH = 10000;
 
-  private final int threadCount;
-
   private BufferAllocator allocator;
 
   private ExecutorService threadPool;
 
-  private final ComparatorType comparatorType;
-
-  public TestParallelSearcher(ComparatorType comparatorType, int threadCount) {
-    this.comparatorType = comparatorType;
-    this.threadCount = threadCount;
-  }
-
-  @Parameterized.Parameters(name = "comparator type = {0}, thread count = {1}")
-  public static Collection<Object[]> getComparatorName() {
-    List<Object[]> params = new ArrayList<>();
+  public static Stream<Arguments> getComparatorName() {
+    List<Arguments> params = new ArrayList<>();
     int[] threadCounts = {1, 2, 5, 10, 20, 50};
     for (ComparatorType type : ComparatorType.values()) {
       for (int count : threadCounts) {
-        params.add(new Object[] {type, count});
+        params.add(Arguments.of(type, count));
       }
     }
-    return params;
+    return params.stream();
   }
 
-  @Before
+  @BeforeEach
   public void prepare() {
     allocator = new RootAllocator(1024 * 1024);
-    threadPool = Executors.newFixedThreadPool(threadCount);
   }
 
-  @After
+  @AfterEach
   public void shutdown() {
     allocator.close();
-    threadPool.shutdown();
+    if (threadPool != null) {
+      threadPool.shutdown();
+    }
   }
 
-  @Test
-  public void testParallelIntSearch() throws ExecutionException, InterruptedException {
+  @ParameterizedTest
+  @MethodSource("getComparatorName")
+  public void testParallelIntSearch(ComparatorType comparatorType, int threadCount)
+      throws ExecutionException, InterruptedException {
+    threadPool = Executors.newFixedThreadPool(threadCount);

Review Comment:
   It appears https://github.com/junit-team/junit5/issues/878 is the feature request.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #42029:
URL: https://github.com/apache/arrow/pull/42029


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #42029:
URL: https://github.com/apache/arrow/pull/42029#discussion_r1632438639


##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/sort/TestFixedWidthSorting.java:
##########
@@ -31,64 +31,47 @@
 import org.apache.arrow.vector.IntVector;
 import org.apache.arrow.vector.SmallIntVector;
 import org.apache.arrow.vector.TinyIntVector;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 /**
  * Test sorting fixed width vectors with random data.
  */
-@RunWith(Parameterized.class)
 public class TestFixedWidthSorting<V extends BaseFixedWidthVector, U extends Comparable<U>> {
 
   static final int[] VECTOR_LENGTHS = new int[] {2, 5, 10, 50, 100, 1000, 3000};
 
   static final double[] NULL_FRACTIONS = {0, 0.1, 0.3, 0.5, 0.7, 0.9, 1};
 
-  private final int length;
-
-  private final double nullFraction;
-
-  private final boolean inPlace;
-
-  private final Function<BufferAllocator, V> vectorGenerator;
-
-  private final TestSortingUtil.DataGenerator<V, U> dataGenerator;
-
   private BufferAllocator allocator;
 
-  @Before
+  @BeforeEach
   public void prepare() {
     allocator = new RootAllocator(Integer.MAX_VALUE);
   }
 
-  @After
+  @AfterEach
   public void shutdown() {
     allocator.close();
   }
 
-  public TestFixedWidthSorting(
-      int length, double nullFraction, boolean inPlace, String desc,
+  @ParameterizedTest(name = "in place = {0}, length = {1}, null fraction = {2}, desc = {3}, vector = {4}")
+  @MethodSource("getParameters")
+  public void testSort(
+      boolean inPlace, int length, double nullFraction, String desc,

Review Comment:
   You could make it a named parameter set instead though: https://stackoverflow.com/questions/57892989/generating-display-names-for-parameterizedtest-in-junit-5



##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/sort/TestFixedWidthSorting.java:
##########
@@ -31,64 +31,47 @@
 import org.apache.arrow.vector.IntVector;
 import org.apache.arrow.vector.SmallIntVector;
 import org.apache.arrow.vector.TinyIntVector;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 /**
  * Test sorting fixed width vectors with random data.
  */
-@RunWith(Parameterized.class)
 public class TestFixedWidthSorting<V extends BaseFixedWidthVector, U extends Comparable<U>> {
 
   static final int[] VECTOR_LENGTHS = new int[] {2, 5, 10, 50, 100, 1000, 3000};
 
   static final double[] NULL_FRACTIONS = {0, 0.1, 0.3, 0.5, 0.7, 0.9, 1};
 
-  private final int length;
-
-  private final double nullFraction;
-
-  private final boolean inPlace;
-
-  private final Function<BufferAllocator, V> vectorGenerator;
-
-  private final TestSortingUtil.DataGenerator<V, U> dataGenerator;
-
   private BufferAllocator allocator;
 
-  @Before
+  @BeforeEach
   public void prepare() {
     allocator = new RootAllocator(Integer.MAX_VALUE);
   }
 
-  @After
+  @AfterEach
   public void shutdown() {
     allocator.close();
   }
 
-  public TestFixedWidthSorting(
-      int length, double nullFraction, boolean inPlace, String desc,
+  @ParameterizedTest(name = "in place = {0}, length = {1}, null fraction = {2}, desc = {3}, vector = {4}")
+  @MethodSource("getParameters")
+  public void testSort(
+      boolean inPlace, int length, double nullFraction, String desc,

Review Comment:
   I would like to keep 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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "llama90 (via GitHub)" <gi...@apache.org>.
llama90 commented on code in PR #42029:
URL: https://github.com/apache/arrow/pull/42029#discussion_r1631804163


##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/sort/TestFixedWidthSorting.java:
##########
@@ -31,64 +31,47 @@
 import org.apache.arrow.vector.IntVector;
 import org.apache.arrow.vector.SmallIntVector;
 import org.apache.arrow.vector.TinyIntVector;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 /**
  * Test sorting fixed width vectors with random data.
  */
-@RunWith(Parameterized.class)
 public class TestFixedWidthSorting<V extends BaseFixedWidthVector, U extends Comparable<U>> {
 
   static final int[] VECTOR_LENGTHS = new int[] {2, 5, 10, 50, 100, 1000, 3000};
 
   static final double[] NULL_FRACTIONS = {0, 0.1, 0.3, 0.5, 0.7, 0.9, 1};
 
-  private final int length;
-
-  private final double nullFraction;
-
-  private final boolean inPlace;
-
-  private final Function<BufferAllocator, V> vectorGenerator;
-
-  private final TestSortingUtil.DataGenerator<V, U> dataGenerator;
-
   private BufferAllocator allocator;
 
-  @Before
+  @BeforeEach
   public void prepare() {
     allocator = new RootAllocator(Integer.MAX_VALUE);
   }
 
-  @After
+  @AfterEach
   public void shutdown() {
     allocator.close();
   }
 
-  public TestFixedWidthSorting(
-      int length, double nullFraction, boolean inPlace, String desc,
+  @ParameterizedTest(name = "in place = {0}, length = {1}, null fraction = {2}, desc = {3}, vector = {4}")
+  @MethodSource("getParameters")
+  public void testSort(
+      boolean inPlace, int length, double nullFraction, String desc,

Review Comment:
   I saw a warning message like this: `Class 'ComparatorType' is exposed outside its defined visibility scope` from the IDE. We can actually ignore this message.
   
   I rolled back for consistency.
   



##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/search/TestParallelSearcher.java:
##########
@@ -21,78 +21,71 @@
 
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.stream.Stream;
 
 import org.apache.arrow.algorithm.sort.DefaultVectorComparators;
 import org.apache.arrow.algorithm.sort.VectorValueComparator;
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.memory.RootAllocator;
 import org.apache.arrow.vector.IntVector;
 import org.apache.arrow.vector.VarCharVector;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 /**
  * Test cases for {@link ParallelSearcher}.
  */
-@RunWith(Parameterized.class)
 public class TestParallelSearcher {
 
-  private enum ComparatorType {
+  public enum ComparatorType {

Review Comment:
   I saw a warning message like this: `Class 'ComparatorType' is exposed outside its defined visibility scope` from the IDE. We can actually ignore this message.
   
   I rolled back for consistency.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42029:
URL: https://github.com/apache/arrow/pull/42029#issuecomment-2155051648

   :warning: GitHub issue #42025 **has been automatically assigned in GitHub** to PR creator.


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #42029:
URL: https://github.com/apache/arrow/pull/42029#discussion_r1631746493


##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/sort/TestFixedWidthSorting.java:
##########
@@ -31,64 +31,47 @@
 import org.apache.arrow.vector.IntVector;
 import org.apache.arrow.vector.SmallIntVector;
 import org.apache.arrow.vector.TinyIntVector;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 /**
  * Test sorting fixed width vectors with random data.
  */
-@RunWith(Parameterized.class)
 public class TestFixedWidthSorting<V extends BaseFixedWidthVector, U extends Comparable<U>> {
 
   static final int[] VECTOR_LENGTHS = new int[] {2, 5, 10, 50, 100, 1000, 3000};
 
   static final double[] NULL_FRACTIONS = {0, 0.1, 0.3, 0.5, 0.7, 0.9, 1};
 
-  private final int length;
-
-  private final double nullFraction;
-
-  private final boolean inPlace;
-
-  private final Function<BufferAllocator, V> vectorGenerator;
-
-  private final TestSortingUtil.DataGenerator<V, U> dataGenerator;
-
   private BufferAllocator allocator;
 
-  @Before
+  @BeforeEach
   public void prepare() {
     allocator = new RootAllocator(Integer.MAX_VALUE);
   }
 
-  @After
+  @AfterEach
   public void shutdown() {
     allocator.close();
   }
 
-  public TestFixedWidthSorting(
-      int length, double nullFraction, boolean inPlace, String desc,
+  @ParameterizedTest(name = "in place = {0}, length = {1}, null fraction = {2}, desc = {3}, vector = {4}")
+  @MethodSource("getParameters")
+  public void testSort(
+      boolean inPlace, int length, double nullFraction, String desc,

Review Comment:
   Seems like it is not being used. IDE should give a hint? 



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "llama90 (via GitHub)" <gi...@apache.org>.
llama90 commented on code in PR #42029:
URL: https://github.com/apache/arrow/pull/42029#discussion_r1631804163


##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/sort/TestFixedWidthSorting.java:
##########
@@ -31,64 +31,47 @@
 import org.apache.arrow.vector.IntVector;
 import org.apache.arrow.vector.SmallIntVector;
 import org.apache.arrow.vector.TinyIntVector;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 /**
  * Test sorting fixed width vectors with random data.
  */
-@RunWith(Parameterized.class)
 public class TestFixedWidthSorting<V extends BaseFixedWidthVector, U extends Comparable<U>> {
 
   static final int[] VECTOR_LENGTHS = new int[] {2, 5, 10, 50, 100, 1000, 3000};
 
   static final double[] NULL_FRACTIONS = {0, 0.1, 0.3, 0.5, 0.7, 0.9, 1};
 
-  private final int length;
-
-  private final double nullFraction;
-
-  private final boolean inPlace;
-
-  private final Function<BufferAllocator, V> vectorGenerator;
-
-  private final TestSortingUtil.DataGenerator<V, U> dataGenerator;
-
   private BufferAllocator allocator;
 
-  @Before
+  @BeforeEach
   public void prepare() {
     allocator = new RootAllocator(Integer.MAX_VALUE);
   }
 
-  @After
+  @AfterEach
   public void shutdown() {
     allocator.close();
   }
 
-  public TestFixedWidthSorting(
-      int length, double nullFraction, boolean inPlace, String desc,
+  @ParameterizedTest(name = "in place = {0}, length = {1}, null fraction = {2}, desc = {3}, vector = {4}")
+  @MethodSource("getParameters")
+  public void testSort(
+      boolean inPlace, int length, double nullFraction, String desc,

Review Comment:
   I saw a warning message like this: `Class 'ComparatorType' is exposed outside its defined visibility scope` from the IDE. We can actually ignore this message.
   
   I rolled back for consistency.
   



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #42029:
URL: https://github.com/apache/arrow/pull/42029#discussion_r1631807795


##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/sort/TestFixedWidthSorting.java:
##########
@@ -31,64 +31,47 @@
 import org.apache.arrow.vector.IntVector;
 import org.apache.arrow.vector.SmallIntVector;
 import org.apache.arrow.vector.TinyIntVector;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 /**
  * Test sorting fixed width vectors with random data.
  */
-@RunWith(Parameterized.class)
 public class TestFixedWidthSorting<V extends BaseFixedWidthVector, U extends Comparable<U>> {
 
   static final int[] VECTOR_LENGTHS = new int[] {2, 5, 10, 50, 100, 1000, 3000};
 
   static final double[] NULL_FRACTIONS = {0, 0.1, 0.3, 0.5, 0.7, 0.9, 1};
 
-  private final int length;
-
-  private final double nullFraction;
-
-  private final boolean inPlace;
-
-  private final Function<BufferAllocator, V> vectorGenerator;
-
-  private final TestSortingUtil.DataGenerator<V, U> dataGenerator;
-
   private BufferAllocator allocator;
 
-  @Before
+  @BeforeEach
   public void prepare() {
     allocator = new RootAllocator(Integer.MAX_VALUE);
   }
 
-  @After
+  @AfterEach
   public void shutdown() {
     allocator.close();
   }
 
-  public TestFixedWidthSorting(
-      int length, double nullFraction, boolean inPlace, String desc,
+  @ParameterizedTest(name = "in place = {0}, length = {1}, null fraction = {2}, desc = {3}, vector = {4}")
+  @MethodSource("getParameters")
+  public void testSort(
+      boolean inPlace, int length, double nullFraction, String desc,

Review Comment:
   I think if it is not used we can remove it for the moment.
   
   cc @lidavidm 



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "llama90 (via GitHub)" <gi...@apache.org>.
llama90 commented on code in PR #42029:
URL: https://github.com/apache/arrow/pull/42029#discussion_r1633208511


##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/search/TestParallelSearcher.java:
##########
@@ -48,45 +46,39 @@ private enum ComparatorType {
 
   private static final int VECTOR_LENGTH = 10000;
 
-  private final int threadCount;
-
   private BufferAllocator allocator;
 
   private ExecutorService threadPool;
 
-  private final ComparatorType comparatorType;
-
-  public TestParallelSearcher(ComparatorType comparatorType, int threadCount) {
-    this.comparatorType = comparatorType;
-    this.threadCount = threadCount;
-  }
-
-  @Parameterized.Parameters(name = "comparator type = {0}, thread count = {1}")
-  public static Collection<Object[]> getComparatorName() {
-    List<Object[]> params = new ArrayList<>();
+  public static Stream<Arguments> getComparatorName() {
+    List<Arguments> params = new ArrayList<>();
     int[] threadCounts = {1, 2, 5, 10, 20, 50};
     for (ComparatorType type : ComparatorType.values()) {
       for (int count : threadCounts) {
-        params.add(new Object[] {type, count});
+        params.add(Arguments.of(type, count));
       }
     }
-    return params;
+    return params.stream();
   }
 
-  @Before
+  @BeforeEach
   public void prepare() {
     allocator = new RootAllocator(1024 * 1024);
-    threadPool = Executors.newFixedThreadPool(threadCount);
   }
 
-  @After
+  @AfterEach
   public void shutdown() {
     allocator.close();
-    threadPool.shutdown();
+    if (threadPool != null) {
+      threadPool.shutdown();
+    }
   }
 
-  @Test
-  public void testParallelIntSearch() throws ExecutionException, InterruptedException {
+  @ParameterizedTest
+  @MethodSource("getComparatorName")
+  public void testParallelIntSearch(ComparatorType comparatorType, int threadCount)
+      throws ExecutionException, InterruptedException {
+    threadPool = Executors.newFixedThreadPool(threadCount);

Review Comment:
   @vibhatha I think it is similar to `initializeDatabase` in [PR](https://github.com/apache/arrow/pull/42038).
   
   ### JUnit 4 Case
   
   `constructor(int threadCount)` -> `@Before` -> `@Test`
   
   - the `threadCount` is changed in the constructor.
   
   ### JUnit 5 Case
   
   `@BeforeEach` -> `@ParameterizedTest`
   
   - The `threadCount` changed when calling the test.
   
   That's why I initialized `threadPool` like `initializeDatabase`.
   
   We need to consider the sequence of function calls to ensure that the `threadCount` is correctly set before initializing the `threadPool`.
   



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #42029:
URL: https://github.com/apache/arrow/pull/42029#issuecomment-2164295766

   After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 23e1f4c2db3501173d1e9b3a77bdc36b4c7b5bf4.
   
   There were no benchmark performance regressions. 🎉
   
   The [full Conbench report](https://github.com/apache/arrow/runs/26161747032) has more details. It also includes information about 8 possible false positives for unstable benchmarks that are known to sometimes produce them.


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "llama90 (via GitHub)" <gi...@apache.org>.
llama90 commented on code in PR #42029:
URL: https://github.com/apache/arrow/pull/42029#discussion_r1633208511


##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/search/TestParallelSearcher.java:
##########
@@ -48,45 +46,39 @@ private enum ComparatorType {
 
   private static final int VECTOR_LENGTH = 10000;
 
-  private final int threadCount;
-
   private BufferAllocator allocator;
 
   private ExecutorService threadPool;
 
-  private final ComparatorType comparatorType;
-
-  public TestParallelSearcher(ComparatorType comparatorType, int threadCount) {
-    this.comparatorType = comparatorType;
-    this.threadCount = threadCount;
-  }
-
-  @Parameterized.Parameters(name = "comparator type = {0}, thread count = {1}")
-  public static Collection<Object[]> getComparatorName() {
-    List<Object[]> params = new ArrayList<>();
+  public static Stream<Arguments> getComparatorName() {
+    List<Arguments> params = new ArrayList<>();
     int[] threadCounts = {1, 2, 5, 10, 20, 50};
     for (ComparatorType type : ComparatorType.values()) {
       for (int count : threadCounts) {
-        params.add(new Object[] {type, count});
+        params.add(Arguments.of(type, count));
       }
     }
-    return params;
+    return params.stream();
   }
 
-  @Before
+  @BeforeEach
   public void prepare() {
     allocator = new RootAllocator(1024 * 1024);
-    threadPool = Executors.newFixedThreadPool(threadCount);
   }
 
-  @After
+  @AfterEach
   public void shutdown() {
     allocator.close();
-    threadPool.shutdown();
+    if (threadPool != null) {
+      threadPool.shutdown();
+    }
   }
 
-  @Test
-  public void testParallelIntSearch() throws ExecutionException, InterruptedException {
+  @ParameterizedTest
+  @MethodSource("getComparatorName")
+  public void testParallelIntSearch(ComparatorType comparatorType, int threadCount)
+      throws ExecutionException, InterruptedException {
+    threadPool = Executors.newFixedThreadPool(threadCount);

Review Comment:
   @vibhatha I think it is similar to `initializeDatabase` in [PR](https://github.com/apache/arrow/pull/42038).
   
   ### JUnit 4 Case
   
   `constructor(int threadCount)` -> `@Before` -> `@Test`
   
   - The `threadCount` is changed in the constructor.
   
   ### JUnit 5 Case
   
   `@BeforeEach` -> `@ParameterizedTest`
   
   - The `threadCount` changed when calling the test.
   
   That's why I initialized `threadPool` like `initializeDatabase`.
   
   We need to consider the sequence of function calls to ensure that the `threadCount` is correctly set before initializing the `threadPool`.
   



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #42029:
URL: https://github.com/apache/arrow/pull/42029#discussion_r1631744463


##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/search/TestParallelSearcher.java:
##########
@@ -21,78 +21,71 @@
 
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.stream.Stream;
 
 import org.apache.arrow.algorithm.sort.DefaultVectorComparators;
 import org.apache.arrow.algorithm.sort.VectorValueComparator;
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.memory.RootAllocator;
 import org.apache.arrow.vector.IntVector;
 import org.apache.arrow.vector.VarCharVector;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 /**
  * Test cases for {@link ParallelSearcher}.
  */
-@RunWith(Parameterized.class)
 public class TestParallelSearcher {
 
-  private enum ComparatorType {
+  public enum ComparatorType {

Review Comment:
   does this need to be Public? 



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #42029:
URL: https://github.com/apache/arrow/pull/42029#discussion_r1632439718


##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/sort/TestFixedWidthSorting.java:
##########
@@ -31,64 +31,47 @@
 import org.apache.arrow.vector.IntVector;
 import org.apache.arrow.vector.SmallIntVector;
 import org.apache.arrow.vector.TinyIntVector;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 /**
  * Test sorting fixed width vectors with random data.
  */
-@RunWith(Parameterized.class)
 public class TestFixedWidthSorting<V extends BaseFixedWidthVector, U extends Comparable<U>> {
 
   static final int[] VECTOR_LENGTHS = new int[] {2, 5, 10, 50, 100, 1000, 3000};
 
   static final double[] NULL_FRACTIONS = {0, 0.1, 0.3, 0.5, 0.7, 0.9, 1};
 
-  private final int length;
-
-  private final double nullFraction;
-
-  private final boolean inPlace;
-
-  private final Function<BufferAllocator, V> vectorGenerator;
-
-  private final TestSortingUtil.DataGenerator<V, U> dataGenerator;
-
   private BufferAllocator allocator;
 
-  @Before
+  @BeforeEach
   public void prepare() {
     allocator = new RootAllocator(Integer.MAX_VALUE);
   }
 
-  @After
+  @AfterEach
   public void shutdown() {
     allocator.close();
   }
 
-  public TestFixedWidthSorting(
-      int length, double nullFraction, boolean inPlace, String desc,
+  @ParameterizedTest(name = "in place = {0}, length = {1}, null fraction = {2}, desc = {3}, vector = {4}")
+  @MethodSource("getParameters")
+  public void testSort(
+      boolean inPlace, int length, double nullFraction, String desc,

Review Comment:
   Yes. We are already at 5.10.2 JUNIT Jupyter dependency. I think that's 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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "llama90 (via GitHub)" <gi...@apache.org>.
llama90 commented on code in PR #42029:
URL: https://github.com/apache/arrow/pull/42029#discussion_r1633119650


##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/sort/TestFixedWidthSorting.java:
##########
@@ -31,64 +31,47 @@
 import org.apache.arrow.vector.IntVector;
 import org.apache.arrow.vector.SmallIntVector;
 import org.apache.arrow.vector.TinyIntVector;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 /**
  * Test sorting fixed width vectors with random data.
  */
-@RunWith(Parameterized.class)
 public class TestFixedWidthSorting<V extends BaseFixedWidthVector, U extends Comparable<U>> {
 
   static final int[] VECTOR_LENGTHS = new int[] {2, 5, 10, 50, 100, 1000, 3000};
 
   static final double[] NULL_FRACTIONS = {0, 0.1, 0.3, 0.5, 0.7, 0.9, 1};
 
-  private final int length;
-
-  private final double nullFraction;
-
-  private final boolean inPlace;
-
-  private final Function<BufferAllocator, V> vectorGenerator;
-
-  private final TestSortingUtil.DataGenerator<V, U> dataGenerator;
-
   private BufferAllocator allocator;
 
-  @Before
+  @BeforeEach
   public void prepare() {
     allocator = new RootAllocator(Integer.MAX_VALUE);
   }
 
-  @After
+  @AfterEach
   public void shutdown() {
     allocator.close();
   }
 
-  public TestFixedWidthSorting(
-      int length, double nullFraction, boolean inPlace, String desc,
+  @ParameterizedTest(name = "in place = {0}, length = {1}, null fraction = {2}, desc = {3}, vector = {4}")
+  @MethodSource("getParameters")
+  public void testSort(
+      boolean inPlace, int length, double nullFraction, String desc,

Review Comment:
   Thank you for your review.
   
   I checked what you said, and it looks like the current code needs to use `argumentSet` to take multiple arguments for each test. This is only available in JUnit `5.11.0` or higher.
   
   So, it seems like it's hard to use Named Parameters right now. Did I understand that correctly?
   
   Additionally, I am working on resolving the conflict. Sorry for the delay; it's because of my main job.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "llama90 (via GitHub)" <gi...@apache.org>.
llama90 commented on PR #42029:
URL: https://github.com/apache/arrow/pull/42029#issuecomment-2155081597

   Thank you for your review. You are very fast 🫢. 
   
   I would like you to focus on the `Parameterized Test` in your review. Additionally, the test involves inheritance for the parameterized setup.
   
   Here is a list of `Parameterized Test` items.
   
   - src/test/java/org/apache/arrow/algorithm/search/TestParallelSearcher.java
   - src/test/java/org/apache/arrow/algorithm/search/TestVectorRangeSearcher.java
   - src/test/java/org/apache/arrow/algorithm/sort/TestFixedWidthSorting.java
   - src/test/java/org/apache/arrow/algorithm/sort/TestVariableWidthSorting.java
   
   #### Inheritance
   
   - src/test/java/org/apache/arrow/algorithm/sort/TestOutOfPlaceVectorSorter.java
     - src/test/java/org/apache/arrow/algorithm/sort/TestFixedWidthOutOfPlaceVectorSorter.java
     - src/test/java/org/apache/arrow/algorithm/sort/TestVariableWidthOutOfPlaceVectorSorter.java
   


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #42029:
URL: https://github.com/apache/arrow/pull/42029#discussion_r1633125244


##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/sort/TestFixedWidthSorting.java:
##########
@@ -31,64 +31,47 @@
 import org.apache.arrow.vector.IntVector;
 import org.apache.arrow.vector.SmallIntVector;
 import org.apache.arrow.vector.TinyIntVector;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 /**
  * Test sorting fixed width vectors with random data.
  */
-@RunWith(Parameterized.class)
 public class TestFixedWidthSorting<V extends BaseFixedWidthVector, U extends Comparable<U>> {
 
   static final int[] VECTOR_LENGTHS = new int[] {2, 5, 10, 50, 100, 1000, 3000};
 
   static final double[] NULL_FRACTIONS = {0, 0.1, 0.3, 0.5, 0.7, 0.9, 1};
 
-  private final int length;
-
-  private final double nullFraction;
-
-  private final boolean inPlace;
-
-  private final Function<BufferAllocator, V> vectorGenerator;
-
-  private final TestSortingUtil.DataGenerator<V, U> dataGenerator;
-
   private BufferAllocator allocator;
 
-  @Before
+  @BeforeEach
   public void prepare() {
     allocator = new RootAllocator(Integer.MAX_VALUE);
   }
 
-  @After
+  @AfterEach
   public void shutdown() {
     allocator.close();
   }
 
-  public TestFixedWidthSorting(
-      int length, double nullFraction, boolean inPlace, String desc,
+  @ParameterizedTest(name = "in place = {0}, length = {1}, null fraction = {2}, desc = {3}, vector = {4}")
+  @MethodSource("getParameters")
+  public void testSort(
+      boolean inPlace, int length, double nullFraction, String desc,

Review Comment:
   @llama90 Please take your time, no rush in our end. 
   
   I thought that feature is available in 5.10.2 merely referring to what is mentioned in the stackoverflow thread. 



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "llama90 (via GitHub)" <gi...@apache.org>.
llama90 commented on code in PR #42029:
URL: https://github.com/apache/arrow/pull/42029#discussion_r1633208511


##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/search/TestParallelSearcher.java:
##########
@@ -48,45 +46,39 @@ private enum ComparatorType {
 
   private static final int VECTOR_LENGTH = 10000;
 
-  private final int threadCount;
-
   private BufferAllocator allocator;
 
   private ExecutorService threadPool;
 
-  private final ComparatorType comparatorType;
-
-  public TestParallelSearcher(ComparatorType comparatorType, int threadCount) {
-    this.comparatorType = comparatorType;
-    this.threadCount = threadCount;
-  }
-
-  @Parameterized.Parameters(name = "comparator type = {0}, thread count = {1}")
-  public static Collection<Object[]> getComparatorName() {
-    List<Object[]> params = new ArrayList<>();
+  public static Stream<Arguments> getComparatorName() {
+    List<Arguments> params = new ArrayList<>();
     int[] threadCounts = {1, 2, 5, 10, 20, 50};
     for (ComparatorType type : ComparatorType.values()) {
       for (int count : threadCounts) {
-        params.add(new Object[] {type, count});
+        params.add(Arguments.of(type, count));
       }
     }
-    return params;
+    return params.stream();
   }
 
-  @Before
+  @BeforeEach
   public void prepare() {
     allocator = new RootAllocator(1024 * 1024);
-    threadPool = Executors.newFixedThreadPool(threadCount);
   }
 
-  @After
+  @AfterEach
   public void shutdown() {
     allocator.close();
-    threadPool.shutdown();
+    if (threadPool != null) {
+      threadPool.shutdown();
+    }
   }
 
-  @Test
-  public void testParallelIntSearch() throws ExecutionException, InterruptedException {
+  @ParameterizedTest
+  @MethodSource("getComparatorName")
+  public void testParallelIntSearch(ComparatorType comparatorType, int threadCount)
+      throws ExecutionException, InterruptedException {
+    threadPool = Executors.newFixedThreadPool(threadCount);

Review Comment:
   @vibhatha I think it is similar to `initializeDatabase` in [PR](https://github.com/apache/arrow/pull/42038).
   
   ### JUnit 4 Case
   
   `constructor(int threadCount)` -> `@Before` -> `@Test`
   
   - The `threadCount` is changed in the constructor.
   
   ### JUnit 5 Case
   
   `@BeforeEach` -> `@ParameterizedTest`
   
   - The `threadCount` changed when calling the test.
   
   That's why I initialized `threadPool` like `initializeDatabase`.
   
   We need to consider the sequence of function calls to ensure that the `threadCount` is correctly set before initializing the `threadPool`.
   
   UPDATE: 
   
   In JUnit 5, instead of using constructors, parameters are directly injected into test methods.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "llama90 (via GitHub)" <gi...@apache.org>.
llama90 commented on code in PR #42029:
URL: https://github.com/apache/arrow/pull/42029#discussion_r1631804997


##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/sort/TestFixedWidthSorting.java:
##########
@@ -31,64 +31,47 @@
 import org.apache.arrow.vector.IntVector;
 import org.apache.arrow.vector.SmallIntVector;
 import org.apache.arrow.vector.TinyIntVector;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 /**
  * Test sorting fixed width vectors with random data.
  */
-@RunWith(Parameterized.class)
 public class TestFixedWidthSorting<V extends BaseFixedWidthVector, U extends Comparable<U>> {
 
   static final int[] VECTOR_LENGTHS = new int[] {2, 5, 10, 50, 100, 1000, 3000};
 
   static final double[] NULL_FRACTIONS = {0, 0.1, 0.3, 0.5, 0.7, 0.9, 1};
 
-  private final int length;
-
-  private final double nullFraction;
-
-  private final boolean inPlace;
-
-  private final Function<BufferAllocator, V> vectorGenerator;
-
-  private final TestSortingUtil.DataGenerator<V, U> dataGenerator;
-
   private BufferAllocator allocator;
 
-  @Before
+  @BeforeEach
   public void prepare() {
     allocator = new RootAllocator(Integer.MAX_VALUE);
   }
 
-  @After
+  @AfterEach
   public void shutdown() {
     allocator.close();
   }
 
-  public TestFixedWidthSorting(
-      int length, double nullFraction, boolean inPlace, String desc,
+  @ParameterizedTest(name = "in place = {0}, length = {1}, null fraction = {2}, desc = {3}, vector = {4}")
+  @MethodSource("getParameters")
+  public void testSort(
+      boolean inPlace, int length, double nullFraction, String desc,

Review Comment:
   Yes, the IDE gives a hint that `Parameter 'desc' is never used`. 
   
   However, I feel that `desc` might add some clarity. I'm not entirely sure whether it should be removed.
   
   ```java
             params.add(Arguments.of(
                 inPlace, length, nullFrac, "TinyIntVector",
                 (Function<BufferAllocator, TinyIntVector>) allocator -> new TinyIntVector("vector", allocator),
                 TestSortingUtil.TINY_INT_GENERATOR
             ));
   
             params.add(Arguments.of(
                 inPlace, length, nullFrac, "SmallIntVector",
                 (Function<BufferAllocator, SmallIntVector>) allocator -> new SmallIntVector("vector", allocator),
                 TestSortingUtil.SMALL_INT_GENERATOR
             ));
   
             params.add(Arguments.of(
                 inPlace, length, nullFrac, "IntVector",
                 (Function<BufferAllocator, IntVector>) allocator -> new IntVector("vector", allocator),
                 TestSortingUtil.INT_GENERATOR
             ));
   
             params.add(Arguments.of(
                 inPlace, length, nullFrac, "BigIntVector",
                 (Function<BufferAllocator, BigIntVector>) allocator -> new BigIntVector("vector", allocator),
                 TestSortingUtil.LONG_GENERATOR
             ));
   
             params.add(Arguments.of(
                 inPlace, length, nullFrac, "Float4Vector",
                 (Function<BufferAllocator, Float4Vector>) allocator -> new Float4Vector("vector", allocator),
                 TestSortingUtil.FLOAT_GENERATOR
             ));
   
             params.add(Arguments.of(
                 inPlace, length, nullFrac, "Float8Vector",
                 (Function<BufferAllocator, Float8Vector>) allocator -> new Float8Vector("vector", allocator),
                 TestSortingUtil.DOUBLE_GENERATOR
             ));
   
   ```



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on PR #42029:
URL: https://github.com/apache/arrow/pull/42029#issuecomment-2155071811

   Will review this. Thanks. 


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] GH-42025: [Java] Update Unit Tests for Algorithm Module [arrow]

Posted by "llama90 (via GitHub)" <gi...@apache.org>.
llama90 commented on code in PR #42029:
URL: https://github.com/apache/arrow/pull/42029#discussion_r1631396216


##########
java/algorithm/src/test/java/org/apache/arrow/algorithm/sort/TestFixedWidthSorting.java:
##########
@@ -31,64 +31,47 @@
 import org.apache.arrow.vector.IntVector;
 import org.apache.arrow.vector.SmallIntVector;
 import org.apache.arrow.vector.TinyIntVector;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 /**
  * Test sorting fixed width vectors with random data.
  */
-@RunWith(Parameterized.class)
 public class TestFixedWidthSorting<V extends BaseFixedWidthVector, U extends Comparable<U>> {
 
   static final int[] VECTOR_LENGTHS = new int[] {2, 5, 10, 50, 100, 1000, 3000};
 
   static final double[] NULL_FRACTIONS = {0, 0.1, 0.3, 0.5, 0.7, 0.9, 1};
 
-  private final int length;
-
-  private final double nullFraction;
-
-  private final boolean inPlace;
-
-  private final Function<BufferAllocator, V> vectorGenerator;
-
-  private final TestSortingUtil.DataGenerator<V, U> dataGenerator;
-
   private BufferAllocator allocator;
 
-  @Before
+  @BeforeEach
   public void prepare() {
     allocator = new RootAllocator(Integer.MAX_VALUE);
   }
 
-  @After
+  @AfterEach
   public void shutdown() {
     allocator.close();
   }
 
-  public TestFixedWidthSorting(
-      int length, double nullFraction, boolean inPlace, String desc,
+  @ParameterizedTest(name = "in place = {0}, length = {1}, null fraction = {2}, desc = {3}, vector = {4}")
+  @MethodSource("getParameters")
+  public void testSort(
+      boolean inPlace, int length, double nullFraction, String desc,

Review Comment:
   Is the `desc` really needed?
   
   Also, `TestVariableWidthSorting.java` is similar.



-- 
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: github-unsubscribe@arrow.apache.org

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