[GitHub] [arrow] rymurr commented on a change in pull request #7100: ARROW-8696: [Java] Convert tests to maven failsafe
rymurr commented on a change in pull request #7100: URL: https://github.com/apache/arrow/pull/7100#discussion_r422988106 ## File path: java/memory/pom.xml ## @@ -46,4 +46,37 @@ + + + Review comment: Thanks for the careful read through! These have been fixed. 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: us...@infra.apache.org
[GitHub] [arrow] rymurr commented on a change in pull request #7100: ARROW-8696: [Java] Convert tests to maven failsafe
rymurr commented on a change in pull request #7100:
URL: https://github.com/apache/arrow/pull/7100#discussion_r422081471
##
File path:
java/memory/src/test/java/org/apache/arrow/memory/ITTestLargeArrowBuf.java
##
@@ -27,38 +32,45 @@
* main method.
*
*/
-public class TestLargeArrowBuf {
+public class ITTestLargeArrowBuf {
+ private static final Logger logger =
LoggerFactory.getLogger(ITTestLargeArrowBuf.class);
Review comment:
As Oscar Wilde says:
> Consistency is the hallmark of the unimaginative.
I have added https://issues.apache.org/jira/browse/ARROW-8739 to track
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:
us...@infra.apache.org
[GitHub] [arrow] rymurr commented on a change in pull request #7100: ARROW-8696: [Java] Convert tests to maven failsafe
rymurr commented on a change in pull request #7100:
URL: https://github.com/apache/arrow/pull/7100#discussion_r422079081
##
File path:
java/vector/src/test/java/org/apache/arrow/vector/ITTestLargeVector.java
##
@@ -45,32 +47,33 @@ private static void testLargeLongVector() {
BigIntVector largeVec = new BigIntVector("vec", allocator)) {
largeVec.allocateNew(vecLength);
- System.out.println("Successfully allocated a vector with capacity " +
vecLength);
+ logger.info("Successfully allocated a vector with capacity {}",
vecLength);
for (int i = 0; i < vecLength; i++) {
largeVec.set(i, i * 10L);
if ((i + 1) % 1 == 0) {
- System.out.println("Successfully written " + (i + 1) + " values");
+ logger.debug("Successfully written {} values", i + 1);
}
}
- System.out.println("Successfully written " + vecLength + " values");
+ logger.info("Successfully written {} values", vecLength);
for (int i = 0; i < vecLength; i++) {
long val = largeVec.get(i);
assertEquals(i * 10L, val);
if ((i + 1) % 1 == 0) {
- System.out.println("Successfully read " + (i + 1) + " values");
+ logger.debug("Successfully read {} values", i + 1);
}
}
- System.out.println("Successfully read " + vecLength + " values");
+ logger.info("Successfully read {} values", vecLength);
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:
us...@infra.apache.org
[GitHub] [arrow] rymurr commented on a change in pull request #7100: ARROW-8696: [Java] Convert tests to maven failsafe
rymurr commented on a change in pull request #7100:
URL: https://github.com/apache/arrow/pull/7100#discussion_r420765126
##
File path:
java/vector/src/test/java/org/apache/arrow/vector/ITTestLargeVector.java
##
@@ -131,16 +139,17 @@ private static void testLargeDecimalVector() {
assertEquals(0, buf.getLong(8));
if ((i + 1) % 1 == 0) {
- System.out.println("Successfully read " + (i + 1) + " values");
+ logger.debug("Successfully read {} values", i + 1);
}
}
- System.out.println("Successfully read " + vecLength + " values");
+ logger.info("Successfully read {} values", vecLength);
}
-System.out.println("Successfully released the large vector.");
+logger.info("Successfully released the large vector.");
}
- private static void testLargeFixedSizeBinaryVector() {
-System.out.println("Testing large fixed size binary vector.");
+ @Test
+ public void testLargeFixedSizeBinaryVector() {
+logger.info("Testing large fixed size binary vector.");
Review comment:
thanks @liyafan82 based on your feedback I have moved the failsafe
plugin/profile up to the main pom and have added the same args as surefire
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:
us...@infra.apache.org
[GitHub] [arrow] rymurr commented on a change in pull request #7100: ARROW-8696: [Java] Convert tests to maven failsafe
rymurr commented on a change in pull request #7100:
URL: https://github.com/apache/arrow/pull/7100#discussion_r420597489
##
File path:
java/vector/src/test/java/org/apache/arrow/vector/ITTestLargeVector.java
##
@@ -131,16 +139,17 @@ private static void testLargeDecimalVector() {
assertEquals(0, buf.getLong(8));
if ((i + 1) % 1 == 0) {
- System.out.println("Successfully read " + (i + 1) + " values");
+ logger.debug("Successfully read {} values", i + 1);
}
}
- System.out.println("Successfully read " + vecLength + " values");
+ logger.info("Successfully read {} values", vecLength);
}
-System.out.println("Successfully released the large vector.");
+logger.info("Successfully released the large vector.");
}
- private static void testLargeFixedSizeBinaryVector() {
-System.out.println("Testing large fixed size binary vector.");
+ @Test
+ public void testLargeFixedSizeBinaryVector() {
+logger.info("Testing large fixed size binary vector.");
Review comment:
Not sure I understand the question. I will answer what I think but
please let me know if I am misunderstanding . these tests are run as part of
the failsafe plugin (by way of the IT prefix on the class name) as such they
are only run when the `integration-test` profile is active.
I see the `arrow.vector.max_allocation_bytes` is set to `Long.MAX_VALUE` by
default and the original test didn't have any docs/settings for changing that
default. If it is supposed to be set to a specific value for the test we can
specify in failsafe args
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:
us...@infra.apache.org
[GitHub] [arrow] rymurr commented on a change in pull request #7100: ARROW-8696: [Java] Convert tests to maven failsafe
rymurr commented on a change in pull request #7100: URL: https://github.com/apache/arrow/pull/7100#discussion_r420587311 ## File path: java/vector/src/test/java/org/apache/arrow/vector/ITTestLargeVector.java ## @@ -33,10 +37,12 @@ * main method. Review comment: removed 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: us...@infra.apache.org
[GitHub] [arrow] rymurr commented on a change in pull request #7100: ARROW-8696: [Java] Convert tests to maven failsafe
rymurr commented on a change in pull request #7100: URL: https://github.com/apache/arrow/pull/7100#discussion_r420587154 ## File path: java/memory/src/test/java/org/apache/arrow/memory/ITTestLargeArrowBuf.java ## @@ -27,38 +32,45 @@ * main method. Review comment: removed 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: us...@infra.apache.org
[GitHub] [arrow] rymurr commented on a change in pull request #7100: ARROW-8696: [Java] Convert tests to maven failsafe
rymurr commented on a change in pull request #7100:
URL: https://github.com/apache/arrow/pull/7100#discussion_r420586995
##
File path:
java/memory/src/test/java/org/apache/arrow/memory/ITTestLargeArrowBuf.java
##
@@ -27,38 +32,45 @@
* main method.
*
*/
-public class TestLargeArrowBuf {
+public class ITTestLargeArrowBuf {
+ private static final Logger logger =
LoggerFactory.getLogger(ITTestLargeArrowBuf.class);
Review comment:
logger is (in my experience anyways) an exception to the rule. Currently
in arrow logger is used in 24 places and LOGGER in 15. I am happy with either
way as long as we are consistent
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:
us...@infra.apache.org
